From c519148155a1a86dd6a60d2fc029e83857f34237 Mon Sep 17 00:00:00 2001 From: Kyle Harding Date: Mon, 2 Feb 2026 12:53:42 -0500 Subject: [PATCH 1/2] 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 | 92 ++++++++--- test/unit/lib/plugins/variables.test.js | 199 +++++++++++++++++++++++- 2 files changed, 268 insertions(+), 23 deletions(-) diff --git a/lib/plugins/variables.js b/lib/plugins/variables.js index 25795c408..5687026df 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) { @@ -25,7 +26,7 @@ module.exports = class Variables extends Diffable { org: this.repo.owner, repo: this.repo.repo }) - return variables + return variables.map(({ name, value }) => ({ name, value })) } /** @@ -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 2784d7afd..41dc80e02 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' + }) + ) + }) + }) + }) }) From 4ca89121366ad2c93664e5f716114c6e0e2f6e75 Mon Sep 17 00:00:00 2001 From: Kyle Harding Date: Tue, 24 Mar 2026 12:04:24 -0400 Subject: [PATCH 2/2] refactor(variables): align with Diffable contract pattern Refactor Variables plugin to match the single-item Diffable contract used by labels, milestones, and other plugins. The previous update() reimplemented sync() logic internally; now each method handles one item and lets Diffable.sync() orchestrate iteration. - Simplify update() from 90-line array-diffing to single-item PATCH - Simplify changed() from JSON.stringify comparison to value check - Remove getChanged(), lodash dependency, .then(res=>res) no-ops - Match labels.js nop return pattern: Promise.resolve([NopCommand]) - Fix inconsistent toUpperCase() between add/remove/update - Let errors propagate to Diffable.sync() instead of swallowing - Normalize find() to strip API metadata fields (created_at, etc.) Co-Authored-By: Claude Opus 4.6 (1M context) Signed-off-by: Kyle Harding --- lib/plugins/variables.js | 247 +++-------------- test/unit/lib/plugins/variables.test.js | 342 ++++++++++-------------- 2 files changed, 172 insertions(+), 417 deletions(-) diff --git a/lib/plugins/variables.js b/lib/plugins/variables.js index 5687026df..a8e65b91e 100644 --- a/lib/plugins/variables.js +++ b/lib/plugins/variables.js @@ -1,4 +1,3 @@ -const _ = require('lodash') const Diffable = require('./diffable') const NopCommand = require('../nopcommand') @@ -7,244 +6,66 @@ module.exports = class Variables extends Diffable { super(...args) if (this.entries) { - // Force all names to uppercase to avoid comparison issues. this.entries.forEach((variable) => { variable.name = variable.name.toUpperCase() }) } } - /** - * Look-up existing variables for a given repository - * - * @see {@link https://docs.github.com/en/rest/actions/variables?apiVersion=2022-11-28#list-repository-variables} list repository variables - * @returns {Array.} Returns a list of variables that exist in a repository - */ - async find () { + find () { this.log.debug(`Finding repo vars for ${this.repo.owner}/${this.repo.repo}`) - const { data: { variables } } = await this.github.request('GET /repos/:org/:repo/actions/variables', { + return this.github.request('GET /repos/:org/:repo/actions/variables', { org: this.repo.owner, repo: this.repo.repo - }) - return variables.map(({ name, value }) => ({ name, value })) - } - - /** - * Compare the existing variables with what we've defined as code - * - * @param {Array.} existing Existing variables defined in the repository - * @param {Array.} variables Variables that we have defined as code - * - * @returns {object} The results of a list comparison - */ - getChanged (existing, variables = []) { - const result = - JSON.stringify( - existing.sort((x1, x2) => { - return x1.name.toUpperCase().localeCompare(x2.name.toUpperCase()) - }) - ) !== - JSON.stringify( - variables.sort((x1, x2) => { - return x1.name.toUpperCase().localeCompare(x2.name.toUpperCase()) - }) - ) - return result + }).then(({ data: { variables } }) => variables.map(({ name, value }) => ({ name, value }))) } - /** - * Compare existing variables with what's defined - * - * @param {Object} existing The existing entries in GitHub - * @param {Object} attrs The entries defined as code - * - * @returns - */ comparator (existing, attrs) { return existing.name === attrs.name } - /** - * Return a list of changed entries - * - * @param {Object} existing The existing entries in GitHub - * @param {Object} attrs The entries defined as code - * - * @returns - */ changed (existing, attrs) { - return this.getChanged(_.castArray(existing), _.castArray(attrs)) + return existing.value !== attrs.value } - /** - * Update an existing variable if the value has changed - * - * @param {Array.} existing Existing variables defined in the repository - * @param {Array.} variables Variables that we have defined as code - * - * @see {@link https://docs.github.com/en/rest/actions/variables?apiVersion=2022-11-28#update-a-repository-variable} update a repository variable - * @returns - */ - async update (existing, variables = []) { - this.log.debug(`Updating a repo var existing params ${JSON.stringify(existing)} and new ${JSON.stringify(variables)}`) - existing = _.castArray(existing) - variables = _.castArray(variables) - 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('POST /repos/:org/:repo/actions/variables', { - org: this.repo.owner, - repo: this.repo.repo, - name: variable.name.toUpperCase(), - value: variable.value.toString() - }) - .then((res) => { - return res - }) - .catch((e) => { - 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('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 - } + update (existing, attrs) { + if (this.nop) { + return Promise.resolve([ + new NopCommand(this.constructor.name, this.repo, null, `Update variable ${attrs.name}`) + ]) } + return this.github.request('PATCH /repos/:org/:repo/actions/variables/:variable_name', { + org: this.repo.owner, + repo: this.repo.repo, + variable_name: attrs.name.toUpperCase(), + value: attrs.value.toString() + }) } - /** - * Add a new variable to a given repository - * - * @param {object} variable The variable to add, with name and value - * - * @see {@link https://docs.github.com/en/rest/actions/variables?apiVersion=2022-11-28#create-a-repository-variable} create a repository variable - * @returns - */ - async add (variable) { - this.log.debug(`Adding a repo var with the params ${JSON.stringify(variable)}`) - + add (attrs) { if (this.nop) { - return new NopCommand( - this.constructor.name, - this.repo, - null, - `Add variable ${variable.name}` - ) + return Promise.resolve([ + new NopCommand(this.constructor.name, this.repo, null, `Add variable ${attrs.name}`) + ]) } - - await this.github - .request('POST /repos/:org/:repo/actions/variables', { - org: this.repo.owner, - repo: this.repo.repo, - name: variable.name, - value: variable.value.toString() - }) - .then((res) => { - return res - }) - .catch((e) => { - this.logError(e) - }) + return this.github.request('POST /repos/:org/:repo/actions/variables', { + org: this.repo.owner, + repo: this.repo.repo, + name: attrs.name.toUpperCase(), + value: attrs.value.toString() + }) } - /** - * Remove variables that aren't defined as code - * - * @param {String} existing Name of the existing variable to remove - * - * @see {@link https://docs.github.com/en/rest/actions/variables?apiVersion=2022-11-28#delete-a-repository-variable} delete a repository variable - * @returns - */ - async remove (existing) { - this.log.debug(`Removing a repo var with the params ${JSON.stringify(existing)}`) - + remove (existing) { if (this.nop) { - return new NopCommand( - this.constructor.name, - this.repo, - null, - `Remove variable ${existing.name}` - ) + return Promise.resolve([ + 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, - repo: this.repo.repo, - variable_name: existing.name - }) - .then((res) => { - return res - }) - .catch((e) => { - this.logError(e) - }) + return this.github.request('DELETE /repos/:org/:repo/actions/variables/:variable_name', { + org: this.repo.owner, + repo: this.repo.repo, + variable_name: existing.name.toUpperCase() + }) } } diff --git a/test/unit/lib/plugins/variables.test.js b/test/unit/lib/plugins/variables.test.js index 41dc80e02..72ae6293f 100644 --- a/test/unit/lib/plugins/variables.test.js +++ b/test/unit/lib/plugins/variables.test.js @@ -7,14 +7,10 @@ describe('Variables', () => { const org = 'bkeepers' const repo = 'test' - function fillVariables (variables = []) { - return variables - } - - function configure (nop = false) { + function configure (nop = false, entries = [{ name: 'test', value: 'test' }]) { const log = { debug: jest.fn(), error: console.error } const errors = [] - return new Variables(nop, github, { owner: org, repo }, [{ name: 'test', value: 'test' }], log, errors) + return new Variables(nop, github, { owner: org, repo }, entries, log, errors) } beforeEach(() => { @@ -23,248 +19,186 @@ describe('Variables', () => { } }) - it('sync', () => { - const plugin = configure() - - when(github.request) - .calledWith('GET /repos/:org/:repo/actions/variables', { org, repo }) - .mockResolvedValue({ - data: { - variables: [ - fillVariables({ - variables: [] - }) - ] - } - }); - - ['variables'].forEach(() => { + describe('constructor', () => { + it('should uppercase entry names', () => { + const plugin = configure(false, [{ name: 'lower_case', value: 'val' }]) + expect(plugin.entries[0].name).toBe('LOWER_CASE') + }) + }) + + describe('find', () => { + it('should return only name and value fields', async () => { when(github.request) .calledWith('GET /repos/:org/:repo/actions/variables', { org, repo }) .mockResolvedValue({ data: { - variables: [{ name: 'DELETE_me', value: 'test' }] + variables: [{ name: 'VAR1', value: 'val1', created_at: '2024-01-01', updated_at: '2024-01-02' }] } }) - }) - when(github.request).calledWith('POST /repos/:org/:repo/actions/variables').mockResolvedValue({}) - - return plugin.sync().then(() => { - expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/actions/variables', { org, repo }); - - ['variables'].forEach(() => { - expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/actions/variables', { org, repo }) - }) - - expect(github.request).toHaveBeenCalledWith( - 'DELETE /repos/:org/:repo/actions/variables/:variable_name', - expect.objectContaining({ - org, - repo, - variable_name: 'DELETE_me' - }) - ) + const plugin = configure() + const result = await plugin.find() - expect(github.request).toHaveBeenCalledWith( - 'POST /repos/:org/:repo/actions/variables', - expect.objectContaining({ - org, - repo, - name: 'TEST', - value: 'test' - }) - ) + expect(result).toEqual([{ name: 'VAR1', value: 'val1' }]) }) }) - describe('noop mode', () => { - describe('sync', () => { - it('should return NopCommands and not make mutating API calls when nop is true', async () => { - const plugin = configure(true) + describe('changed', () => { + it('should return true when values differ', () => { + const plugin = configure() + expect(plugin.changed({ name: 'X', value: 'old' }, { name: 'X', value: 'new' })).toBe(true) + }) - when(github.request) - .calledWith('GET /repos/:org/:repo/actions/variables', { org, repo }) - .mockResolvedValue({ - data: { - variables: [{ name: 'EXISTING_VAR', value: 'existing-value' }] - } - }) + it('should return false when values match', () => { + const plugin = configure() + expect(plugin.changed({ name: 'X', value: 'same' }, { name: 'X', value: 'same' })).toBe(false) + }) + }) - const result = await plugin.sync() + describe('sync', () => { + it('should add new and remove stale variables', () => { + const plugin = configure() - // Should have made GET call to fetch existing variables - expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/actions/variables', { org, repo }) + when(github.request) + .calledWith('GET /repos/:org/:repo/actions/variables', { org, repo }) + .mockResolvedValue({ + data: { + variables: [{ name: 'DELETE_ME', value: 'test' }] + } + }) - // Should NOT have made any mutating calls (POST, PATCH, DELETE) - expect(github.request).not.toHaveBeenCalledWith( - expect.stringMatching(/^(POST|PATCH|DELETE)/), - expect.anything() + return plugin.sync().then(() => { + expect(github.request).toHaveBeenCalledWith( + 'DELETE /repos/:org/:repo/actions/variables/:variable_name', + expect.objectContaining({ org, repo, variable_name: 'DELETE_ME' }) ) - // 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() + expect(github.request).toHaveBeenCalledWith( + 'POST /repos/:org/:repo/actions/variables', + expect.objectContaining({ org, repo, name: 'TEST', value: 'test' }) ) - - // 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' } + it('should return NopCommands and not mutate when nop is true', async () => { + const plugin = configure(true) - const result = await plugin.add(variable) - - expect(result).toBeInstanceOf(NopCommand) - expect(result.plugin).toBe('Variables') - expect(github.request).not.toHaveBeenCalled() - }) + when(github.request) + .calledWith('GET /repos/:org/:repo/actions/variables', { org, repo }) + .mockResolvedValue({ + data: { + variables: [{ name: 'EXISTING_VAR', value: 'existing-value' }] + } + }) - it('should make API call when nop is false', async () => { - const plugin = configure(false) - const variable = { name: 'NEW_VAR', value: 'new-value' } + const result = await plugin.sync() - await plugin.add(variable) + expect(github.request).toHaveBeenCalledWith('GET /repos/:org/:repo/actions/variables', { org, repo }) + expect(github.request).not.toHaveBeenCalledWith( + expect.stringMatching(/^(POST|PATCH|DELETE)/), + expect.anything() + ) - expect(github.request).toHaveBeenCalledWith( - 'POST /repos/:org/:repo/actions/variables', - expect.objectContaining({ - org, - repo, - name: 'NEW_VAR', - value: 'new-value' - }) - ) - }) + expect(Array.isArray(result)).toBe(true) + expect(result.length).toBeGreaterThan(0) + // resArray contains: INFO NopCommand (flat), then [NopCommand] arrays from add/remove/update + const flat = result.flat() + flat.forEach(cmd => expect(cmd).toBeInstanceOf(NopCommand)) }) - 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) + it('should return NopCommand results when updating via sync', async () => { + const plugin = configure(true, [{ name: 'TEST', value: 'new-value' }]) - expect(result).toBeInstanceOf(NopCommand) - expect(result.plugin).toBe('Variables') - expect(github.request).not.toHaveBeenCalled() - }) + when(github.request) + .calledWith('GET /repos/:org/:repo/actions/variables', { org, repo }) + .mockResolvedValue({ + data: { + variables: [{ name: 'TEST', value: 'old-value' }] + } + }) - it('should make API call when nop is false', async () => { - const plugin = configure(false) - const existing = { name: 'EXISTING_VAR', value: 'existing-value' } + const result = await plugin.sync() - await plugin.remove(existing) + expect(github.request).not.toHaveBeenCalledWith( + expect.stringMatching(/^(POST|PATCH|DELETE)/), + expect.anything() + ) - expect(github.request).toHaveBeenCalledWith( - 'DELETE /repos/:org/:repo/actions/variables/:variable_name', - expect.objectContaining({ - org, - repo, - variable_name: 'EXISTING_VAR' - }) - ) - }) + expect(Array.isArray(result)).toBe(true) + const flat = result.flat() + flat.forEach(cmd => expect(cmd).toBeInstanceOf(NopCommand)) }) + }) - 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' }] + describe('add', () => { + it('should return NopCommand array when nop is true', async () => { + const plugin = configure(true) + const result = await plugin.add({ name: 'NEW_VAR', value: 'new-value' }) - const result = await plugin.update(existing, updated) + expect(Array.isArray(result)).toBe(true) + expect(result[0]).toBeInstanceOf(NopCommand) + expect(result[0].plugin).toBe('Variables') + expect(github.request).not.toHaveBeenCalled() + }) - expect(result).toBeInstanceOf(NopCommand) - expect(github.request).not.toHaveBeenCalled() - }) + it('should make POST request when nop is false', async () => { + const plugin = configure(false) + await plugin.add({ name: 'NEW_VAR', value: 'new-value' }) - 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 = [] + expect(github.request).toHaveBeenCalledWith( + 'POST /repos/:org/:repo/actions/variables', + expect.objectContaining({ org, repo, name: 'NEW_VAR', value: 'new-value' }) + ) + }) + }) - const result = await plugin.update(existing, updated) + describe('remove', () => { + it('should return NopCommand array when nop is true', async () => { + const plugin = configure(true) + const result = await plugin.remove({ name: 'EXISTING_VAR', value: 'existing-value' }) - expect(result).toBeInstanceOf(NopCommand) - expect(github.request).not.toHaveBeenCalled() - }) + expect(Array.isArray(result)).toBe(true) + expect(result[0]).toBeInstanceOf(NopCommand) + expect(result[0].plugin).toBe('Variables') + 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' }] + it('should make DELETE request when nop is false', async () => { + const plugin = configure(false) + await plugin.remove({ name: 'EXISTING_VAR', value: 'existing-value' }) - const result = await plugin.update(existing, updated) + expect(github.request).toHaveBeenCalledWith( + 'DELETE /repos/:org/:repo/actions/variables/:variable_name', + expect.objectContaining({ org, repo, variable_name: 'EXISTING_VAR' }) + ) + }) + }) - 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() - }) + describe('update', () => { + it('should return NopCommand array when nop is true', async () => { + const plugin = configure(true) + const result = await plugin.update( + { name: 'VAR1', value: 'old-value' }, + { name: 'VAR1', value: 'new-value' } + ) - 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' }] + expect(Array.isArray(result)).toBe(true) + expect(result[0]).toBeInstanceOf(NopCommand) + expect(result[0].plugin).toBe('Variables') + expect(github.request).not.toHaveBeenCalled() + }) - await plugin.update(existing, updated) + it('should make PATCH request when nop is false', async () => { + const plugin = configure(false) + await plugin.update( + { name: 'VAR1', value: 'old-value' }, + { name: 'VAR1', value: 'new-value' } + ) - expect(github.request).toHaveBeenCalledWith( - 'PATCH /repos/:org/:repo/actions/variables/:variable_name', - expect.objectContaining({ - org, - repo, - variable_name: 'VAR1', - value: 'new-value' - }) - ) - }) + expect(github.request).toHaveBeenCalledWith( + 'PATCH /repos/:org/:repo/actions/variables/:variable_name', + expect.objectContaining({ org, repo, variable_name: 'VAR1', value: 'new-value' }) + ) }) }) })