Skip to content

Commit 723c3be

Browse files
klutchellklutchell
committed
feat(variables): add noop mode support
- Add noop mode support to Variables plugin add/remove/update methods - Return NopCommand instead of making API calls when nop=true - Add comprehensive tests for noop mode behavior Signed-off-by: Kyle Harding <kyle@balena.io>
1 parent 447021f commit 723c3be

File tree

2 files changed

+241
-22
lines changed

2 files changed

+241
-22
lines changed

‎lib/plugins/variables.js‎

Lines changed: 72 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
const _ = require('lodash')
22
const Diffable = require('./diffable')
3+
const NopCommand = require('../nopcommand')
34

45
module.exports = class Variables extends Diffable {
56
constructor (...args) {
@@ -91,17 +92,51 @@ module.exports = class Variables extends Diffable {
9192
const changed = this.getChanged(existing, variables)
9293

9394
if (changed) {
95+
const nopCommands = []
9496
let existingVariables = [...existing]
97+
9598
for (const variable of variables) {
9699
const existingVariable = existingVariables.find((_var) => _var.name === variable.name)
97100
if (existingVariable) {
98101
existingVariables = existingVariables.filter((_var) => _var.name !== variable.name)
99102
if (existingVariable.value !== variable.value) {
103+
if (this.nop) {
104+
nopCommands.push(new NopCommand(
105+
this.constructor.name,
106+
this.repo,
107+
null,
108+
`Update variable ${variable.name}`
109+
))
110+
} else {
111+
await this.github
112+
.request('PATCH /repos/:org/:repo/actions/variables/:variable_name', {
113+
org: this.repo.owner,
114+
repo: this.repo.repo,
115+
variable_name: variable.name.toUpperCase(),
116+
value: variable.value.toString()
117+
})
118+
.then((res) => {
119+
return res
120+
})
121+
.catch((e) => {
122+
this.logError(e)
123+
})
124+
}
125+
}
126+
} else {
127+
if (this.nop) {
128+
nopCommands.push(new NopCommand(
129+
this.constructor.name,
130+
this.repo,
131+
null,
132+
`Add variable ${variable.name}`
133+
))
134+
} else {
100135
await this.github
101-
.request('PATCH /repos/:org/:repo/actions/variables/:variable_name', {
136+
.request('POST /repos/:org/:repo/actions/variables', {
102137
org: this.repo.owner,
103138
repo: this.repo.repo,
104-
variable_name: variable.name.toUpperCase(),
139+
name: variable.name.toUpperCase(),
105140
value: variable.value.toString()
106141
})
107142
.then((res) => {
@@ -111,13 +146,23 @@ module.exports = class Variables extends Diffable {
111146
this.logError(e)
112147
})
113148
}
149+
}
150+
}
151+
152+
for (const variable of existingVariables) {
153+
if (this.nop) {
154+
nopCommands.push(new NopCommand(
155+
this.constructor.name,
156+
this.repo,
157+
null,
158+
`Remove variable ${variable.name}`
159+
))
114160
} else {
115161
await this.github
116-
.request('POST /repos/:org/:repo/actions/variables', {
162+
.request('DELETE /repos/:org/:repo/actions/variables/:variable_name', {
117163
org: this.repo.owner,
118164
repo: this.repo.repo,
119-
name: variable.name.toUpperCase(),
120-
value: variable.value.toString()
165+
variable_name: variable.name.toUpperCase()
121166
})
122167
.then((res) => {
123168
return res
@@ -128,19 +173,8 @@ module.exports = class Variables extends Diffable {
128173
}
129174
}
130175

131-
for (const variable of existingVariables) {
132-
await this.github
133-
.request('DELETE /repos/:org/:repo/actions/variables/:variable_name', {
134-
org: this.repo.owner,
135-
repo: this.repo.repo,
136-
variable_name: variable.name.toUpperCase()
137-
})
138-
.then((res) => {
139-
return res
140-
})
141-
.catch((e) => {
142-
this.logError(e)
143-
})
176+
if (this.nop) {
177+
return nopCommands
144178
}
145179
}
146180
}
@@ -155,6 +189,16 @@ module.exports = class Variables extends Diffable {
155189
*/
156190
async add (variable) {
157191
this.log.debug(`Adding a repo var with the params ${JSON.stringify(variable)}`)
192+
193+
if (this.nop) {
194+
return new NopCommand(
195+
this.constructor.name,
196+
this.repo,
197+
null,
198+
`Add variable ${variable.name}`
199+
)
200+
}
201+
158202
await this.github
159203
.request('POST /repos/:org/:repo/actions/variables', {
160204
org: this.repo.owner,
@@ -180,6 +224,16 @@ module.exports = class Variables extends Diffable {
180224
*/
181225
async remove (existing) {
182226
this.log.debug(`Removing a repo var with the params ${JSON.stringify(existing)}`)
227+
228+
if (this.nop) {
229+
return new NopCommand(
230+
this.constructor.name,
231+
this.repo,
232+
null,
233+
`Remove variable ${existing.name}`
234+
)
235+
}
236+
183237
await this.github
184238
.request('DELETE /repos/:org/:repo/actions/variables/:variable_name', {
185239
org: this.repo.owner,

‎test/unit/lib/plugins/variables.test.js‎

Lines changed: 169 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
const { when } = require('jest-when')
22
const Variables = require('../../../../lib/plugins/variables')
3+
const NopCommand = require('../../../../lib/nopcommand')
34

45
describe('Variables', () => {
56
let github
@@ -10,13 +11,13 @@ describe('Variables', () => {
1011
return variables
1112
}
1213

13-
function configure () {
14-
const log = { debug: console.debug, error: console.error }
14+
function configure (nop = false) {
15+
const log = { debug: jest.fn(), error: console.error }
1516
const errors = []
16-
return new Variables(undefined, github, { owner: org, repo }, [{ name: 'test', value: 'test' }], log, errors)
17+
return new Variables(nop, github, { owner: org, repo }, [{ name: 'test', value: 'test' }], log, errors)
1718
}
1819

19-
beforeAll(() => {
20+
beforeEach(() => {
2021
github = {
2122
request: jest.fn().mockReturnValue(Promise.resolve(true))
2223
}
@@ -76,4 +77,168 @@ describe('Variables', () => {
7677
)
7778
})
7879
})
80+
81+
describe('noop mode', () => {
82+
describe('sync', () => {
83+
it('should return NopCommands and not make mutating API calls when nop is true', async () => {
84+
const plugin = configure(true)
85+
86+
when(github.request)
87+
.calledWith('GET /repos/:org/:repo/actions/variables', { org, repo })
88+
.mockResolvedValue({
89+
data: {
90+
variables: [{ name: 'EXISTING_VAR', value: 'existing-value' }]
91+
}
92+
})
93+
94+
const result = await plugin.sync()
95+
96+
// Should have made GET call to fetch existing variables
97+
expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/actions/variables', { org, repo })
98+
99+
// Should NOT have made any mutating calls (POST, PATCH, DELETE)
100+
expect(github.request).not.toHaveBeenCalledWith(
101+
expect.stringMatching(/^(POST|PATCH|DELETE)/),
102+
expect.anything()
103+
)
104+
105+
// Result should contain NopCommands
106+
expect(Array.isArray(result)).toBe(true)
107+
expect(result.length).toBeGreaterThan(0)
108+
result.forEach(cmd => expect(cmd).toBeInstanceOf(NopCommand))
109+
})
110+
})
111+
112+
describe('add', () => {
113+
it('should return NopCommand and not make API call when nop is true', async () => {
114+
const plugin = configure(true)
115+
const variable = { name: 'NEW_VAR', value: 'new-value' }
116+
117+
const result = await plugin.add(variable)
118+
119+
expect(result).toBeInstanceOf(NopCommand)
120+
expect(result.plugin).toBe('Variables')
121+
expect(github.request).not.toHaveBeenCalled()
122+
})
123+
124+
it('should make API call when nop is false', async () => {
125+
const plugin = configure(false)
126+
const variable = { name: 'NEW_VAR', value: 'new-value' }
127+
128+
await plugin.add(variable)
129+
130+
expect(github.request).toHaveBeenCalledWith(
131+
'POST /repos/:org/:repo/actions/variables',
132+
expect.objectContaining({
133+
org,
134+
repo,
135+
name: 'NEW_VAR',
136+
value: 'new-value'
137+
})
138+
)
139+
})
140+
})
141+
142+
describe('remove', () => {
143+
it('should return NopCommand and not make API call when nop is true', async () => {
144+
const plugin = configure(true)
145+
const existing = { name: 'EXISTING_VAR', value: 'existing-value' }
146+
147+
const result = await plugin.remove(existing)
148+
149+
expect(result).toBeInstanceOf(NopCommand)
150+
expect(result.plugin).toBe('Variables')
151+
expect(github.request).not.toHaveBeenCalled()
152+
})
153+
154+
it('should make API call when nop is false', async () => {
155+
const plugin = configure(false)
156+
const existing = { name: 'EXISTING_VAR', value: 'existing-value' }
157+
158+
await plugin.remove(existing)
159+
160+
expect(github.request).toHaveBeenCalledWith(
161+
'DELETE /repos/:org/:repo/actions/variables/:variable_name',
162+
expect.objectContaining({
163+
org,
164+
repo,
165+
variable_name: 'EXISTING_VAR'
166+
})
167+
)
168+
})
169+
})
170+
171+
describe('update', () => {
172+
it('should return NopCommand array and not make API calls when nop is true', async () => {
173+
const plugin = configure(true)
174+
const existing = [{ name: 'VAR1', value: 'old-value' }]
175+
const updated = [{ name: 'VAR1', value: 'new-value' }]
176+
177+
const result = await plugin.update(existing, updated)
178+
179+
expect(Array.isArray(result)).toBe(true)
180+
expect(result).toHaveLength(1)
181+
expect(result[0]).toBeInstanceOf(NopCommand)
182+
expect(result[0].plugin).toBe('Variables')
183+
expect(github.request).not.toHaveBeenCalled()
184+
})
185+
186+
it('should return NopCommand when adding new variable in update with nop true', async () => {
187+
const plugin = configure(true)
188+
const existing = []
189+
const updated = [{ name: 'NEW_VAR', value: 'new-value' }]
190+
191+
const result = await plugin.update(existing, updated)
192+
193+
expect(Array.isArray(result)).toBe(true)
194+
expect(result).toHaveLength(1)
195+
expect(result[0]).toBeInstanceOf(NopCommand)
196+
expect(github.request).not.toHaveBeenCalled()
197+
})
198+
199+
it('should return NopCommand when deleting variable in update with nop true', async () => {
200+
const plugin = configure(true)
201+
const existing = [{ name: 'OLD_VAR', value: 'old-value' }]
202+
const updated = []
203+
204+
const result = await plugin.update(existing, updated)
205+
206+
expect(Array.isArray(result)).toBe(true)
207+
expect(result).toHaveLength(1)
208+
expect(result[0]).toBeInstanceOf(NopCommand)
209+
expect(github.request).not.toHaveBeenCalled()
210+
})
211+
212+
it('should return multiple NopCommands for multiple operations with nop true', async () => {
213+
const plugin = configure(true)
214+
const existing = [{ name: 'UPDATE_VAR', value: 'old' }, { name: 'DELETE_VAR', value: 'delete-me' }]
215+
const updated = [{ name: 'UPDATE_VAR', value: 'new' }, { name: 'ADD_VAR', value: 'added' }]
216+
217+
const result = await plugin.update(existing, updated)
218+
219+
expect(Array.isArray(result)).toBe(true)
220+
expect(result).toHaveLength(3) // 1 update + 1 add + 1 delete
221+
result.forEach(cmd => expect(cmd).toBeInstanceOf(NopCommand))
222+
expect(github.request).not.toHaveBeenCalled()
223+
})
224+
225+
it('should make API calls when nop is false', async () => {
226+
const plugin = configure(false)
227+
const existing = [{ name: 'VAR1', value: 'old-value' }]
228+
const updated = [{ name: 'VAR1', value: 'new-value' }]
229+
230+
await plugin.update(existing, updated)
231+
232+
expect(github.request).toHaveBeenCalledWith(
233+
'PATCH /repos/:org/:repo/actions/variables/:variable_name',
234+
expect.objectContaining({
235+
org,
236+
repo,
237+
variable_name: 'VAR1',
238+
value: 'new-value'
239+
})
240+
)
241+
})
242+
})
243+
})
79244
})

0 commit comments

Comments
 (0)