Skip to content

Fix: Run the unit:integrations vitest project in CI and de-rot its version test - #245

Closed
AmaadMartin wants to merge 1 commit into
mainfrom
fix/run-unit-integrations-project-in-ci
Closed

Fix: Run the unit:integrations vitest project in CI and de-rot its version test#245
AmaadMartin wants to merge 1 commit into
mainfrom
fix/run-unit-integrations-project-in-ci

Conversation

@AmaadMartin

@AmaadMartin AmaadMartin commented Jul 29, 2026

Copy link
Copy Markdown
Owner

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

  1. Link to an existing issue (if applicable):

No existing issue.

  1. Or, if no issue exists, describe the change:

Problem:

vitest.config.ts declares six projects (unit:core, unit:dev,
unit:integrations, integration, e2e, cross-language), but the test
scripts in the root package.json enumerate the unit projects by hand and
omitted unit:integrations:

script projects run (before)
test unit:core, unit:dev, integration, e2e
test:unit unit:core, unit:dev
test:coverage unit:core, unit:dev, integration, e2e (+ --coverage)

.github/workflows/validation.yaml runs exactly one test command,
npm run test:coverage, so integrations/test/**/*_test.ts was never executed
by CI. unit:integrations was the only declared project that no script and no
workflow ran. (cross-language is intentionally excluded from the default
suite — it needs a Go toolchain and has its own workflow,
.github/workflows/cross-language-integration.yml.)

That dark corner then rotted. integrations/test/version_test.ts asserted a
hardcoded expect(version).toBe('1.3.0') while integrations/src/version.ts
had already been bumped to 1.4.0, so the integrations workspace's only unit
test failed the moment it was run — invisibly, because nothing ran it.

Solution:

  1. Add --project unit:integrations to test, test:unit and
    test:coverage, positioned after unit:dev so the ordering matches the
    project declaration order in vitest.config.ts. test:integration,
    test:e2e and test:cross-language are deliberate single-project entry
    points and are unchanged. integrations/test/**/*_test.ts is now enforced on
    every push and PR via validation.yaml, across all three OS matrix legs.

  2. Rewrite the assertion in integrations/test/version_test.ts so it cannot rot
    again. release-please-config.json registers integrations/src/version.ts
    as a generic extra-files target and uses the node-workspace +
    linked-versions plugins, so every release rewrites both that literal and
    the version field of integrations/package.json in the same commit. Any
    hardcoded literal in the test is therefore guaranteed to rot at the next
    release. The test now reads integrations/package.json at runtime and
    asserts equality against it, making the manifest a self-maintaining oracle.

    A second assertion checks semver shape. The equality check catches a partial
    release that bumps one file but not the other; the shape check catches the
    degenerate case where both drift together into something that is not a
    version at all.

    resolveJsonModule is not enabled anywhere in the tsconfig chain, so the
    manifest is read with readFileSync(new URL('../package.json', import.meta.url)) rather than a JSON import, which would not typecheck.

Coverage thresholds: unchanged. vitest.config.ts is not touched by this
PR. Running unit:integrations moves integrations/src/version.ts and
index.ts from the uncovered into the covered column (Vitest's coverage.all
already counted those 22 lines in the denominator), so the numbers move up, not
down. Verified on a full CI npm run test:coverage run of this branch:
All files reported 90.14 % statements / 88.95 % branches / 91.04 %
functions / 90.14 % lines
, comfortably above the committed thresholds of
86 / 87 / 88 / 86. CI confirms the project now runs:
✓ unit:integrations integrations/test/version_test.ts (2 tests) on all three
OS legs.

No production code, public API, or dependency changes — no file under any
src/ directory is modified. Exactly two files change.

Note on a pre-existing, unrelated local failure.
dev/test/cli/cli_create_test.ts'should handle Vertex AI selection with gcloud defaults' fails on machines that have a local gcloud configuration
exported (createAgent() prefers GOOGLE_CLOUD_PROJECT /
GOOGLE_CLOUD_LOCATION over the mocked gcloud config get-value output, and
the suite does not isolate process.env). It lives in unit:dev, which CI
already ran before this change, so it is pre-existing and out of scope here; it
passes in CI, where those variables are unset. Left untouched deliberately —
not skipped, not deleted.

Testing Plan

Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes.

Unit Tests:
[x] I have added or updated unit tests for my change.
[x] All unit tests pass locally.

$ npx vitest run --project unit:integrations
 ✓ |unit:integrations| integrations/test/version_test.ts (2 tests) 6ms
 Test Files  1 passed (1)
      Tests  2 passed (2)
$ npm run test:unit      # now selects three projects
> vitest --project unit:core --project unit:dev --project unit:integrations
 ✓  unit:integrations  integrations/test/version_test.ts (2 tests) 7ms
 Test Files  1 failed | 174 passed (175)
      Tests  1 failed | 2413 passed (2414)

The single failure is the pre-existing cli_create_test.ts gcloud-dependent
case described above; it is untouched by this diff and reproduces identically
without these changes.

Negative tests — proving the new assertions actually guard:

# manifest drifts, module does not -> equality must fail
$ sed -i '0,/"version": "1.4.0"/s//"version": "9.9.9"/' integrations/package.json
$ npx vitest run --project unit:integrations
AssertionError: expected '1.4.0' to be '9.9.9' // Object.is equality
      Tests  1 failed | 1 passed (2)

# module drifts to a non-version -> both must fail
$ sed -i "s/'1\.4\.0'/'not-a-version'/" integrations/src/version.ts
$ npx vitest run --project unit:integrations
AssertionError: expected 'not-a-version' to be '1.4.0'
AssertionError: expected 'not-a-version' to match /^\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?(?:\+[0-9A-Za-z.-]+)?$/
      Tests  2 failed (2)

The semver pattern was checked against real release-please output shapes so it
cannot false-positive and block a release: 1.4.0, 1.4.0-alpha.1,
1.5.0-rc-1 and 1.4.0+build.3 all match; not-a-version and 1.4 do not.

Lint / format / license:

$ npx prettier --check integrations/test/version_test.ts package.json   # clean
$ npx eslint integrations/test/version_test.ts                          # clean
$ bash scripts/check_license.sh   # All files have the correct license header.

Manual End-to-End (E2E) Tests:

Simulate a release-please bump and confirm the test no longer rots. Under the
old hardcoded assertion this scenario failed; it now passes, which is the whole
point of the change.

# bump every workspace file the way release-please would
sed -i "s/'1\.4\.0'/'1.5.0'/" integrations/src/version.ts
sed -i '0,/"version": "1.4.0"/s//"version": "1.5.0"/' integrations/package.json

npx vitest run --project unit:integrations
#  Test Files  1 passed (1)
#       Tests  2 passed (2)

git checkout -- integrations/src/version.ts integrations/package.json

Checklist

[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas.
[x] I have added tests that prove my fix is effective or that my feature works.
[x] New and existing unit tests pass locally with my changes.

… test

vitest.config.ts declares six projects, but the `test`, `test:unit` and
`test:coverage` scripts enumerate the unit projects by hand and omitted
`unit:integrations`. CI runs `test:coverage`, so `integrations/test/**/*_test.ts`
was never executed by .github/workflows/validation.yaml and the entire
integrations test surface was dark.

Add `unit:integrations` to those three scripts, ordered to match the project
declaration order in vitest.config.ts. `test:integration`, `test:e2e` and
`test:cross-language` are deliberate single-project entry points and are left
alone; `cross-language` stays out of the default suite because it needs a Go
toolchain and has its own workflow.

Then repair the test that rotted while nobody was running it. It asserted a
hardcoded '1.3.0' against an export release-please had already bumped to
'1.4.0'. Derive the expectation from integrations/package.json at runtime
instead: release-please rewrites the manifest and src/version.ts in the same
release commit, so the manifest is a self-maintaining oracle and the literal
cannot drift again. A second assertion checks semver shape, which catches the
degenerate case where both files drift together into something that is not a
version at all.

Coverage thresholds are unaffected. No production code, public API or
dependency changes.
@AmaadMartin
AmaadMartin force-pushed the fix/run-unit-integrations-project-in-ci branch from d2ef13d to 72013d3 Compare July 29, 2026 13:48
@AmaadMartin AmaadMartin changed the title Fix: run all unit:* vitest projects in CI and repair the two unit tests it exposes Fix: Run the unit:integrations vitest project in CI and de-rot its version test Jul 29, 2026
@AmaadMartin

Copy link
Copy Markdown
Owner Author

Heads-up for whoever reviews these: this PR and #236 are duplicates — both wire --project unit:integrations into test/test:unit/test:coverage (byte-identical package.json hunk) and both de-rot integrations/test/version_test.ts by asserting against the version declared in integrations/package.json.

The only substantive differences are that this PR reads the manifest with readFileSync + JSON.parse where #236 uses a JSON module import, and this PR adds a second semver-shape assertion.

They conflict on both files, so only one should land. #236 was opened first (2026-07-29T09:55:08Z vs 2026-07-29T12:15:53Z). Filing this as a cross-reference only — no opinion here on which one you keep.

@AmaadMartin

Copy link
Copy Markdown
Owner Author

Closing as a duplicate of #236, which lands the same change and was opened first (2026-07-29T09:55:08Z vs 12:15:53Z here).

Both PRs make the identical, byte-for-byte edit to the three root test scripts (test, test:unit, test:coverage gain --project unit:integrations). The only difference is how integrations/test/version_test.ts derives the expected version:

I verified #236's approach locally rather than assuming: it typechecks under the repo's module: nodenext (resolveJsonModule is implied true for node16/nodenext, so the JSON import compiles), the project passes, and a mutation of integrations/src/version.ts to '0.0.0-mutant' makes it fail with expected '0.0.0-mutant' to be '1.4.0' — so the assertion is real, not vacuous. #236 is the simpler of the two: one line, no cast, same behaviour.

#236 is also the base of a live stack — #311 (wildcard unit:* project selection) and #343 (CI guard that fails when a vitest project is run by no npm script) both branch from it — so it is the version that should carry forward. Keeping this PR open would risk a second, competing upstream port of the same four-line change.

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