Skip to content

XS✔ ◾ Streamline unit test suite#785

Merged
muiriswoulfe merged 28 commits into
mainfrom
muiriswoulfe/test-improvements
Apr 27, 2026
Merged

XS✔ ◾ Streamline unit test suite#785
muiriswoulfe merged 28 commits into
mainfrom
muiriswoulfe/test-improvements

Conversation

@muiriswoulfe
Copy link
Copy Markdown
Member

Summary

The unit test suite is refactored to reduce maintenance cost without reducing coverage. The changes target long-standing pain points where refactors cascade into large test-edit diffs, and replace exhaustive sample-based checks with property-based coverage.

Changes

  • Test-only debug log assertions removed: verifications of internal tracing log calls that coupled tests to private method names have been dropped.
  • Localisation stubbing centralised: a shared helper replaces the per-spec beforeEach blocks that re-implemented the resource file as mock stubs.
  • Environment variable handling centralised: a scoped helper with automatic per-test restoration replaces hand-rolled process.env juggling.
  • Oversized spec files split: the four largest spec files are broken up by describe block or public method, reducing per-file line count and making test failures easier to triage.
  • SUT construction factored: each split family exposes a factory that collapses the repeated inline new X(instance(...), ...) boilerplate to a single line per test.
  • Property-based coverage expanded: additional fast-check properties back up the example-based tables for input parsing so invariants are verified over ranges rather than hand-picked values.
  • Invalid-input fixtures consolidated: common "invalid string" sets shared by multiple input specs are now exported from a single fixture module.

Impact

  • Refactors of private methods no longer cascade into large test-edit diffs.
  • Coverage is preserved at 100% across statements, branches, functions, and lines.
  • Test count is preserved; interior example-based cases remain in place as documentation alongside the new property tests.

Strips log-verification assertions that tested internal implementation details
(method-entry tracing, JSON response dumps, mid-flow checkpoints, cross-cutting
info-log enumerations, redundant log verifies) while preserving all tests of
observable behaviour.

- Removes 2,640 lines across 11 spec files (~15.7% reduction)
- All 783 tests still pass
- 100% coverage preserved (statements, branches, functions, lines)

Adds TestPlan.md documenting the broader unit test improvement strategy.
Adds tests/testUtilities/stubLocalization.ts that reads the real
resources.resjson file and wires runnerInvoker.loc() to return actual values
with util.format parameter substitution. Replaces hand-written loc() stubs in
seven spec files with a single helper call, removing ~370 lines of duplicated
resource mocks.

Side effects caught by the migration:
- azureReposInvoker.spec.ts had outdated stub text for the insufficient
  permissions message ('Code > Read' vs the real 'Code > Read & write'); five
  assertions updated to match the real resource.
- tokenManager.spec.ts "invalid scheme" test was passing accidentally because
  its manual stub args did not match the actual SUT call, leaving loc() to
  return null. The test now receives the real formatted error string and
  asserts it, matching the test's own stated intent.

inputs.spec.ts is not migrated in this pass; it embeds the mock return values
in verify(logger.logInfo(...)) assertions across dozens of tests, which
requires a separate per-test refactor.

- 783 tests still pass, 100% coverage preserved.
Adds tests/testUtilities/stubEnv.ts exporting a stubEnv helper that captures
the pre-test value of each environment variable and restores it via a global
afterEach hook. Replaces hand-written process.env.X = "value" assignments and
matching delete process.env.X cleanups across nine spec files.

Design notes:
- stubEnv takes variadic [name, value] tuples rather than an object literal so
  that POSIX-style uppercase variable names flow through without needing the
  camelCase object-key naming convention to be suppressed.
- Reflect.deleteProperty is used instead of delete to avoid the dynamic-delete
  rule trigger; no ESLint suppressions are required anywhere in the helper or
  its call sites.
- Pass undefined as a value to temporarily unset a variable; the original
  value is restored at afterEach time regardless.
- Replaces the existing after() cleanup blocks in tokenManager.spec.ts and
  azureReposInvoker.spec.ts, and the afterEach cleanup in gitInvoker.spec.ts
  and gitHubReposInvoker.spec.ts.

- 783 tests still pass, 100% coverage preserved, lint clean.
- Introduce a new skill to refresh pinned CI/CD dependencies.
- Enforce consistency of Node.js runtime across workflows and pipelines.
- Provide guidelines for when to use this skill and the process involved.
- Include verification steps and constraints to ensure proper updates.
Splits the four largest spec files into 28 smaller siblings to make
refactors cheaper and triage easier:

- inputs.spec.ts (1,053 lines) → 8 files per nested describe.
- azureReposInvoker.spec.ts (1,848 lines) → 7 files per public method.
- gitHubReposInvoker.spec.ts (1,513 lines) → 7 files per public method.
- codeMetrics.spec.ts (1,529 lines) → 6 files grouped by topic
  (defaults, nonDefaults, fileMatching, edgeCases, and the existing
  getFilesNotRequiringReview/getDeletedFilesNotRequiringReview describes).

Shared beforeEach setup is extracted into per-file *TestSetup.ts helpers
co-located with the splits. All test cases are preserved verbatim and
coverage remains at 100% across statements, branches, functions, and
lines.

Implements step 4 of TestPlan.md.
Adds a `createSut(...)` helper to each `*TestSetup.ts` file that constructs
the SUT from the mocked collaborators. Tests previously inlined the full
`new X(instance(a), instance(b), ...)` block at every call site — now each
test constructs the SUT on a single line:

  const sut = createSut(logger, runnerInvoker);

Removes ~500 lines of boilerplate across the 22 split spec files while
keeping test bodies otherwise unchanged. Unused `instance` imports pruned
where no longer referenced.

Implements step 6 of TestPlan.md. Step 5 (test data builders) was
evaluated and skipped — the targeted constructors all take 3–5 simple
positional args, where a builder would add verbosity without improving
readability.
Adds `fast-check` property tests for `baseSize`, `growthRate`, `testFactor`,
`fileMatchingPatterns`, and `testMatchingPatterns` alongside the existing
`codeFileExtensions` coverage in `inputs.property.spec.ts`. The new
properties cover the "any value satisfying X → converted result" and "any
value failing X → default" invariants that the example-based tests in the
per-input spec files can only sample pointwise.

Also generalises the local `createInputs` helper to take an
`InputOverrides` bag so each property can target a single input field
without interfering with the others.

Implements step 7 of TestPlan.md. The existing example-based tables stay
in place — deletion of interior cases is deferred per the plan's
prioritisation.
Extracts two shared fixture arrays into
`tests/testUtilities/fixtures/invalidInputs.ts`:

- `invalidNumericStrings` — strings that fail to parse as a positive
  numeric value, used by the base size, growth rate, and test factor
  specs. Growth rate and test factor spread the fixture and append
  `"Infinity"` locally.
- `invalidPatternStrings` — strings that reduce to empty after
  whitespace handling, used by the file matching pattern, test matching
  pattern, and code file extension specs.

Prevents the set of "invalid" strings from drifting between the six
input specs that shared the same values by copy-paste. Implements step 9
of TestPlan.md.
@muiriswoulfe
Copy link
Copy Markdown
Member Author

muiriswoulfe commented Apr 17, 2026

PR Metrics

Thanks for keeping your pull request small.
Thanks for adding tests.

Lines
Product Code 6
Test Code 7,588
Subtotal 7,594
Ignored Code 5
Total 7,599

Metrics computed by PR Metrics. Add it to your Azure DevOps and GitHub PRs!

@muiriswoulfe muiriswoulfe changed the title Streamline unit test suite XS✔ ◾ Streamline unit test suite Apr 17, 2026
@muiriswoulfe
Copy link
Copy Markdown
Member Author

Super-linter summary

Language Validation result
BIOME_LINT Fail ❌
CHECKOV Pass ✅
EDITORCONFIG Pass ✅
GITHUB_ACTIONS Pass ✅
GITHUB_ACTIONS_ZIZMOR Pass ✅
GITLEAKS Pass ✅
GIT_MERGE_CONFLICT_MARKERS Pass ✅
JSCPD Pass ✅
JSON_PRETTIER Pass ✅
MARKDOWN Pass ✅
MARKDOWN_PRETTIER Pass ✅
NATURAL_LANGUAGE Pass ✅
POWERSHELL Pass ✅
PRE_COMMIT Pass ✅
SPELL_CODESPELL Pass ✅
TRIVY Pass ✅
TYPESCRIPT_PRETTIER Pass ✅
XML Pass ✅
YAML Pass ✅
YAML_PRETTIER Pass ✅

Super-linter detected linting errors

For more information, see the GitHub Actions workflow run

Powered by Super-linter

BIOME_LINT
The number of diagnostics exceeds the limit allowed. Use --max-diagnostics to increase it.
Diagnostics not shown: 112.
Checked 130 files in 1279ms. No fixes applied.
Found 132 warnings.src/task/tests/metrics/codeMetrics.defaults.spec.ts:8:8 lint/style/useImportType  FIXABLE  ━━━━━━━━━━

  ! All these imports are only used as types.

     7 │ import { createCodeMetricsMocks, createSut } from "./codeMetricsTestSetup.js";
   > 8 │ import CodeMetrics from "../../src/metrics/codeMetrics.js";
       │        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     9 │ import CodeMetricsData from "../../src/metrics/codeMetricsData.js";
    10 │ import GitInvoker from "../../src/git/gitInvoker.js";

  i Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.

  i Safe fix: Use import type.

    8 │ import·type·CodeMetrics·from·"../../src/metrics/codeMetrics.js";
      │        +++++

src/task/tests/metrics/codeMetrics.defaults.spec.ts:10:8 lint/style/useImportType  FIXABLE  ━━━━━━━━━━

  ! All these imports are only used as types.

     8 │ import CodeMetrics from "../../src/metrics/codeMetrics.js";
     9 │ import CodeMetricsData from "../../src/metrics/codeMetricsData.js";
  > 10 │ import GitInvoker from "../../src/git/gitInvoker.js";
       │        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    11 │ import Inputs from "../../src/metrics/inputs.js";
    12 │ import Logger from "../../src/utilities/logger.js";

  i Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.

  i Safe fix: Use import type.

    10 │ import·type·GitInvoker·from·"../../src/git/gitInvoker.js";
       │        +++++

src/task/tests/metrics/codeMetrics.defaults.spec.ts:11:8 lint/style/useImportType  FIXABLE  ━━━━━━━━━━

  ! All these imports are only used as types.

     9 │ import CodeMetricsData from "../../src/metrics/codeMetricsData.js";
    10 │ import GitInvoker from "../../src/git/gitInvoker.js";
  > 11 │ import Inputs from "../../src/metrics/inputs.js";
       │        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    12 │ import Logger from "../../src/utilities/logger.js";
    13 │ import RunnerInvoker from "../../src/runners/runnerInvoker.js";

  i Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.

  i Safe fix: Use import type.

    11 │ import·type·Inputs·from·"../../src/metrics/inputs.js";
       │        +++++

src/task/tests/metrics/codeMetrics.defaults.spec.ts:12:8 lint/style/useImportType  FIXABLE  ━━━━━━━━━━

  ! All these imports are only used as types.

    10 │ import GitInvoker from "../../src/git/gitInvoker.js";
    11 │ import Inputs from "../../src/metrics/inputs.js";
  > 12 │ import Logger from "../../src/utilities/logger.js";
       │        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    13 │ import RunnerInvoker from "../../src/runners/runnerInvoker.js";
    14 │ import assert from "node:assert/strict";

  i Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.

  i Safe fix: Use import type.

    12 │ import·type·Logger·from·"../../src/utilities/logger.js";
       │        +++++

src/task/tests/metrics/codeMetrics.defaults.spec.ts:13:8 lint/style/useImportType  FIXABLE  ━━━━━━━━━━

  ! All these imports are only used as types.

    11 │ import Inputs from "../../src/metrics/inputs.js";
    12 │ import Logger from "../../src/utilities/logger.js";
  > 13 │ import RunnerInvoker from "../../src/runners/runnerInvoker.js";
       │        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    14 │ import assert from "node:assert/strict";
    15 │ import { when } from "ts-mockito";

  i Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.

  i Safe fix: Use import type.

    13 │ import·type·RunnerInvoker·from·"../../src/runners/runnerInvoker.js";
       │        +++++

src/task/tests/metrics/codeMetrics.edgeCases.spec.ts:9:8 lint/style/useImportType  FIXABLE  ━━━━━━━━━━

  ! All these imports are only used as types.

     7 │ import { anyString, when } from "ts-mockito";
     8 │ import { createCodeMetricsMocks, createSut } from "./codeMetricsTestSetup.js";
   > 9 │ import CodeMetrics from "../../src/metrics/codeMetrics.js";
       │        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    10 │ import CodeMetricsData from "../../src/metrics/codeMetricsData.js";
    11 │ import GitInvoker from "../../src/git/gitInvoker.js";

  i Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.

  i Safe fix: Use import type.

    9 │ import·type·CodeMetrics·from·"../../src/metrics/codeMetrics.js";
      │        +++++

src/task/tests/metrics/codeMetrics.edgeCases.spec.ts:11:8 lint/style/useImportType  FIXABLE  ━━━━━━━━━━

  ! All these imports are only used as types.

     9 │ import CodeMetrics from "../../src/metrics/codeMetrics.js";
    10 │ import CodeMetricsData from "../../src/metrics/codeMetricsData.js";
  > 11 │ import GitInvoker from "../../src/git/gitInvoker.js";
       │        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    12 │ import Inputs from "../../src/metrics/inputs.js";
    13 │ import Logger from "../../src/utilities/logger.js";

  i Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.

  i Safe fix: Use import type.

    11 │ import·type·GitInvoker·from·"../../src/git/gitInvoker.js";
       │        +++++

src/task/tests/metrics/codeMetrics.edgeCases.spec.ts:12:8 lint/style/useImportType  FIXABLE  ━━━━━━━━━━

  ! All these imports are only used as types.

    10 │ import CodeMetricsData from "../../src/metrics/codeMetricsData.js";
    11 │ import GitInvoker from "../../src/git/gitInvoker.js";
  > 12 │ import Inputs from "../../src/metrics/inputs.js";
       │        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    13 │ import Logger from "../../src/utilities/logger.js";
    14 │ import RunnerInvoker from "../../src/runners/runnerInvoker.js";

  i Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.

  i Safe fix: Use import type.

    12 │ import·type·Inputs·from·"../../src/metrics/inputs.js";
       │        +++++

src/task/tests/metrics/codeMetrics.edgeCases.spec.ts:13:8 lint/style/useImportType  FIXABLE  ━━━━━━━━━━

  ! All these imports are only used as types.

    11 │ import GitInvoker from "../../src/git/gitInvoker.js";
    12 │ import Inputs from "../../src/metrics/inputs.js";
  > 13 │ import Logger from "../../src/utilities/logger.js";
       │        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    14 │ import RunnerInvoker from "../../src/runners/runnerInvoker.js";
    15 │ import assert from "node:assert/strict";

  i Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.

  i Safe fix: Use import type.

    13 │ import·type·Logger·from·"../../src/utilities/logger.js";
       │        +++++

src/task/tests/metrics/codeMetrics.edgeCases.spec.ts:14:8 lint/style/useImportType  FIXABLE  ━━━━━━━━━━

  ! All these imports are only used as types.

    12 │ import Inputs from "../../src/metrics/inputs.js";
    13 │ import Logger from "../../src/utilities/logger.js";
  > 14 │ import RunnerInvoker from "../../src/runners/runnerInvoker.js";
       │        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    15 │ import assert from "node:assert/strict";
    16 │

  i Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.

  i Safe fix: Use import type.

    14 │ import·type·RunnerInvoker·from·"../../src/runners/runnerInvoker.js";
       │        +++++

src/task/tests/metrics/codeMetrics.getDeletedFilesNotRequiringReview.spec.ts:9:8 lint/style/useImportType  FIXABLE  ━━━━━━━━━━

  ! All these imports are only used as types.

     7 │ import * as AssertExtensions from "../testUtilities/assertExtensions.js";
     8 │ import { createCodeMetricsMocks, createSut } from "./codeMetricsTestSetup.js";
   > 9 │ import CodeMetrics from "../../src/metrics/codeMetrics.js";
       │        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    10 │ import GitInvoker from "../../src/git/gitInvoker.js";
    11 │ import Inputs from "../../src/metrics/inputs.js";

  i Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.

  i Safe fix: Use import type.

    9 │ import·type·CodeMetrics·from·"../../src/metrics/codeMetrics.js";
      │        +++++

src/task/tests/metrics/codeMetrics.getDeletedFilesNotRequiringReview.spec.ts:10:8 lint/style/useImportType  FIXABLE  ━━━━━━━━━━

  ! All these imports are only used as types.

     8 │ import { createCodeMetricsMocks, createSut } from "./codeMetricsTestSetup.js";
     9 │ import CodeMetrics from "../../src/metrics/codeMetrics.js";
  > 10 │ import GitInvoker from "../../src/git/gitInvoker.js";
       │        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    11 │ import Inputs from "../../src/metrics/inputs.js";
    12 │ import Logger from "../../src/utilities/logger.js";

  i Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.

  i Safe fix: Use import type.

    10 │ import·type·GitInvoker·from·"../../src/git/gitInvoker.js";
       │        +++++

src/task/tests/metrics/codeMetrics.getDeletedFilesNotRequiringReview.spec.ts:11:8 lint/style/useImportType  FIXABLE  ━━━━━━━━━━

  ! All these imports are only used as types.

     9 │ import CodeMetrics from "../../src/metrics/codeMetrics.js";
    10 │ import GitInvoker from "../../src/git/gitInvoker.js";
  > 11 │ import Inputs from "../../src/metrics/inputs.js";
       │        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    12 │ import Logger from "../../src/utilities/logger.js";
    13 │ import RunnerInvoker from "../../src/runners/runnerInvoker.js";

  i Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.

  i Safe fix: Use import type.

    11 │ import·type·Inputs·from·"../../src/metrics/inputs.js";
       │        +++++

src/task/tests/metrics/codeMetrics.getDeletedFilesNotRequiringReview.spec.ts:12:8 lint/style/useImportType  FIXABLE  ━━━━━━━━━━

  ! All these imports are only used as types.

    10 │ import GitInvoker from "../../src/git/gitInvoker.js";
    11 │ import Inputs from "../../src/metrics/inputs.js";
  > 12 │ import Logger from "../../src/utilities/logger.js";
       │        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    13 │ import RunnerInvoker from "../../src/runners/runnerInvoker.js";
    14 │ import assert from "node:assert/strict";

  i Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.

  i Safe fix: Use import type.

    12 │ import·type·Logger·from·"../../src/utilities/logger.js";
       │        +++++

src/task/tests/metrics/codeMetrics.getDeletedFilesNotRequiringReview.spec.ts:13:8 lint/style/useImportType  FIXABLE  ━━━━━━━━━━

  ! All these imports are only used as types.

    11 │ import Inputs from "../../src/metrics/inputs.js";
    12 │ import Logger from "../../src/utilities/logger.js";
  > 13 │ import RunnerInvoker from "../../src/runners/runnerInvoker.js";
       │        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    14 │ import assert from "node:assert/strict";
    15 │ import { when } from "ts-mockito";

  i Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.

  i Safe fix: Use import type.

    13 │ import·type·RunnerInvoker·from·"../../src/runners/runnerInvoker.js";
       │        +++++

src/task/tests/metrics/codeMetrics.getFilesNotRequiringReview.spec.ts:9:8 lint/style/useImportType  FIXABLE  ━━━━━━━━━━

  ! All these imports are only used as types.

     7 │ import * as AssertExtensions from "../testUtilities/assertExtensions.js";
     8 │ import { createCodeMetricsMocks, createSut } from "./codeMetricsTestSetup.js";
   > 9 │ import CodeMetrics from "../../src/metrics/codeMetrics.js";
       │        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    10 │ import GitInvoker from "../../src/git/gitInvoker.js";
    11 │ import Inputs from "../../src/metrics/inputs.js";

  i Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.

  i Safe fix: Use import type.

    9 │ import·type·CodeMetrics·from·"../../src/metrics/codeMetrics.js";
      │        +++++

src/task/tests/metrics/codeMetrics.getFilesNotRequiringReview.spec.ts:10:8 lint/style/useImportType  FIXABLE  ━━━━━━━━━━

  ! All these imports are only used as types.

     8 │ import { createCodeMetricsMocks, createSut } from "./codeMetricsTestSetup.js";
     9 │ import CodeMetrics from "../../src/metrics/codeMetrics.js";
  > 10 │ import GitInvoker from "../../src/git/gitInvoker.js";
       │        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    11 │ import Inputs from "../../src/metrics/inputs.js";
    12 │ import Logger from "../../src/utilities/logger.js";

  i Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.

  i Safe fix: Use import type.

    10 │ import·type·GitInvoker·from·"../../src/git/gitInvoker.js";
       │        +++++

src/task/tests/metrics/codeMetrics.getFilesNotRequiringReview.spec.ts:11:8 lint/style/useImportType  FIXABLE  ━━━━━━━━━━

  ! All these imports are only used as types.

     9 │ import CodeMetrics from "../../src/metrics/codeMetrics.js";
    10 │ import GitInvoker from "../../src/git/gitInvoker.js";
  > 11 │ import Inputs from "../../src/metrics/inputs.js";
       │        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    12 │ import Logger from "../../src/utilities/logger.js";
    13 │ import RunnerInvoker from "../../src/runners/runnerInvoker.js";

  i Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.

  i Safe fix: Use import type.

    11 │ import·type·Inputs·from·"../../src/metrics/inputs.js";
       │        +++++

src/task/tests/metrics/codeMetrics.getFilesNotRequiringReview.spec.ts:12:8 lint/style/useImportType  FIXABLE  ━━━━━━━━━━

  ! All these imports are only used as types.

    10 │ import GitInvoker from "../../src/git/gitInvoker.js";
    11 │ import Inputs from "../../src/metrics/inputs.js";
  > 12 │ import Logger from "../../src/utilities/logger.js";
       │        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    13 │ import RunnerInvoker from "../../src/runners/runnerInvoker.js";
    14 │ import assert from "node:assert/strict";

  i Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.

  i Safe fix: Use import type.

    12 │ import·type·Logger·from·"../../src/utilities/logger.js";
       │        +++++

src/task/tests/metrics/codeMetrics.getFilesNotRequiringReview.spec.ts:13:8 lint/style/useImportType  FIXABLE  ━━━━━━━━━━

  ! All these imports are only used as types.

    11 │ import Inputs from "../../src/metrics/inputs.js";
    12 │ import Logger from "../../src/utilities/logger.js";
  > 13 │ import RunnerInvoker from "../../src/runners/runnerInvoker.js";
       │        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    14 │ import assert from "node:assert/strict";
    15 │ import { when } from "ts-mockito";

  i Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.

  i Safe fix: Use import type.

    13 │ import·type·RunnerInvoker·from·"../../src/runners/runnerInvoker.js";
       │        +++++

lint ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  × Some warnings were emitted while running checks.

Copy link
Copy Markdown
Contributor

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

Refactors the PR Metrics TypeScript unit tests to reduce brittleness and maintenance cost by centralizing common test scaffolding (localization/env stubs, SUT factories) and expanding property-based testing for input parsing, while keeping coverage intact.

Changes:

  • Added shared test helpers for localization (stubLocalization) and environment variable scoping (stubEnv), plus shared invalid-input fixtures.
  • Split large specs into smaller, method-focused spec files with shared mock/SUT setup modules.
  • Expanded fast-check property-based tests for Inputs parsing invariants.

Reviewed changes

Copilot reviewed 50 out of 50 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/task/tests/utilities/validator.spec.ts Switches env var setup/cleanup to stubEnv for safer per-test restoration.
src/task/tests/testUtilities/stubLocalization.ts New helper that wires mocked RunnerInvoker.loc() to real resources.resjson.
src/task/tests/testUtilities/stubEnv.ts New helper to set/unset env vars with automatic restoration via afterEach.
src/task/tests/testUtilities/fixtures/invalidInputs.ts New shared fixtures for invalid numeric/pattern input strings.
src/task/tests/runners/runnerInvoker.spec.ts Replaces manual process.env juggling with stubEnv.
src/task/tests/repos/tokenManager.spec.ts Centralizes env setup, removes debug-log coupling, and uses real localization stubbing.
src/task/tests/repos/gitHubReposInvokerTestSetup.ts New shared mock/SUT factory for GitHub repos invoker tests (+ env + localization stubs).
src/task/tests/repos/gitHubReposInvoker.updateComment.spec.ts New focused spec covering updateComment().
src/task/tests/repos/gitHubReposInvoker.setTitleAndDescription.spec.ts New focused spec covering setTitleAndDescription().
src/task/tests/repos/gitHubReposInvoker.isAccessTokenAvailable.spec.ts New focused spec covering token availability behavior on GitHub/Azure contexts.
src/task/tests/repos/gitHubReposInvoker.getComments.spec.ts New focused spec for PR + file comment retrieval/mapping.
src/task/tests/repos/gitHubReposInvoker.deleteCommentThread.spec.ts New focused spec covering deletion of review comment threads.
src/task/tests/repos/gitHubReposInvoker.createComment.spec.ts New focused spec covering comment creation paths and error handling.
src/task/tests/repos/azureReposInvokerTestSetup.ts New shared mock/SUT factory for Azure repos invoker tests (+ env + localization stubs).
src/task/tests/repos/azureReposInvoker.updateComment.spec.ts New focused spec covering update thread/comment API behavior and access errors.
src/task/tests/repos/azureReposInvoker.setTitleAndDescription.spec.ts New focused spec covering PR title/description updates and access errors.
src/task/tests/repos/azureReposInvoker.isAccessTokenAvailable.spec.ts New focused spec covering token manager fallback and failure messaging.
src/task/tests/repos/azureReposInvoker.getTitleAndDescription.spec.ts New focused spec for reading PR title/description and env validation branches.
src/task/tests/repos/azureReposInvoker.getComments.spec.ts New focused spec for Azure thread/comment mapping and malformed payload handling.
src/task/tests/repos/azureReposInvoker.deleteCommentThread.spec.ts New focused spec covering delete operations and access errors.
src/task/tests/repos/azureReposInvoker.createComment.spec.ts New focused spec covering thread creation paths and access errors.
src/task/tests/pullRequests/pullRequestComments.spec.ts Uses stubLocalization and removes debug log assertions coupling tests to tracing.
src/task/tests/pullRequests/pullRequest.spec.ts Uses stubEnv/stubLocalization and removes debug log assertions.
src/task/tests/metrics/inputsTestSetup.ts New shared mocks/SUT factory for Inputs initialization specs.
src/task/tests/metrics/inputs.testMatchingPatterns.spec.ts New focused spec for testMatchingPatterns parsing using shared fixtures.
src/task/tests/metrics/inputs.testFactor.spec.ts New focused spec for testFactor parsing using shared fixtures.
src/task/tests/metrics/inputs.property.spec.ts Expands property-based coverage across more Inputs fields and invariants.
src/task/tests/metrics/inputs.growthRate.spec.ts New focused spec for growthRate parsing using shared fixtures.
src/task/tests/metrics/inputs.fileMatchingPatterns.spec.ts New focused spec for fileMatchingPatterns parsing and truncation behavior.
src/task/tests/metrics/inputs.codeFileExtensions.spec.ts New focused spec for codeFileExtensions parsing/normalization.
src/task/tests/metrics/inputs.baseSize.spec.ts New focused spec for baseSize parsing.
src/task/tests/metrics/inputs.alwaysCloseComment.spec.ts New focused spec for alwaysCloseComment parsing.
src/task/tests/metrics/inputs.allInputs.spec.ts New spec covering “all defaults” and “all specified” initialization behavior.
src/task/tests/metrics/codeMetricsTestSetup.ts New shared mocks/SUT factory for CodeMetrics tests (with localization stubbing).
src/task/tests/metrics/codeMetricsCalculator.spec.ts Uses stubEnv/stubLocalization and removes debug log assertions.
src/task/tests/metrics/codeMetrics.getFilesNotRequiringReview.spec.ts New focused spec for parsing/validation of diff summary and file extraction.
src/task/tests/metrics/codeMetrics.getDeletedFilesNotRequiringReview.spec.ts New focused spec for deleted-file extraction error paths.
src/task/tests/metrics/codeMetrics.fileMatching.spec.ts New focused spec for file include/exclude glob behavior and impacts on metrics.
src/task/tests/metrics/codeMetrics.edgeCases.spec.ts New focused spec for edge conditions (e.g., disabled test factor, large multipliers).
src/task/tests/metrics/codeMetrics.defaults.spec.ts New table-driven spec covering many default-input diff scenarios.
src/task/tests/git/octokitGitDiffParser.spec.ts Removes debug log assertions to reduce coupling to internal tracing.
.github/skills/update-ci-dependencies/SKILL.md Adds a repository “skill” doc describing how to refresh CI dependency pins consistently.

Comment thread src/task/tests/metrics/inputs.fileMatchingPatterns.spec.ts
Comment thread src/task/tests/metrics/inputs.codeFileExtensions.spec.ts Outdated
Adds the @typescript-eslint/consistent-type-imports rule and applies
its auto-fix across the source and spec files, so imports used only in
type positions use the dedicated `import type` syntax.
@muiriswoulfe
Copy link
Copy Markdown
Member Author

Super-linter summary

Language Validation result
BIOME_LINT Fail ❌
CHECKOV Pass ✅
EDITORCONFIG Pass ✅
GITHUB_ACTIONS Pass ✅
GITHUB_ACTIONS_ZIZMOR Pass ✅
GITLEAKS Pass ✅
GIT_MERGE_CONFLICT_MARKERS Pass ✅
JSCPD Pass ✅
JSON_PRETTIER Pass ✅
MARKDOWN Pass ✅
MARKDOWN_PRETTIER Pass ✅
NATURAL_LANGUAGE Pass ✅
POWERSHELL Pass ✅
PRE_COMMIT Pass ✅
SPELL_CODESPELL Pass ✅
TRIVY Pass ✅
TYPESCRIPT_PRETTIER Pass ✅
XML Pass ✅
YAML Pass ✅
YAML_PRETTIER Pass ✅

Super-linter detected linting errors

For more information, see the GitHub Actions workflow run

Powered by Super-linter

BIOME_LINT
Checked 130 files in 938ms. No fixes applied.
Found 1 warning.src/task/tests/repos/azureReposInvoker.setTitleAndDescription.spec.ts:14:8 lint/style/useImportType  FIXABLE  ━━━━━━━━━━

  ! All these imports are only used as types.

    12 │ import ErrorWithStatus from "../wrappers/errorWithStatus.js";
    13 │ import type GitInvoker from "../../src/git/gitInvoker.js";
  > 14 │ import { type GitPullRequest } from "azure-devops-node-api/interfaces/GitInterfaces.js";
       │        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    15 │ import type { IGitApi } from "azure-devops-node-api/GitApi.js";
    16 │ import type Logger from "../../src/utilities/logger.js";

  i Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.

  i Safe fix: Use import type.

     12  12 │   import ErrorWithStatus from "../wrappers/errorWithStatus.js";
     13  13 │   import type GitInvoker from "../../src/git/gitInvoker.js";
     14     │ - import·{·type·GitPullRequest·}·from·"azure-devops-node-api/interfaces/GitInterfaces.js";
         14 │ + import·type·{·GitPullRequest·}·from·"azure-devops-node-api/interfaces/GitInterfaces.js";
     15  15 │   import type { IGitApi } from "azure-devops-node-api/GitApi.js";
     16  16 │   import type Logger from "../../src/utilities/logger.js";


lint ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  × Some warnings were emitted while running checks.

Converts the inline `import { type GitPullRequest }` to
`import type { GitPullRequest }` for consistency with surrounding
imports. ESLint's consistent-type-imports rule accepts both forms, so
this was a manual cleanup.
Copilot AI review requested due to automatic review settings April 20, 2026 10:20
@muiriswoulfe
Copy link
Copy Markdown
Member Author

Super-linter summary

Language Validation result
BIOME_LINT Pass ✅
CHECKOV Pass ✅
EDITORCONFIG Pass ✅
GITHUB_ACTIONS Pass ✅
GITHUB_ACTIONS_ZIZMOR Pass ✅
GITLEAKS Pass ✅
GIT_MERGE_CONFLICT_MARKERS Pass ✅
JSCPD Pass ✅
JSON_PRETTIER Pass ✅
MARKDOWN Pass ✅
MARKDOWN_PRETTIER Pass ✅
NATURAL_LANGUAGE Pass ✅
POWERSHELL Pass ✅
PRE_COMMIT Pass ✅
SPELL_CODESPELL Pass ✅
TRIVY Pass ✅
TYPESCRIPT_PRETTIER Pass ✅
XML Pass ✅
YAML Pass ✅
YAML_PRETTIER Pass ✅

All files and directories linted successfully

For more information, see the GitHub Actions workflow run

Powered by Super-linter

@muiriswoulfe
Copy link
Copy Markdown
Member Author

Super-linter summary

Language Validation result
BIOME_LINT Pass ✅
CHECKOV Pass ✅
EDITORCONFIG Pass ✅
GITHUB_ACTIONS Pass ✅
GITHUB_ACTIONS_ZIZMOR Pass ✅
GITLEAKS Pass ✅
GIT_MERGE_CONFLICT_MARKERS Pass ✅
JSCPD Pass ✅
JSON_PRETTIER Pass ✅
MARKDOWN Pass ✅
MARKDOWN_PRETTIER Pass ✅
NATURAL_LANGUAGE Pass ✅
POWERSHELL Pass ✅
PRE_COMMIT Pass ✅
SPELL_CODESPELL Pass ✅
TRIVY Pass ✅
TYPESCRIPT_PRETTIER Pass ✅
XML Pass ✅
YAML Pass ✅
YAML_PRETTIER Pass ✅

All files and directories linted successfully

For more information, see the GitHub Actions workflow run

Powered by Super-linter

Copy link
Copy Markdown
Contributor

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

Copilot reviewed 52 out of 56 changed files in this pull request and generated 1 comment.

Comment thread src/task/tests/repos/gitHubReposInvoker.getComments.spec.ts Outdated
@microsoft microsoft deleted a comment from muiriswoulfe Apr 20, 2026
The `getIssueCommentsResponse` constant is a shared object. Assigning
`body` on `response.data[0]` previously mutated the constant itself,
leaving hidden coupling between tests in the same file (and any future
suite that touches the constant). `structuredClone` makes each test work
on an independent copy so mutations never escape.
@muiriswoulfe
Copy link
Copy Markdown
Member Author

Super-linter summary

Language Validation result
BIOME_LINT Pass ✅
CHECKOV Pass ✅
EDITORCONFIG Pass ✅
GITHUB_ACTIONS Pass ✅
GITHUB_ACTIONS_ZIZMOR Pass ✅
GITLEAKS Pass ✅
GIT_MERGE_CONFLICT_MARKERS Pass ✅
JSCPD Pass ✅
JSON_PRETTIER Pass ✅
MARKDOWN Pass ✅
MARKDOWN_PRETTIER Pass ✅
NATURAL_LANGUAGE Pass ✅
POWERSHELL Pass ✅
PRE_COMMIT Pass ✅
SPELL_CODESPELL Pass ✅
TRIVY Pass ✅
TYPESCRIPT_PRETTIER Pass ✅
XML Pass ✅
YAML Pass ✅
YAML_PRETTIER Pass ✅

All files and directories linted successfully

For more information, see the GitHub Actions workflow run

Powered by Super-linter

@microsoft microsoft deleted a comment from muiriswoulfe Apr 20, 2026
- Update test files to use localized strings for logging assertions.
- Refactor localization stubbing to utilize a new localize function.
- Enhance clarity and maintainability of test code by reducing duplication.
@muiriswoulfe
Copy link
Copy Markdown
Member Author

Super-linter summary

Language Validation result
BIOME_LINT Pass ✅
CHECKOV Pass ✅
EDITORCONFIG Pass ✅
GITHUB_ACTIONS Pass ✅
GITHUB_ACTIONS_ZIZMOR Pass ✅
GITLEAKS Pass ✅
GIT_MERGE_CONFLICT_MARKERS Pass ✅
JSCPD Pass ✅
JSON_PRETTIER Pass ✅
MARKDOWN Pass ✅
MARKDOWN_PRETTIER Pass ✅
NATURAL_LANGUAGE Pass ✅
POWERSHELL Pass ✅
PRE_COMMIT Pass ✅
SPELL_CODESPELL Pass ✅
TRIVY Pass ✅
TYPESCRIPT_PRETTIER Pass ✅
XML Pass ✅
YAML Pass ✅
YAML_PRETTIER Pass ✅

All files and directories linted successfully

For more information, see the GitHub Actions workflow run

Powered by Super-linter

Copy link
Copy Markdown
Contributor

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

Copilot reviewed 53 out of 57 changed files in this pull request and generated 4 comments.

Comment thread src/task/tests/pullRequestMetrics.spec.ts
Comment thread src/task/tests/repos/tokenManager.spec.ts
- Add cleanup step for debug and release directories before copying source files.
- Ensure test:fast command also cleans up debug directory before execution.
@muiriswoulfe
Copy link
Copy Markdown
Member Author

Super-linter summary

Language Validation result
BIOME_LINT Pass ✅
CHECKOV Pass ✅
EDITORCONFIG Pass ✅
GITHUB_ACTIONS Pass ✅
GITHUB_ACTIONS_ZIZMOR Pass ✅
GITLEAKS Pass ✅
GIT_MERGE_CONFLICT_MARKERS Pass ✅
JSCPD Pass ✅
JSON_PRETTIER Pass ✅
MARKDOWN Pass ✅
MARKDOWN_PRETTIER Pass ✅
NATURAL_LANGUAGE Pass ✅
POWERSHELL Pass ✅
PRE_COMMIT Pass ✅
SPELL_CODESPELL Pass ✅
TRIVY Pass ✅
TYPESCRIPT_PRETTIER Pass ✅
XML Pass ✅
YAML Pass ✅
YAML_PRETTIER Pass ✅

All files and directories linted successfully

For more information, see the GitHub Actions workflow run

Powered by Super-linter

Replace hard-coded English strings in test assertions with localize()
lookups against resources.resjson. Covers pullRequestMetrics.succeeded,
repos.azureReposInvoker.noAzureReposAccessToken,
repos.gitHubReposInvoker.noGitHubAccessToken, and
repos.tokenManager.incorrectAuthorizationScheme.
@muiriswoulfe
Copy link
Copy Markdown
Member Author

Super-linter summary

Language Validation result
BIOME_LINT Pass ✅
CHECKOV Pass ✅
EDITORCONFIG Pass ✅
GITHUB_ACTIONS Pass ✅
GITHUB_ACTIONS_ZIZMOR Pass ✅
GITLEAKS Pass ✅
GIT_MERGE_CONFLICT_MARKERS Pass ✅
JSCPD Pass ✅
JSON_PRETTIER Pass ✅
MARKDOWN Pass ✅
MARKDOWN_PRETTIER Pass ✅
NATURAL_LANGUAGE Pass ✅
POWERSHELL Pass ✅
PRE_COMMIT Pass ✅
SPELL_CODESPELL Pass ✅
TRIVY Pass ✅
TYPESCRIPT_PRETTIER Pass ✅
XML Pass ✅
YAML Pass ✅
YAML_PRETTIER Pass ✅

All files and directories linted successfully

For more information, see the GitHub Actions workflow run

Powered by Super-linter

Copy link
Copy Markdown
Contributor

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

Copilot reviewed 54 out of 58 changed files in this pull request and generated 2 comments.

Comment thread src/task/tests/repos/gitHubReposInvoker.createComment.spec.ts Outdated
@muiriswoulfe
Copy link
Copy Markdown
Member Author

Super-linter summary

Language Validation result
BIOME_LINT Pass ✅
CHECKOV Pass ✅
EDITORCONFIG Pass ✅
GITHUB_ACTIONS Pass ✅
GITHUB_ACTIONS_ZIZMOR Pass ✅
GITLEAKS Pass ✅
GIT_MERGE_CONFLICT_MARKERS Pass ✅
JSCPD Pass ✅
JSON_PRETTIER Pass ✅
MARKDOWN Pass ✅
MARKDOWN_PRETTIER Pass ✅
NATURAL_LANGUAGE Pass ✅
POWERSHELL Pass ✅
PRE_COMMIT Pass ✅
SPELL_CODESPELL Pass ✅
TRIVY Pass ✅
TYPESCRIPT_PRETTIER Pass ✅
XML Pass ✅
YAML Pass ✅
YAML_PRETTIER Pass ✅

All files and directories linted successfully

For more information, see the GitHub Actions workflow run

Powered by Super-linter

The test stubs thenResolve(null), so rename the case from
"returns undefined" to "returns null" to match the actual behaviour.
@muiriswoulfe
Copy link
Copy Markdown
Member Author

Super-linter summary

Language Validation result
BIOME_LINT Pass ✅
CHECKOV Pass ✅
EDITORCONFIG Pass ✅
GITHUB_ACTIONS Pass ✅
GITHUB_ACTIONS_ZIZMOR Pass ✅
GITLEAKS Pass ✅
GIT_MERGE_CONFLICT_MARKERS Pass ✅
JSCPD Pass ✅
JSON_PRETTIER Pass ✅
MARKDOWN Pass ✅
MARKDOWN_PRETTIER Pass ✅
NATURAL_LANGUAGE Pass ✅
POWERSHELL Pass ✅
PRE_COMMIT Pass ✅
SPELL_CODESPELL Pass ✅
TRIVY Pass ✅
TYPESCRIPT_PRETTIER Pass ✅
XML Pass ✅
YAML Pass ✅
YAML_PRETTIER Pass ✅

All files and directories linted successfully

For more information, see the GitHub Actions workflow run

Powered by Super-linter

Reconcile main's userAgent relocation to constants.ts with our local
test-split refactor: drop the local export from gitHubReposInvoker.ts,
re-source userAgent in gitHubReposInvokerTestSetup.ts, and update
Update-Version.ps1 to track the new path. Port the dotfile and
non-parseable URL coverage from main's monolithic specs into the
matching split files. Regenerate dist/index.mjs.
@muiriswoulfe
Copy link
Copy Markdown
Member Author

Super-linter summary

Language Validation result
CHECKOV Pass ✅
EDITORCONFIG Pass ✅
GITHUB_ACTIONS Pass ✅
GITHUB_ACTIONS_ZIZMOR Pass ✅
GITLEAKS Pass ✅
GIT_COMMITLINT Fail ❌
GIT_MERGE_CONFLICT_MARKERS Pass ✅
JSCPD Pass ✅
JSON_PRETTIER Pass ✅
MARKDOWN Pass ✅
MARKDOWN_PRETTIER Pass ✅
NATURAL_LANGUAGE Pass ✅
POWERSHELL Pass ✅
PRE_COMMIT Pass ✅
SPELL_CODESPELL Pass ✅
TRIVY Pass ✅
TYPESCRIPT_PRETTIER Pass ✅
XML Pass ✅
YAML Pass ✅
YAML_PRETTIER Pass ✅

Super-linter detected linting errors

For more information, see the GitHub Actions workflow run

Powered by Super-linter

GIT_COMMITLINT
�[90m⧗�[39m   input: �[1mMerge branch 'main' into muiriswoulfe/test-improvements

Reconcile main's userAgent relocation to constants.ts with our local
test-split refactor: drop the local export from gitHubReposInvoker.ts,
re-source userAgent in gitHubReposInvokerTestSetup.ts, and update
Update-Version.ps1 to track the new path. Port the dotfile and
non-parseable URL coverage from main's monolithic specs into the
matching split files. Regenerate dist/index.mjs.

�[22m
�[1m�[32m✔�[39m   found 0 problems, 0 warnings�[22m
�[90m⧗�[39m   input: �[1mtest: rename createReviewComment test to match its stub

The test stubs thenResolve(null), so rename the case from
"returns undefined" to "returns null" to match the actual behaviour.�[22m
�[1m�[32m✔�[39m   found 0 problems, 0 warnings�[22m
�[90m⧗�[39m   input: �[1mchore: fix linting�[22m
�[1m�[32m✔�[39m   found 0 problems, 0 warnings�[22m
�[90m⧗�[39m   input: �[1mrefactor(tests): derive hard-coded error strings via localize()

Replace hard-coded English strings in test assertions with localize()
lookups against resources.resjson. Covers pullRequestMetrics.succeeded,
repos.azureReposInvoker.noAzureReposAccessToken,
repos.gitHubReposInvoker.noGitHubAccessToken, and
repos.tokenManager.incorrectAuthorizationScheme.�[22m
�[1m�[32m✔�[39m   found 0 problems, 0 warnings�[22m
�[90m⧗�[39m   input: �[1mrefactor(scripts): enhance build initialization commands

- Add cleanup step for debug and release directories before copying source files.
- Ensure test:fast command also cleans up debug directory before execution.�[22m
�[1m�[32m✔�[39m   found 0 problems, 0 warnings�[22m
�[90m⧗�[39m   input: �[1mrefactor(tests): improve localization handling in tests

- Update test files to use localized strings for logging assertions.
- Refactor localization stubbing to utilize a new localize function.
- Enhance clarity and maintainability of test code by reducing duplication.�[22m
�[1m�[32m✔�[39m   found 0 problems, 0 warnings�[22m
�[90m⧗�[39m   input: �[1mClone getIssueCommentsResponse before mutating in tests

The `getIssueCommentsResponse` constant is a shared object. Assigning
`body` on `response.data[0]` previously mutated the constant itself,
leaving hidden coupling between tests in the same file (and any future
suite that touches the constant). `structuredClone` makes each test work
on an independent copy so mutations never escape.�[22m
�[31m✖�[39m   subject may not be empty �[90m[subject-empty]�[39m
�[31m✖�[39m   type may not be empty �[90m[type-empty]�[39m

�[1m�[31m✖�[39m   found 2 problems, 0 warnings�[22m
ⓘ   Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint

�[90m⧗�[39m   input: �[1mchore: fix linting, update dist folder�[22m
�[1m�[32m✔�[39m   found 0 problems, 0 warnings�[22m
�[90m⧗�[39m   input: �[1mShare user agent constant between production and tests

- Export the user-agent string as a named constant from gitHubReposInvoker.ts
  so tests can reference the production value instead of hard-coding their
  own copy that can drift on version bumps.
- Update the version-bump script to stop touching the deleted
  gitHubReposInvoker.spec.ts and rely on the test setup re-exporting the
  production constant.�[22m
�[31m✖�[39m   subject may not be empty �[90m[subject-empty]�[39m
�[31m✖�[39m   type may not be empty �[90m[type-empty]�[39m

�[1m�[31m✖�[39m   found 2 problems, 0 warnings�[22m
ⓘ   Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint

�[90m⧗�[39m   input: �[1mchore: fix linting�[22m
�[1m�[32m✔�[39m   found 0 problems, 0 warnings�[22m
�[90m⧗�[39m   input: �[1mAddress Copilot review feedback

- Stub PR_METRICS_ACCESS_TOKEN in tokenManager spec so the production-code
  assignment cannot leak into subsequent tests.
- Assert octokitWrapper.initialize/updateIssueComment are never called when
  updateComment is invoked with null content.
- Assert octokitWrapper.initialize/updatePull are never called when
  setTitleAndDescription is invoked with null title and description.�[22m
�[31m✖�[39m   subject may not be empty �[90m[subject-empty]�[39m
�[31m✖�[39m   type may not be empty �[90m[type-empty]�[39m

�[1m�[31m✖�[39m   found 2 problems, 0 warnings�[22m
ⓘ   Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint

�[90m⧗�[39m   input: �[1mchore: remove update-ci-dependencies skill

This skill does not belong on this branch.�[22m
�[1m�[32m✔�[39m   found 0 problems, 0 warnings�[22m
�[90m⧗�[39m   input: �[1mStub the correct env var in isPullRequest negative test

The test claims to cover 'SYSTEM_PULLREQUEST_PULLREQUESTID is not
defined' but was unsetting SYSTEM_PULLREQUEST_TARGETBRANCH, which the
production isPullRequest getter never reads on the Azure Pipelines
branch. Stub the variable the code actually checks so the assertion
exercises the intended condition.�[22m
�[31m✖�[39m   subject may not be empty �[90m[subject-empty]�[39m
�[31m✖�[39m   type may not be empty �[90m[type-empty]�[39m

�[1m�[31m✖�[39m   found 2 problems, 0 warnings�[22m
ⓘ   Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint

�[90m⧗�[39m   input: �[1mAddress PR review feedback on Inputs specs

- inputs.fileMatchingPatterns: replace the hard-coded 200/250 literals
  in the truncation test with references to the maxPatternCount
  constant and its toLocaleString() form, so the test stays in sync
  with production if the limit is ever changed.
- inputs.codeFileExtensions: remove a second copy of the lowercase
  conversion test that was identical to the earlier one.�[22m
�[31m✖�[39m   subject may not be empty �[90m[subject-empty]�[39m
�[31m✖�[39m   type may not be empty �[90m[type-empty]�[39m

�[1m�[31m✖�[39m   found 2 problems, 0 warnings�[22m
ⓘ   Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint

�[90m⧗�[39m   input: �[1mClear managed env vars before the test suite starts

The stubEnv helper captures an env variable's pre-test value and
restores it in afterEach. On a developer workstation the pre-test value
is typically unset, so the restore-to-original semantics match the
old pattern of an explicit delete after each test.

On GitHub Actions, however, the runner pre-populates GITHUB_ACTION,
GITHUB_REF, GITHUB_BASE_REF, and related variables. When a test stubs
one of these for a GitHub-runner scenario and afterEach runs, the
helper restores the variable to the Actions-set value rather than
clearing it, leaving later tests that rely on the variable being unset
to misbehave.

Adds a single before() hook that unsets every test-managed environment
variable once at suite startup. With the host-inherited values cleared,
the restore-to-original logic now restores to "unset" for subsequent
tests, matching the behavior the old delete-after-test code provided.�[22m
�[31m✖�[39m   subject may not be empty �[90m[subject-empty]�[39m
�[31m✖�[39m   type may not be empty �[90m[type-empty]�[39m

�[1m�[31m✖�[39m   found 2 problems, 0 warnings�[22m
ⓘ   Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint

�[90m⧗�[39m   input: �[1mchore: fix linting�[22m
�[1m�[32m✔�[39m   found 0 problems, 0 warnings�[22m
�[90m⧗�[39m   input: �[1mUse separate type import for GitPullRequest

Converts the inline `import { type GitPullRequest }` to
`import type { GitPullRequest }` for consistency with surrounding
imports. ESLint's consistent-type-imports rule accepts both forms, so
this was a manual cleanup.�[22m
�[31m✖�[39m   subject may not be empty �[90m[subject-empty]�[39m
�[31m✖�[39m   type may not be empty �[90m[type-empty]�[39m

�[1m�[31m✖�[39m   found 2 problems, 0 warnings�[22m
ⓘ   Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint

�[90m⧗�[39m   input: �[1mEnforce consistent-type-imports via ESLint

Adds the @typescript-eslint/consistent-type-imports rule and applies
its auto-fix across the source and spec files, so imports used only in
type positions use the dedicated `import type` syntax.�[22m
�[31m✖�[39m   subject may not be empty �[90m[subject-empty]�[39m
�[31m✖�[39m   type may not be empty �[90m[type-empty]�[39m

�[1m�[31m✖�[39m   found 2 problems, 0 warnings�[22m
ⓘ   Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint

�[90m⧗�[39m   input: �[1mRemoving plan�[22m
�[31m✖�[39m   subject may not be empty �[90m[subject-empty]�[39m
�[31m✖�[39m   type may not be empty �[90m[type-empty]�[39m

�[1m�[31m✖�[39m   found 2 problems, 0 warnings�[22m
ⓘ   Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint

�[90m⧗�[39m   input: �[1mConsolidate invalid-input fixtures shared across Inputs specs

Extracts two shared fixture arrays into
`tests/testUtilities/fixtures/invalidInputs.ts`:

- `invalidNumericStrings` — strings that fail to parse as a positive
  numeric value, used by the base size, growth rate, and test factor
  specs. Growth rate and test factor spread the fixture and append
  `"Infinity"` locally.
- `invalidPatternStrings` — strings that reduce to empty after
  whitespace handling, used by the file matching pattern, test matching
  pattern, and code file extension specs.

Prevents the set of "invalid" strings from drifting between the six
input specs that shared the same values by copy-paste. Implements step 9
of TestPlan.md.�[22m
�[31m✖�[39m   subject may not be empty �[90m[subject-empty]�[39m
�[31m✖�[39m   type may not be empty �[90m[type-empty]�[39m

�[1m�[31m✖�[39m   found 2 problems, 0 warnings�[22m
ⓘ   Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint

�[90m⧗�[39m   input: �[1mExpand property-based coverage for Inputs parsing

Adds `fast-check` property tests for `baseSize`, `growthRate`, `testFactor`,
`fileMatchingPatterns`, and `testMatchingPatterns` alongside the existing
`codeFileExtensions` coverage in `inputs.property.spec.ts`. The new
properties cover the "any value satisfying X → converted result" and "any
value failing X → default" invariants that the example-based tests in the
per-input spec files can only sample pointwise.

Also generalises the local `createInputs` helper to take an
`InputOverrides` bag so each property can target a single input field
without interfering with the others.

Implements step 7 of TestPlan.md. The existing example-based tables stay
in place — deletion of interior cases is deferred per the plan's
prioritisation.�[22m
�[31m✖�[39m   subject may not be empty �[90m[subject-empty]�[39m
�[31m✖�[39m   type may not be empty �[90m[type-empty]�[39m

�[1m�[31m✖�[39m   found 2 problems, 0 warnings�[22m
ⓘ   Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint

�[90m⧗�[39m   input: �[1mExtract createSut factory per spec family

Adds a `createSut(...)` helper to each `*TestSetup.ts` file that constructs
the SUT from the mocked collaborators. Tests previously inlined the full
`new X(instance(a), instance(b), ...)` block at every call site — now each
test constructs the SUT on a single line:

  const sut = createSut(logger, runnerInvoker);

Removes ~500 lines of boilerplate across the 22 split spec files while
keeping test bodies otherwise unchanged. Unused `instance` imports pruned
where no longer referenced.

Implements step 6 of TestPlan.md. Step 5 (test data builders) was
evaluated and skipped — the targeted constructors all take 3–5 simple
positional args, where a builder would add verbosity without improving
readability.�[22m
�[31m✖�[39m   subject may not be empty �[90m[subject-empty]�[39m
�[31m✖�[39m   type may not be empty �[90m[type-empty]�[39m

�[1m�[31m✖�[39m   found 2 problems, 0 warnings�[22m
ⓘ   Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint

�[90m⧗�[39m   input: �[1mSplit oversized spec files by topic and per public method

Splits the four largest spec files into 28 smaller siblings to make
refactors cheaper and triage easier:

- inputs.spec.ts (1,053 lines) → 8 files per nested describe.
- azureReposInvoker.spec.ts (1,848 lines) → 7 files per public method.
- gitHubReposInvoker.spec.ts (1,513 lines) → 7 files per public method.
- codeMetrics.spec.ts (1,529 lines) → 6 files grouped by topic
  (defaults, nonDefaults, fileMatching, edgeCases, and the existing
  getFilesNotRequiringReview/getDeletedFilesNotRequiringReview describes).

Shared beforeEach setup is extracted into per-file *TestSetup.ts helpers
co-located with the splits. All test cases are preserved verbatim and
coverage remains at 100% across statements, branches, functions, and
lines.

Implements step 4 of TestPlan.md.�[22m
�[31m✖�[39m   subject may not be empty �[90m[subject-empty]�[39m
�[31m✖�[39m   type may not be empty �[90m[type-empty]�[39m

�[1m�[31m✖�[39m   found 2 problems, 0 warnings�[22m
ⓘ   Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint

�[90m⧗�[39m   input: �[1mchore(ci): add skill for updating CI dependencies

- Introduce a new skill to refresh pinned CI/CD dependencies.
- Enforce consistency of Node.js runtime across workflows and pipelines.
- Provide guidelines for when to use this skill and the process involved.
- Include verification steps and constraints to ensure proper updates.�[22m
�[1m�[32m✔�[39m   found 0 problems, 0 warnings�[22m
�[90m⧗�[39m   input: �[1mReplace process.env juggling with stubEnv helper

Adds tests/testUtilities/stubEnv.ts exporting a stubEnv helper that captures
the pre-test value of each environment variable and restores it via a global
afterEach hook. Replaces hand-written process.env.X = "value" assignments and
matching delete process.env.X cleanups across nine spec files.

Design notes:
- stubEnv takes variadic [name, value] tuples rather than an object literal so
  that POSIX-style uppercase variable names flow through without needing the
  camelCase object-key naming convention to be suppressed.
- Reflect.deleteProperty is used instead of delete to avoid the dynamic-delete
  rule trigger; no ESLint suppressions are required anywhere in the helper or
  its call sites.
- Pass undefined as a value to temporarily unset a variable; the original
  value is restored at afterEach time regardless.
- Replaces the existing after() cleanup blocks in tokenManager.spec.ts and
  azureReposInvoker.spec.ts, and the afterEach cleanup in gitInvoker.spec.ts
  and gitHubReposInvoker.spec.ts.

- 783 tests still pass, 100% coverage preserved, lint clean.�[22m
�[31m✖�[39m   subject may not be empty �[90m[subject-empty]�[39m
�[31m✖�[39m   type may not be empty �[90m[type-empty]�[39m

�[1m�[31m✖�[39m   found 2 problems, 0 warnings�[22m
ⓘ   Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint

�[90m⧗�[39m   input: �[1mReplace localisation mock blocks with stubLocalization helper

Adds tests/testUtilities/stubLocalization.ts that reads the real
resources.resjson file and wires runnerInvoker.loc() to return actual values
with util.format parameter substitution. Replaces hand-written loc() stubs in
seven spec files with a single helper call, removing ~370 lines of duplicated
resource mocks.

Side effects caught by the migration:
- azureReposInvoker.spec.ts had outdated stub text for the insufficient
  permissions message ('Code > Read' vs the real 'Code > Read & write'); five
  assertions updated to match the real resource.
- tokenManager.spec.ts "invalid scheme" test was passing accidentally because
  its manual stub args did not match the actual SUT call, leaving loc() to
  return null. The test now receives the real formatted error string and
  asserts it, matching the test's own stated intent.

inputs.spec.ts is not migrated in this pass; it embeds the mock return values
in verify(logger.logInfo(...)) assertions across dozens of tests, which
requires a separate per-test refactor.

- 783 tests still pass, 100% coverage preserved.�[22m
�[31m✖�[39m   subject may not be empty �[90m[subject-empty]�[39m
�[31m✖�[39m   type may not be empty �[90m[type-empty]�[39m

�[1m�[31m✖�[39m   found 2 problems, 0 warnings�[22m
ⓘ   Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint

�[90m⧗�[39m   input: �[1mRemove implementation-testing log verifications from unit tests

Strips log-verification assertions that tested internal implementation details
(method-entry tracing, JSON response dumps, mid-flow checkpoints, cross-cutting
info-log enumerations, redundant log verifies) while preserving all tests of
observable behaviour.

- Removes 2,640 lines across 11 spec files (~15.7% reduction)
- All 783 tests still pass
- 100% coverage preserved (statements, branches, functions, lines)

Adds TestPlan.md documenting the broader unit test improvement strategy.�[22m
�[31m✖�[39m   subject may not be empty �[90m[subject-empty]�[39m
�[31m✖�[39m   type may not be empty �[90m[type-empty]�[39m

�[1m�[31m✖�[39m   found 2 problems, 0 warnings�[22m
ⓘ   Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint

Copy link
Copy Markdown
Contributor

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

Copilot reviewed 52 out of 55 changed files in this pull request and generated no new comments.

@muiriswoulfe
Copy link
Copy Markdown
Member Author

Super-linter summary

Language Validation result
CHECKOV Pass ✅
EDITORCONFIG Pass ✅
GITHUB_ACTIONS Pass ✅
GITHUB_ACTIONS_ZIZMOR Pass ✅
GITLEAKS Pass ✅
GIT_MERGE_CONFLICT_MARKERS Pass ✅
JSCPD Pass ✅
JSON_PRETTIER Pass ✅
MARKDOWN Pass ✅
MARKDOWN_PRETTIER Pass ✅
NATURAL_LANGUAGE Pass ✅
POWERSHELL Pass ✅
PRE_COMMIT Pass ✅
SPELL_CODESPELL Pass ✅
TRIVY Pass ✅
TYPESCRIPT_PRETTIER Pass ✅
XML Pass ✅
YAML Pass ✅
YAML_PRETTIER Pass ✅

All files and directories linted successfully

For more information, see the GitHub Actions workflow run

Powered by Super-linter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants