Skip to content

Commit 5469c90

Browse files
klutchellklutchell
authored
Add tests to update and delete environment vars (#766)
* Lint environments.js Signed-off-by: Kyle Harding <[email protected]> * Restore list of variables to be deleted Signed-off-by: Kyle Harding <[email protected]> * Add tests to update and delete environment vars Signed-off-by: Kyle Harding <[email protected]> --------- Signed-off-by: Kyle Harding <[email protected]>
1 parent 9dbe6c7 commit 5469c90

File tree

2 files changed

+173
-45
lines changed

2 files changed

+173
-45
lines changed

‎lib/plugins/environments.js‎

Lines changed: 46 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
const Diffable = require('./diffable')
2-
const MergeDeep = require('../mergeDeep')
32
const NopCommand = require('../nopcommand')
43

54
module.exports = class Environments extends Diffable {
@@ -19,13 +18,13 @@ module.exports = class Environments extends Diffable {
1918
}
2019
}
2120

22-
async nopifyRequest(url, options, description) {
21+
async nopifyRequest (url, options, description) {
2322
if (!this.nop) {
24-
await this.github.request(url, options);
23+
await this.github.request(url, options)
2524
} else {
26-
return Promise.resolve([
27-
new NopCommand(this.constructor.name, this.repo, url, description)
28-
])
25+
return Promise.resolve([
26+
new NopCommand(this.constructor.name, this.repo, url, description)
27+
])
2928
}
3029
}
3130

@@ -147,7 +146,7 @@ module.exports = class Environments extends Diffable {
147146
custom_branch_policies: !!attrs.deployment_branch_policy.custom_branch_policies
148147
}
149148
}
150-
await this.nopifyRequest(`PUT /repos/:org/:repo/environments/:environment_name`, options, 'Update environment settings');
149+
await this.nopifyRequest('PUT /repos/:org/:repo/environments/:environment_name', options, 'Update environment settings')
151150
}
152151

153152
if (deploymentBranchPolicy && attrs.deployment_branch_policy && attrs.deployment_branch_policy.custom_branch_policies) {
@@ -157,14 +156,14 @@ module.exports = class Environments extends Diffable {
157156
await this.nopifyRequest('DELETE /repos/:org/:repo/environments/:environment_name/deployment-branch-policies/:branch_policy_id', {
158157
...baseRequestOptions,
159158
branch_policy_id: policy.id
160-
}, 'Delete deployment branch policy')
159+
}, 'Delete deployment branch policy')
161160
}
162161

163162
for (const policy of attrs.deployment_branch_policy.custom_branch_policies) {
164163
await this.nopifyRequest(
165-
'POST /repos/:org/:repo/environments/:environment_name/deployment-branch-policies',{
166-
...baseRequestOptions,
167-
name: policy
164+
'POST /repos/:org/:repo/environments/:environment_name/deployment-branch-policies', {
165+
...baseRequestOptions,
166+
name: policy
168167
}, 'Create deployment branch policy')
169168
}
170169
}
@@ -175,22 +174,23 @@ module.exports = class Environments extends Diffable {
175174
for (const variable of attrs.variables) {
176175
const existingVariable = existingVariables.find((_var) => _var.name === variable.name)
177176
if (existingVariable) {
178-
existingVariables = existingVariables.filter(_var => _var.name === variable.name)
177+
// Maintain a list of existing variables that will be deleted after the patch/post loop
178+
existingVariables = existingVariables.filter(_var => _var.name !== variable.name)
179179
if (existingVariable.value !== variable.value) {
180180
await this.nopifyRequest(
181181
'PATCH /repos/:org/:repo/environments/:environment_name/variables/:variable_name', {
182-
...baseRequestOptions,
183-
variable_name: variable.name,
184-
value: variable.value
182+
...baseRequestOptions,
183+
variable_name: variable.name,
184+
value: variable.value
185185
}, 'Update environment variable'
186-
)
186+
)
187187
}
188188
} else {
189189
await this.nopifyRequest(
190190
'POST /repos/:org/:repo/environments/:environment_name/variables', {
191-
...baseRequestOptions,
192-
name: variable.name,
193-
value: variable.value
191+
...baseRequestOptions,
192+
name: variable.name,
193+
value: variable.value
194194
}, 'Create environment variable'
195195
)
196196
}
@@ -199,8 +199,8 @@ module.exports = class Environments extends Diffable {
199199
for (const variable of existingVariables) {
200200
await this.nopifyRequest(
201201
'DELETE /repos/:org/:repo/environments/:environment_name/variables/:variable_name', {
202-
...baseRequestOptions,
203-
variable_name: variable.name
202+
...baseRequestOptions,
203+
variable_name: variable.name
204204
}, 'Delete environment variable'
205205
)
206206
}
@@ -215,19 +215,18 @@ module.exports = class Environments extends Diffable {
215215
if (!existingRule) {
216216
await this.nopifyRequest(
217217
'POST /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', {
218-
...baseRequestOptions,
219-
integration_id: rule.app_id
218+
...baseRequestOptions,
219+
integration_id: rule.app_id
220220
}, 'Create deployment protection rule'
221-
)
221+
)
222222
}
223223
}
224224
for (const rule of existingRules) {
225225
await this.nopifyRequest('DELETE /repos/:org/:repo/environments/:environment_name/deployment_protection_rules/:rule_id', {
226226
...baseRequestOptions,
227227
rule_id: rule.id
228-
}, "Delete deployment protection rule")
228+
}, 'Delete deployment protection rule')
229229
}
230-
231230
}
232231
}
233232

@@ -237,48 +236,50 @@ module.exports = class Environments extends Diffable {
237236
repo: this.repo.repo,
238237
environment_name: attrs.name
239238
}
240-
239+
241240
await this.nopifyRequest(
242241
'PUT /repos/:org/:repo/environments/:environment_name', {
243-
...baseRequestOptions,
244-
wait_timer: attrs.wait_timer,
245-
prevent_self_review: attrs.prevent_self_review,
246-
reviewers: attrs.reviewers,
247-
deployment_branch_policy: attrs.deployment_branch_policy == null ? null : {
242+
...baseRequestOptions,
243+
wait_timer: attrs.wait_timer,
244+
prevent_self_review: attrs.prevent_self_review,
245+
reviewers: attrs.reviewers,
246+
deployment_branch_policy: attrs.deployment_branch_policy == null
247+
? null
248+
: {
248249
protected_branches: !!attrs.deployment_branch_policy.protected_branches,
249250
custom_branch_policies: !!attrs.deployment_branch_policy.custom_branch_policies
250-
}
251+
}
251252
}, 'Update environment settings')
252253

253254
if (attrs.deployment_branch_policy && attrs.deployment_branch_policy.custom_branch_policies) {
254255
for (const policy of attrs.deployment_branch_policy.custom_branch_policies) {
255256
await this.nopifyRequest(
256257
'POST /repos/:org/:repo/environments/:environment_name/deployment-branch-policies', {
257-
...baseRequestOptions,
258-
name: policy.name
258+
...baseRequestOptions,
259+
name: policy.name
259260
}, 'Create deployment branch policy'
260261
)
261262
}
262263
}
263264

264265
if (attrs.variables) {
265-
for(const variable of attrs.variables) {
266+
for (const variable of attrs.variables) {
266267
await this.nopifyRequest(
267268
'POST /repos/:org/:repo/environments/:environment_name/variables', {
268269
...baseRequestOptions,
269270
name: variable.name,
270271
value: variable.value
271272
}, 'Create environment variable'
272-
)
273-
}
273+
)
274274
}
275+
}
275276

276277
if (attrs.deployment_protection_rules) {
277278
for (const rule of attrs.deployment_protection_rules) {
278279
await this.nopifyRequest(
279280
'POST /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', {
280-
...baseRequestOptions,
281-
integration_id: rule.app_id
281+
...baseRequestOptions,
282+
integration_id: rule.app_id
282283
}, 'Create deployment protection rule'
283284
)
284285
}
@@ -293,16 +294,16 @@ module.exports = class Environments extends Diffable {
293294
}
294295

295296
await this.nopifyRequest(
296-
'DELETE /repos/:org/:repo/environments/:environment_name', {
297-
...baseRequestOptions
298-
}, 'Delete environment'
297+
'DELETE /repos/:org/:repo/environments/:environment_name', {
298+
...baseRequestOptions
299+
}, 'Delete environment'
299300
)
300-
}
301+
}
301302

302303
sync () {
303304
const resArray = []
304305
if (this.entries) {
305-
let filteredEntries = this.filterEntries()
306+
const filteredEntries = this.filterEntries()
306307
return this.find().then(existingRecords => {
307308
// Remove any null or undefined values from the diffables (usually comes from repo override)
308309
for (const entry of filteredEntries) {

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

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,133 @@ describe('Environments Plugin test suite', () => {
403403
})
404404
})
405405

406+
// update variable
407+
describe('When there is an existing variable and config calls for a different value', () => {
408+
it('detect divergence and update the variable', async () => {
409+
// arrange
410+
environmentName = 'variables_environment'
411+
// represent config with a reviewers being a user and a team
412+
const plugin = new Environments(undefined, github, { owner: org, repo }, [
413+
{
414+
name: environmentName,
415+
variables: [
416+
{
417+
name: 'TEST',
418+
value: 'test-updated'
419+
}
420+
]
421+
}
422+
], log, errors)
423+
424+
// model an existing environment with a variable that has a different value
425+
when(github.request)
426+
.calledWith('GET /repos/:org/:repo/environments', { org, repo })
427+
.mockResolvedValue({
428+
data: {
429+
environments: [
430+
fillEnvironment({
431+
name: environmentName
432+
})
433+
]
434+
}
435+
})
436+
437+
// model an existing environment with a variable that has a different value
438+
when(github.request)
439+
.calledWith('GET /repos/:org/:repo/environments/:environment_name/variables', { org, repo, environment_name: environmentName })
440+
.mockResolvedValue({
441+
data: {
442+
variables: [
443+
{
444+
name: 'TEST',
445+
value: 'test'
446+
}
447+
]
448+
}
449+
})
450+
451+
// act - run sync() in environments.js
452+
await plugin.sync().then(() => {
453+
// assert - update the variables
454+
expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments', { org, repo })
455+
expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/variables', { org, repo, environment_name: environmentName })
456+
expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', { org, repo, environment_name: environmentName })
457+
expect(github.request).toHaveBeenCalledWith('PATCH /repos/:org/:repo/environments/:environment_name/variables/:variable_name', expect.objectContaining({
458+
org,
459+
repo,
460+
environment_name: environmentName,
461+
variable_name: 'test',
462+
value: 'test-updated'
463+
}))
464+
})
465+
})
466+
})
467+
468+
// delete variable
469+
describe('When there are multiple variables and config calls for one to be deleted', () => {
470+
it('detect divergence and delete the variable', async () => {
471+
// arrange
472+
environmentName = 'variables_environment'
473+
// represent config with a reviewers being a user and a team
474+
const plugin = new Environments(undefined, github, { owner: org, repo }, [
475+
{
476+
name: environmentName,
477+
variables: [
478+
{
479+
name: 'TEST',
480+
value: 'test'
481+
}
482+
]
483+
}
484+
], log, errors)
485+
486+
// model an existing environment with a variable that has a different value
487+
when(github.request)
488+
.calledWith('GET /repos/:org/:repo/environments', { org, repo })
489+
.mockResolvedValue({
490+
data: {
491+
environments: [
492+
fillEnvironment({
493+
name: environmentName
494+
})
495+
]
496+
}
497+
})
498+
499+
// model an existing environment with a variable that has a different value
500+
when(github.request)
501+
.calledWith('GET /repos/:org/:repo/environments/:environment_name/variables', { org, repo, environment_name: environmentName })
502+
.mockResolvedValue({
503+
data: {
504+
variables: [
505+
{
506+
name: 'TEST',
507+
value: 'test'
508+
},
509+
{
510+
name: 'TEST2',
511+
value: 'test2'
512+
}
513+
]
514+
}
515+
})
516+
517+
// act - run sync() in environments.js
518+
await plugin.sync().then(() => {
519+
// assert - update the variables
520+
expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments', { org, repo })
521+
expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/variables', { org, repo, environment_name: environmentName })
522+
expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/environments/:environment_name/deployment_protection_rules', { org, repo, environment_name: environmentName })
523+
expect(github.request).toHaveBeenCalledWith('DELETE /repos/:org/:repo/environments/:environment_name/variables/:variable_name', expect.objectContaining({
524+
org,
525+
repo,
526+
environment_name: environmentName,
527+
variable_name: 'test2'
528+
}))
529+
})
530+
})
531+
})
532+
406533
// add deployment protection rules
407534
describe('When there are no existing deployment protection rules, but config calls for one', () => {
408535
it('detect divergence and add the deployment protection rule', async () => {

0 commit comments

Comments
 (0)