Repository navigation
ci: scope new-service pull requests to their own integration suite - #805
Merged
Merged
Conversation
A new-service PR still ran every suite because three files outside the domain folders change on each one: - package.json (exports, scripts, version) is ignored. A dependency change or version bump still runs everything through package-lock.json. - tests/utils/constants/<name>.ts is scoped like src/services/<name>/ when <name> is a suite domain; common.ts and entity-named files still run everything. - tests/integration/config/unified-setup.ts, where services are registered, runs only the always-on suites when the change only adds lines (learned from `git diff --numstat`); any other edit runs everything. The always-on suites load the file. A drift guard fails when a domain-named constants file is used by another suite. It found the observability agent-traces suite using agent fixtures from agents.ts; those now live in common.ts as TEST_AGENT, which AGENT_TEST_CONSTANTS references so the unit tests are unchanged. Verified against #664 (feat/platform-groups): scope=platform. Co-Authored-By: Claude Fable 5.1 <[email protected]>
Contributor
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
Everything under scripts/ is CI and release tooling (API-surface check, sample builds, docs, release metadata, the scoping resolver itself); nothing there takes part in an integration run, and each script has its own unit tests. A broken resolver fails the fail-closed integration-scope job rather than skipping anything silently. Co-Authored-By: Claude Fable 5.1 <[email protected]>
Contributor
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
tests/unit was already ignored; tests/utils holds unit-test fixtures and helpers. Four fixture files under tests/utils/constants are read by the agents, http and observability suites, so a fixture-only edit is knowingly uncovered until the weekly run or the next change to that suite. This supersedes the by-name rule for tests/utils/constants/<name>.ts and its drift guard, and reverts the TEST_AGENT fixture move they required. Co-Authored-By: Claude Fable 5.1 <[email protected]>
Contributor
|
✅ No issues found. Checked for bugs and CLAUDE.md compliance. |
|
amrit-agarwal-1
approved these changes
Oct 5, 2026
abhishekks805
approved these changes
Oct 5, 2026
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The changes match the intended trade-offs, with only a minor documentation inconsistency remaining.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Refines integration-test scoping so new-service PRs run their relevant suites rather than the full suite.
Changes:
- Ignores package metadata, scripts, and test helpers.
- Uses diff statistics to limit additions-only registry changes to always-on suites.
- Updates resolver tests and documents coverage trade-offs.
| File | Description |
|---|---|
| tests/unit/scripts/integration-scope.test.ts | Tests exclusions, registry scoping, and diff parsing. |
| tests/integration/README.md | Documents scoping rules and accepted coverage gaps. |
| scripts/integration-scope.mjs | Implements exclusions and additions-only detection. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+663
to
+665
| `tests/utils/` is ignored as a whole although a few fixture values under | ||
| `tests/utils/constants/` are read by the agents, http and observability suites, so a | ||
| fixture-only edit is not covered until the weekly run or the next change to that suite. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #775 and #802. A new-service pull request such as #664 still ran the full integration suite, because files outside the domain folders change on every such PR. Each now has a rule:
package.json(exports, scripts, version)package-lock.json.tests/utils/(unit-test fixtures and helpers;tests/unit/was already ignored)mocks/tests/utils/constants/(agents.ts,memory.ts,http.ts,common.ts) are read by integration suites, so a fixture-only edit is knowingly uncovered until the weekly run or the next change to that suite.tests/integration/config/unified-setup.tswhen the change only adds lines, which is how a service is registeredscripts/integration-scopejob.Mechanics:
--basenow readsgit diff --numstat, so the resolver knows which changed files only gained lines (parseNumstat, anadditiveset threaded throughresolveArgs→resolveScope→classify).--fileshas no such information and keeps the conservative behaviour.Verified against #664 (
feat/platform-groups, rebased on main):scope=platform, withunified-setup.ts only adds lines (a service registration); the always-on suites load itin the log. Dry runs on single files:tests/utils/constants/platform.tsnonetests/utils/setup.tsnonepackage.jsonnonepackage.json+package-lock.jsonallAccepted trade-offs, stated in the README: an edit to an npm script in
package.jsonis not covered by a pull-request run; an additions-only edit to the registry that changes behaviour for one domain is covered only by the always-on suites; a fixture-only edit undertests/utils/is not covered. Core, workflow andtests/integration/configchanges other than that still run everything.Docs:
tests/integration/README.md("Which suites run on a pull request").Test plan
npm run lint,npm run test:unit(nosrc/change, typecheck unaffected)scope=platformcoverage / integration-scopeon this PR resolvesscope=none: the script, its tests and the README are all ignored paths🤖 Generated with Claude Code