Skip to content

fix(variables): add noop mode support #926

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
klutchell wants to merge 1 commit into
base: main-enterprise
Choose a base branch
from
Open

fix(variables): add noop mode support #926

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 72 additions & 18 deletions lib/plugins/variables.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const _ = require('lodash')
const Diffable = require('./diffable')
const NopCommand = require('../nopcommand')

module.exports = class Variables extends Diffable {
constructor (...args) {
Expand Down Expand Up @@ -91,17 +92,51 @@ module.exports = class Variables extends Diffable {
const changed = this.getChanged(existing, variables)

if (changed) {
const nopCommands = []
let existingVariables = [...existing]

for (const variable of variables) {
const existingVariable = existingVariables.find((_var) => _var.name === variable.name)
if (existingVariable) {
existingVariables = existingVariables.filter((_var) => _var.name !== variable.name)
if (existingVariable.value !== variable.value) {
if (this.nop) {
nopCommands.push(new NopCommand(
this.constructor.name,
this.repo,
null,
`Update variable ${variable.name}`
))
} else {
await this.github
.request('PATCH /repos/:org/:repo/actions/variables/:variable_name', {
org: this.repo.owner,
repo: this.repo.repo,
variable_name: variable.name.toUpperCase(),
value: variable.value.toString()
})
.then((res) => {
return res
})
.catch((e) => {
this.logError(e)
})
}
}
} else {
if (this.nop) {
nopCommands.push(new NopCommand(
this.constructor.name,
this.repo,
null,
`Add variable ${variable.name}`
))
} else {
await this.github
.request('PATCH /repos/:org/:repo/actions/variables/:variable_name', {
.request('POST /repos/:org/:repo/actions/variables', {
org: this.repo.owner,
repo: this.repo.repo,
variable_name: variable.name.toUpperCase(),
name: variable.name.toUpperCase(),
value: variable.value.toString()
})
.then((res) => {
Expand All @@ -111,13 +146,23 @@ module.exports = class Variables extends Diffable {
this.logError(e)
})
}
}
}

for (const variable of existingVariables) {
if (this.nop) {
nopCommands.push(new NopCommand(
this.constructor.name,
this.repo,
null,
`Remove variable ${variable.name}`
))
} else {
await this.github
.request('POST /repos/:org/:repo/actions/variables', {
.request('DELETE /repos/:org/:repo/actions/variables/:variable_name', {
org: this.repo.owner,
repo: this.repo.repo,
name: variable.name.toUpperCase(),
value: variable.value.toString()
variable_name: variable.name.toUpperCase()
})
.then((res) => {
return res
Expand All @@ -128,19 +173,8 @@ module.exports = class Variables extends Diffable {
}
}

for (const variable of existingVariables) {
await this.github
.request('DELETE /repos/:org/:repo/actions/variables/:variable_name', {
org: this.repo.owner,
repo: this.repo.repo,
variable_name: variable.name.toUpperCase()
})
.then((res) => {
return res
})
.catch((e) => {
this.logError(e)
})
if (this.nop) {
return nopCommands.length === 1 ? nopCommands[0] : nopCommands
}
}
}
Expand All @@ -155,6 +189,16 @@ module.exports = class Variables extends Diffable {
*/
async add (variable) {
this.log.debug(`Adding a repo var with the params ${JSON.stringify(variable)}`)

if (this.nop) {
return new NopCommand(
this.constructor.name,
this.repo,
null,
`Add variable ${variable.name}`
)
}

await this.github
.request('POST /repos/:org/:repo/actions/variables', {
org: this.repo.owner,
Expand All @@ -180,6 +224,16 @@ module.exports = class Variables extends Diffable {
*/
async remove (existing) {
this.log.debug(`Removing a repo var with the params ${JSON.stringify(existing)}`)

if (this.nop) {
return new NopCommand(
this.constructor.name,
this.repo,
null,
`Remove variable ${existing.name}`
)
}

await this.github
.request('DELETE /repos/:org/:repo/actions/variables/:variable_name', {
org: this.repo.owner,
Expand Down
199 changes: 195 additions & 4 deletions test/unit/lib/plugins/variables.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const { when } = require('jest-when')
const Variables = require('../../../../lib/plugins/variables')
const NopCommand = require('../../../../lib/nopcommand')

describe('Variables', () => {
let github
Expand All @@ -10,13 +11,13 @@ describe('Variables', () => {
return variables
}

function configure () {
const log = { debug: console.debug, error: console.error }
function configure (nop = false) {
const log = { debug: jest.fn(), error: console.error }
const errors = []
return new Variables(undefined, github, { owner: org, repo }, [{ name: 'test', value: 'test' }], log, errors)
return new Variables(nop, github, { owner: org, repo }, [{ name: 'test', value: 'test' }], log, errors)
}

beforeAll(() => {
beforeEach(() => {
github = {
request: jest.fn().mockReturnValue(Promise.resolve(true))
}
Expand Down Expand Up @@ -76,4 +77,194 @@ describe('Variables', () => {
)
})
})

describe('noop mode', () => {
describe('sync', () => {
it('should return NopCommands and not make mutating API calls when nop is true', async () => {
const plugin = configure(true)

when(github.request)
.calledWith('GET /repos/:org/:repo/actions/variables', { org, repo })
.mockResolvedValue({
data: {
variables: [{ name: 'EXISTING_VAR', value: 'existing-value' }]
}
})

const result = await plugin.sync()

// Should have made GET call to fetch existing variables
expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/actions/variables', { org, repo })

// Should NOT have made any mutating calls (POST, PATCH, DELETE)
expect(github.request).not.toHaveBeenCalledWith(
expect.stringMatching(/^(POST|PATCH|DELETE)/),
expect.anything()
)

// Result should contain NopCommands
expect(Array.isArray(result)).toBe(true)
expect(result.length).toBeGreaterThan(0)
result.forEach(cmd => expect(cmd).toBeInstanceOf(NopCommand))
})

it('should return flat NopCommand array when updating variable value via sync', async () => {
const log = { debug: jest.fn(), error: console.error }
const errors = []
const plugin = new Variables(true, github, { owner: org, repo }, [{ name: 'TEST', value: 'new-value' }], log, errors)

when(github.request)
.calledWith('GET /repos/:org/:repo/actions/variables', { org, repo })
.mockResolvedValue({
data: {
variables: [{ name: 'TEST', value: 'old-value' }]
}
})

const result = await plugin.sync()

// Should have made GET call
expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/actions/variables', { org, repo })

// Should NOT have made any mutating calls
expect(github.request).not.toHaveBeenCalledWith(
expect.stringMatching(/^(POST|PATCH|DELETE)/),
expect.anything()
)

// Result should be a flat array of NopCommands (not nested)
expect(Array.isArray(result)).toBe(true)
result.forEach(cmd => {
expect(cmd).toBeInstanceOf(NopCommand)
expect(Array.isArray(cmd)).toBe(false)
})
})
})

describe('add', () => {
it('should return NopCommand and not make API call when nop is true', async () => {
const plugin = configure(true)
const variable = { name: 'NEW_VAR', value: 'new-value' }

const result = await plugin.add(variable)

expect(result).toBeInstanceOf(NopCommand)
expect(result.plugin).toBe('Variables')
expect(github.request).not.toHaveBeenCalled()
})

it('should make API call when nop is false', async () => {
const plugin = configure(false)
const variable = { name: 'NEW_VAR', value: 'new-value' }

await plugin.add(variable)

expect(github.request).toHaveBeenCalledWith(
'POST /repos/:org/:repo/actions/variables',
expect.objectContaining({
org,
repo,
name: 'NEW_VAR',
value: 'new-value'
})
)
})
})

describe('remove', () => {
it('should return NopCommand and not make API call when nop is true', async () => {
const plugin = configure(true)
const existing = { name: 'EXISTING_VAR', value: 'existing-value' }

const result = await plugin.remove(existing)

expect(result).toBeInstanceOf(NopCommand)
expect(result.plugin).toBe('Variables')
expect(github.request).not.toHaveBeenCalled()
})

it('should make API call when nop is false', async () => {
const plugin = configure(false)
const existing = { name: 'EXISTING_VAR', value: 'existing-value' }

await plugin.remove(existing)

expect(github.request).toHaveBeenCalledWith(
'DELETE /repos/:org/:repo/actions/variables/:variable_name',
expect.objectContaining({
org,
repo,
variable_name: 'EXISTING_VAR'
})
)
})
})

describe('update', () => {
it('should return single NopCommand for single operation with nop true', async () => {
const plugin = configure(true)
const existing = { name: 'VAR1', value: 'old-value' }
const updated = { name: 'VAR1', value: 'new-value' }

const result = await plugin.update(existing, updated)

expect(result).toBeInstanceOf(NopCommand)
expect(result.plugin).toBe('Variables')
expect(github.request).not.toHaveBeenCalled()
})

it('should return single NopCommand when adding new variable in update with nop true', async () => {
const plugin = configure(true)
const existing = []
const updated = [{ name: 'NEW_VAR', value: 'new-value' }]

const result = await plugin.update(existing, updated)

expect(result).toBeInstanceOf(NopCommand)
expect(github.request).not.toHaveBeenCalled()
})

it('should return single NopCommand when deleting variable in update with nop true', async () => {
const plugin = configure(true)
const existing = [{ name: 'OLD_VAR', value: 'old-value' }]
const updated = []

const result = await plugin.update(existing, updated)

expect(result).toBeInstanceOf(NopCommand)
expect(github.request).not.toHaveBeenCalled()
})

it('should return multiple NopCommands for multiple operations with nop true', async () => {
const plugin = configure(true)
const existing = [{ name: 'UPDATE_VAR', value: 'old' }, { name: 'DELETE_VAR', value: 'delete-me' }]
const updated = [{ name: 'UPDATE_VAR', value: 'new' }, { name: 'ADD_VAR', value: 'added' }]

const result = await plugin.update(existing, updated)

expect(Array.isArray(result)).toBe(true)
expect(result).toHaveLength(3) // 1 update + 1 add + 1 delete
result.forEach(cmd => expect(cmd).toBeInstanceOf(NopCommand))
expect(github.request).not.toHaveBeenCalled()
})

it('should make API calls when nop is false', async () => {
const plugin = configure(false)
const existing = [{ name: 'VAR1', value: 'old-value' }]
const updated = [{ name: 'VAR1', value: 'new-value' }]

await plugin.update(existing, updated)

expect(github.request).toHaveBeenCalledWith(
'PATCH /repos/:org/:repo/actions/variables/:variable_name',
expect.objectContaining({
org,
repo,
variable_name: 'VAR1',
value: 'new-value'
})
)
})
})
})
})
Loading