From f2eea334c69ebcbc95817e7acbb69630a1d63265 Mon Sep 17 00:00:00 2001 From: Kyle Harding Date: Mon, 2 Feb 2026 12:53:42 -0500 Subject: [PATCH] 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 --- lib/plugins/variables.js | 90 ++++++++--- test/unit/lib/plugins/variables.test.js | 199 +++++++++++++++++++++++- 2 files changed, 267 insertions(+), 22 deletions(-) diff --git a/lib/plugins/variables.js b/lib/plugins/variables.js index 25795c40..37f1f7ec 100644 --- a/lib/plugins/variables.js +++ b/lib/plugins/variables.js @@ -1,5 +1,6 @@ const _ = require('lodash') const Diffable = require('./diffable') +const NopCommand = require('../nopcommand') module.exports = class Variables extends Diffable { constructor (...args) { @@ -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) => { @@ -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 @@ -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 } } } @@ -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, @@ -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, diff --git a/test/unit/lib/plugins/variables.test.js b/test/unit/lib/plugins/variables.test.js index 2784d7af..41dc80e0 100644 --- a/test/unit/lib/plugins/variables.test.js +++ b/test/unit/lib/plugins/variables.test.js @@ -1,5 +1,6 @@ const { when } = require('jest-when') const Variables = require('../../../../lib/plugins/variables') +const NopCommand = require('../../../../lib/nopcommand') describe('Variables', () => { let github @@ -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)) } @@ -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' + }) + ) + }) + }) + }) })