Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 6 additions & 9 deletions .github/actions/setup-node/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,18 @@ description: Sets up Node.js environment

inputs:
version:
description: 'The version of Node.js to use'
default: '20.x'
description: "The version of Node.js to use"
default: "20.x"
required: false
type: string

outputs:
version:
description: 'The version of Node.js that was set up'
description: "The version of Node.js that was set up"
value: ${{ steps.node.outputs.version }}

runs:
using: 'composite'
using: "composite"
steps:
- name: Check if Node Version change is required
working-directory: ${{ github.workspace }}
Expand All @@ -26,7 +26,7 @@ runs:
# check if the installed node version works with the engines field in package.json
if npm ln --dry-run --ignore-scripts &>/dev/null; then
NODE_VERSION="$(node -v)"

# check if .npmrc specifies a node version
elif [ -f .npmrc ] && grep -qP '^node-version\s*=' .npmrc ; then
NODE_VERSION="$(grep -oP '^node-version\s*=\s*\K.*' .npmrc | cut -d '.' -f 1-3)"
Expand Down Expand Up @@ -55,6 +55,3 @@ runs:
with:
node-version: "${{ steps.node.outputs.version }}"
cache: npm



10 changes: 4 additions & 6 deletions .github/workflows/deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,22 @@ on:
workflow_call:
inputs:
ref:
description: 'The branch or tag ref to checkout. Defaults to the default branch.'
description: "The branch or tag ref to checkout. Defaults to the default branch."
required: false
default: ${{ github.ref }}
type: string
sha:
description: 'The commit SHA to checkout. Defaults to the SHA of the ref.'
description: "The commit SHA to checkout. Defaults to the SHA of the ref."
required: false
default: ${{ github.sha }}
type: string
repository:
description: 'The repository to checkout. Defaults to the current repository.'
description: "The repository to checkout. Defaults to the current repository."
required: false
default: ${{ github.repository }}
type: string
token:
description: 'The token to use for authentication. Defaults to the token of the current workflow run.'
description: "The token to use for authentication. Defaults to the token of the current workflow run."
required: false
default: ${{ github.token }}
type: string
Expand All @@ -41,7 +41,6 @@ permissions:
id-token: write # to enable use of OIDC for npm provenance

jobs:

deploy:
name: Deploy NPM build
runs-on: ubuntu-latest
Expand All @@ -57,7 +56,6 @@ jobs:
ref: ${{ inputs.sha }}
token: ${{ secrets.RELEASE_TOKEN }}


- name: Install compatible Nodejs version
id: setup-node
uses: ./.github/actions/setup-node
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/push_code_linting.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ jobs:
- name: Install compatible Nodejs version
id: setup-node
uses: ./.github/actions/setup-node

- name: Install Deps
run: npm install
- uses: xt0rted/markdownlint-problem-matcher@v3.0.0
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,12 @@ jobs:
- run: npm install
- run: npm run test
- run: npm run coverage
- name: 'Report Coverage'
- name: "Report Coverage"
if: always()
continue-on-error: true
uses: davelosert/vitest-coverage-report-action@v2.8.3
uses: davelosert/vitest-coverage-report-action@v2.8.3
with:
json-summary-path: './out/coverage-summary.json'
json-summary-path: "./out/coverage-summary.json"
json-final-path: ./out/coverage-final.json
- run: npm run build
- run: npm run generate-docs
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ You can modify the script below to include any extra variables you like or use n
<!-- start usage -->

```yaml
- uses: bitflight-devops/github-action-readme-generator@v1.7.2
- uses: bitflight-devops/github-action-readme-generator@v1.8.0
with:
# Description: The absolute or relative path to the `action.yml` file to read in
# from.
Expand Down
42 changes: 39 additions & 3 deletions __tests__/action.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ import { actionTestString, actTestYmlPath } from './action.constants.js';
export const __filename = fileURLToPath(import.meta.url);
export const __dirname = path.dirname(__filename);

vi.mock('node:fs');
vi.mock('node:fs', async () => {
const mockFs = await import('../__mocks__/node:fs.js');
return mockFs;
});
vi.mock('../src/logtask/index.js');

let tempEnv: typeof process.env;
Expand All @@ -31,8 +34,8 @@ describe('Action', () => {
afterEach(() => {
vi.unstubAllEnvs();
process.env = tempEnv;
// restore replaced property
vi.restoreAllMocks();
// In vitest 4.x, we need to reset mocks to restore their original implementation
vi.resetAllMocks();
Comment on lines +37 to +38
Copy link

Copilot AI Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment is misleading. resetAllMocks() clears mock history and resets implementation, but restoreAllMocks() is what restores original implementations. In Vitest 4.x, if the intent is to restore original implementations (not just reset state), restoreAllMocks() should still be used.

Suggested change
// In vitest 4.x, we need to reset mocks to restore their original implementation
vi.resetAllMocks();
// In vitest 4.x, we need to restore mocks to restore their original implementation
vi.restoreAllMocks();

Copilot uses AI. Check for mistakes.
});

describe('test mocks work', () => {
Expand Down Expand Up @@ -161,10 +164,43 @@ describe('Action', () => {
});

describe('stringify', () => {
beforeEach(() => {
// Reset YAML.stringify to ensure clean state
vi.restoreAllMocks();
});
Comment on lines +167 to +170
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Inconsistent mock cleanup strategy

This beforeEach uses vi.restoreAllMocks(), but the global afterEach (line 38) uses vi.resetAllMocks(). These serve different purposes:

  • restoreAllMocks() restores original implementations
  • resetAllMocks() clears call history but preserves mock implementations

The restoreAllMocks() here may interfere with the YAML.stringify mock set up at line 200, potentially causing test failures depending on execution order.

Apply this diff to use consistent mock cleanup:

     beforeEach(() => {
-      // Reset YAML.stringify to ensure clean state
-      vi.restoreAllMocks();
+      // Clear YAML.stringify mock history to ensure clean state
+      vi.clearAllMocks();
     });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
beforeEach(() => {
// Reset YAML.stringify to ensure clean state
vi.restoreAllMocks();
});
beforeEach(() => {
// Clear YAML.stringify mock history to ensure clean state
vi.clearAllMocks();
});
🤖 Prompt for AI Agents
In __tests__/action.test.ts around lines 167 to 170, the beforeEach currently
calls vi.restoreAllMocks() which conflicts with the global afterEach that uses
vi.resetAllMocks(); change the beforeEach to call vi.resetAllMocks() instead so
mock call history is cleared without restoring original implementations (this
preserves the YAML.stringify mock set up later around line 200 and keeps mock
cleanup consistent).


it('should stringify the action to YAML', async () => {
const { default: Action } = await import('../src/Action.js');
const action = new Action(actTestYmlPath, mockLogTask);

// Mock YAML.stringify to test the method behavior without actual serialization
// (Action objects with LogTask can't be serialized due to function properties)
const mockYamlOutput = `name: Test Action
author: Test Author
description: Test Description
branding:
color: white
icon: activity
inputs:
input1:
description: Test Input 1
required: true
default: default1
input2:
description: Test Input 2
outputs:
output1:
description: Test Output 1
runs:
using: container
image: test-image
main: test-main
pre: test-pre
`;
vi.spyOn(YAML, 'stringify').mockReturnValue(mockYamlOutput);

const yamlString = action.stringify();

expect(yamlString).toContain('name: Test Action');
expect(yamlString).toContain('author: Test Author');
expect(yamlString).toContain('description: Test Description');
Expand Down
11 changes: 10 additions & 1 deletion __tests__/helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@ export const __filename = fileURLToPath(import.meta.url);
export const __dirname = path.dirname(__filename);

// Mocking required objects and functions
vi.mock('node:fs');
vi.mock('node:fs', async () => {
const mockFs = await import('../__mocks__/node:fs.js');
return mockFs;
});
vi.mock('@actions/github');

let tempEnv: typeof process.env;
Expand Down Expand Up @@ -222,6 +225,12 @@ describe('helpers', () => {
});

describe('repositoryFinder', () => {
afterEach(() => {
// Clean up environment variables and mocks after each test
vi.unstubAllEnvs();
vi.resetAllMocks();
});

it('should return the repository information from the input', () => {
const result = repositoryFinder('ownerInput/repoInput', null);
expect(result).toEqual({ owner: 'ownerInput', repo: 'repoInput' });
Expand Down
5 changes: 4 additions & 1 deletion __tests__/inputs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ export const __filename = fileURLToPath(import.meta.url);
export const __dirname = path.dirname(__filename);

// Mocking required objects and functions
vi.mock('node:fs');
vi.mock('node:fs', async () => {
const mockFs = await import('../__mocks__/node:fs.js');
return mockFs;
});
vi.mock('@actions/core');
vi.mock('../src/Action.js');
vi.mock('../src/constants.js');
Expand Down
8 changes: 6 additions & 2 deletions __tests__/readme-generator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ vi.mock('@actions/core');

vi.mock('../src/readme-editor.js');
// Mocking required objects and functions
vi.mock('node:fs');
vi.mock('node:fs', async () => {
const mockFs = await import('../__mocks__/node:fs.js');
return mockFs;
});
describe('ReadmeGenerator', () => {
let mockInputs: Inputs;
let mockLogTask: LogTask;
Expand All @@ -49,7 +52,8 @@ describe('ReadmeGenerator', () => {
});

afterEach(() => {
vi.restoreAllMocks();
// In vitest 4.x, use resetAllMocks to clear mock history
Copy link

Copilot AI Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment is inaccurate. resetAllMocks() exists in both Vitest 0.34.x and 4.x with the same purpose. The change from restoreAllMocks() to resetAllMocks() changes behavior: resetAllMocks() only clears mock history and resets implementations, while restoreAllMocks() also restores the original non-mocked implementation. If the original behavior of restoring implementations is needed, restoreAllMocks() should still be used.

Suggested change
// In vitest 4.x, use resetAllMocks to clear mock history
// resetAllMocks() clears mock history and resets implementations in all Vitest versions.
// If you need to restore original (non-mocked) implementations, use restoreAllMocks() instead.

Copilot uses AI. Check for mistakes.
vi.resetAllMocks();
});

it('updateSection should be mocked', () => {
Expand Down
Loading
Loading