Skip to content

Commit f2eea33

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 f2eea33

File tree

2 files changed

+267
-22
lines changed

2 files changed

+267
-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.length === 1 ? nopCommands[0] : 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: 195 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,194 @@ 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+
it('should return flat NopCommand array when updating variable value via sync', async () => {
112+
const log = { debug: jest.fn(), error: console.error }
113+
const errors = []
114+
const plugin = new Variables(true, github, { owner: org, repo }, [{ name: 'TEST', value: 'new-value' }], log, errors)
115+
116+
when(github.request)
117+
.calledWith('GET /repos/:org/:repo/actions/variables', { org, repo })
118+
.mockResolvedValue({
119+
data: {
120+
variables: [{ name: 'TEST', value: 'old-value' }]
121+
}
122+
})
123+
124+
const result = await plugin.sync()
125+
126+
// Should have made GET call
127+
expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/actions/variables', { org, repo })
128+
129+
// Should NOT have made any mutating calls
130+
expect(github.request).not.toHaveBeenCalledWith(
131+
expect.stringMatching(/^(POST|PATCH|DELETE)/),
132+
expect.anything()
133+
)
134+
135+
// Result should be a flat array of NopCommands (not nested)
136+
expect(Array.isArray(result)).toBe(true)
137+
result.forEach(cmd => {
138+
expect(cmd).toBeInstanceOf(NopCommand)
139+
expect(Array.isArray(cmd)).toBe(false)
140+
})
141+
})
142+
})
143+
144+
describe('add', () => {
145+
it('should return NopCommand and not make API call when nop is true', async () => {
146+
const plugin = configure(true)
147+
const variable = { name: 'NEW_VAR', value: 'new-value' }
148+
149+
const result = await plugin.add(variable)
150+
151+
expect(result).toBeInstanceOf(NopCommand)
152+
expect(result.plugin).toBe('Variables')
153+
expect(github.request).not.toHaveBeenCalled()
154+
})
155+
156+
it('should make API call when nop is false', async () => {
157+
const plugin = configure(false)
158+
const variable = { name: 'NEW_VAR', value: 'new-value' }
159+
160+
await plugin.add(variable)
161+
162+
expect(github.request).toHaveBeenCalledWith(
163+
'POST /repos/:org/:repo/actions/variables',
164+
expect.objectContaining({
165+
org,
166+
repo,
167+
name: 'NEW_VAR',
168+
value: 'new-value'
169+
})
170+
)
171+
})
172+
})
173+
174+
describe('remove', () => {
175+
it('should return NopCommand and not make API call when nop is true', async () => {
176+
const plugin = configure(true)
177+
const existing = { name: 'EXISTING_VAR', value: 'existing-value' }
178+
179+
const result = await plugin.remove(existing)
180+
181+
expect(result).toBeInstanceOf(NopCommand)
182+
expect(result.plugin).toBe('Variables')
183+
expect(github.request).not.toHaveBeenCalled()
184+
})
185+
186+
it('should make API call when nop is false', async () => {
187+
const plugin = configure(false)
188+
const existing = { name: 'EXISTING_VAR', value: 'existing-value' }
189+
190+
await plugin.remove(existing)
191+
192+
expect(github.request).toHaveBeenCalledWith(
193+
'DELETE /repos/:org/:repo/actions/variables/:variable_name',
194+
expect.objectContaining({
195+
org,
196+
repo,
197+
variable_name: 'EXISTING_VAR'
198+
})
199+
)
200+
})
201+
})
202+
203+
describe('update', () => {
204+
it('should return single NopCommand for single operation with nop true', async () => {
205+
const plugin = configure(true)
206+
const existing = { name: 'VAR1', value: 'old-value' }
207+
const updated = { name: 'VAR1', value: 'new-value' }
208+
209+
const result = await plugin.update(existing, updated)
210+
211+
expect(result).toBeInstanceOf(NopCommand)
212+
expect(result.plugin).toBe('Variables')
213+
expect(github.request).not.toHaveBeenCalled()
214+
})
215+
216+
it('should return single NopCommand when adding new variable in update with nop true', async () => {
217+
const plugin = configure(true)
218+
const existing = []
219+
const updated = [{ name: 'NEW_VAR', value: 'new-value' }]
220+
221+
const result = await plugin.update(existing, updated)
222+
223+
expect(result).toBeInstanceOf(NopCommand)
224+
expect(github.request).not.toHaveBeenCalled()
225+
})
226+
227+
it('should return single NopCommand when deleting variable in update with nop true', async () => {
228+
const plugin = configure(true)
229+
const existing = [{ name: 'OLD_VAR', value: 'old-value' }]
230+
const updated = []
231+
232+
const result = await plugin.update(existing, updated)
233+
234+
expect(result).toBeInstanceOf(NopCommand)
235+
expect(github.request).not.toHaveBeenCalled()
236+
})
237+
238+
it('should return multiple NopCommands for multiple operations with nop true', async () => {
239+
const plugin = configure(true)
240+
const existing = [{ name: 'UPDATE_VAR', value: 'old' }, { name: 'DELETE_VAR', value: 'delete-me' }]
241+
const updated = [{ name: 'UPDATE_VAR', value: 'new' }, { name: 'ADD_VAR', value: 'added' }]
242+
243+
const result = await plugin.update(existing, updated)
244+
245+
expect(Array.isArray(result)).toBe(true)
246+
expect(result).toHaveLength(3) // 1 update + 1 add + 1 delete
247+
result.forEach(cmd => expect(cmd).toBeInstanceOf(NopCommand))
248+
expect(github.request).not.toHaveBeenCalled()
249+
})
250+
251+
it('should make API calls when nop is false', async () => {
252+
const plugin = configure(false)
253+
const existing = [{ name: 'VAR1', value: 'old-value' }]
254+
const updated = [{ name: 'VAR1', value: 'new-value' }]
255+
256+
await plugin.update(existing, updated)
257+
258+
expect(github.request).toHaveBeenCalledWith(
259+
'PATCH /repos/:org/:repo/actions/variables/:variable_name',
260+
expect.objectContaining({
261+
org,
262+
repo,
263+
variable_name: 'VAR1',
264+
value: 'new-value'
265+
})
266+
)
267+
})
268+
})
269+
})
79270
})

0 commit comments

Comments
 (0)