Skip to content

Comments

Added unit tests, code coverage, and CI validation#24

Open
Jwaegebaert wants to merge 1 commit intopnp:mainfrom
Jwaegebaert:dev/tests
Open

Added unit tests, code coverage, and CI validation#24
Jwaegebaert wants to merge 1 commit intopnp:mainfrom
Jwaegebaert:dev/tests

Conversation

@Jwaegebaert
Copy link
Contributor

@Jwaegebaert Jwaegebaert commented Feb 5, 2026

  • 13 unit tests covering all code branches (success paths, error handling, edge cases)
  • 100% code coverage enforcement
  • GitHub Actions workflow that validates both tests on push/PR

You may ask why I went with Vitest instead of Mocha. Vitest includes built-in mocking, coverage, and native TypeScript support. Eliminating the need for separate packages (Sinon, c8, ts-node) that Mocha would've required. And the setup was quite easy 😄

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds comprehensive testing infrastructure to the CLI for Microsoft 365 GitHub Action, including 13 unit tests with Vitest, 100% code coverage enforcement, and automated CI validation through GitHub Actions. The changes ensure code quality and prevent regressions by validating all code paths including success scenarios, error handling, and edge cases.

Changes:

  • Added Vitest testing framework with v8 coverage provider and 100% coverage thresholds
  • Created 13 unit tests covering m365 CLI validation, script execution from file paths, inline script execution, file cleanup, and environment variable handling
  • Added GitHub Actions workflow to run tests and coverage on push/PR events

Reviewed changes

Copilot reviewed 4 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
vitest.config.mts Configures Vitest with Node environment, v8 coverage provider, and 100% coverage thresholds for all metrics
src/main.test.ts Implements 13 comprehensive unit tests using mocks for @actions/core, @actions/exec, @actions/io, and fs modules
package.json Adds test scripts (test, test:watch, test:coverage) and Vitest dependencies (vitest ^4.0.18, @vitest/coverage-v8 ^4.0.18)
package-lock.json Locks all new test-related dependencies including Vitest, Vite, and associated tooling
.github/workflows/tests.yml Creates CI workflow that runs on push/PR to validate tests and coverage, uploading coverage reports as artifacts
tsconfig.json Excludes test files (src/**/*.test.ts) from TypeScript compilation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +216 to +236
it('should use RUNNER_TEMP when set', async () => {
process.env.RUNNER_TEMP = '/custom/runner/temp';
vi.resetModules();

const inlineScript = 'm365 status';
mockCore.getInput.mockImplementation((name: string) => {
if (name === 'CLI_MICROSOFT365_SCRIPT_PATH') return '';
if (name === 'CLI_MICROSOFT365_SCRIPT') return inlineScript;
if (name === 'IS_POWERSHELL') return 'false';
return '';
});

await import('./main');

const writeCall = mockFs.writeFileSync.mock.calls[0];
expect(writeCall[0]).toMatch(/custom[\\\/]runner[\\\/]temp[\\\/]CLI_MICROSOFT365_GITHUB_ACTION_.*\.sh$/);
});

it('should fall back to tmpdir when RUNNER_TEMP is not set', async () => {
delete process.env.RUNNER_TEMP;
vi.resetModules();
Copy link

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

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

In these tests for RUNNER_TEMP environment variable, vi.resetModules() is called within the test (lines 218 and 236) but the mock modules are not re-imported after the reset. This means the mocks set up in beforeEach are cleared but not re-established, which could lead to the tests interacting with real modules instead of mocks. Consider re-importing and re-mocking the modules after vi.resetModules(), similar to what's done in beforeEach (lines 20-35).

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant