Repository navigation
test: classify networked suites and lock live-network runs - #372
Merged
Merged
Conversation
andrevmatos
requested review from
a team,
PabloMansanet and
aelmanaa
as code owners
August 25, 2026 16:10
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
π andrevmatos, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
aelmanaa
previously approved these changes
Aug 25, 2026
andrevmatos
force-pushed
the
tests/contention
branch
from
August 25, 2026 16:21
60bbb17 to
61a6eb5
Compare
andrevmatos
force-pushed
the
tests/contention
branch
2 times, most recently
from
August 25, 2026 16:27
8119986 to
2f8ff5c
Compare
Coverage Report |
There was a problem hiding this comment.
Pull request overview
Adds suffix-based test classification and cross-process network locks to reduce RPC contention during concurrent test execution.
Changes:
- Adds unit-test filtering and workspace scripts.
- Serializes networked suites by resource tags.
- Updates documentation and CI tooling.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/useResource.ts |
Adds filesystem-based resource locks. |
scripts/run-tests.mjs |
Adds suffix-based test filtering. |
package.json |
Updates root unit-test command. |
CONTRIBUTING.md |
Documents test categories and locks. |
CHANGELOG.md |
Records test infrastructure changes. |
ccip-sdk/src/ton/logs.integration.test.ts |
Locks TON testnet access. |
ccip-sdk/src/ton/index.integration.test.ts |
Locks TON testnet access. |
ccip-sdk/src/sui/integration.test.ts |
Locks Sui and Fuji access. |
ccip-sdk/src/solana/fork.test.ts |
Locks Solana and API access. |
ccip-sdk/src/solana/__tests__/integration.test.ts |
Locks utilized RPC networks. |
ccip-sdk/src/evm/integration.test.ts |
Locks EVM networks and API access. |
ccip-sdk/src/evm/fork.test.ts |
Locks fork upstreams and API access. |
ccip-sdk/src/evm/dest-liquidity.integration.test.ts |
Locks Sepolia and Fuji access. |
ccip-sdk/src/evm/dest-liquidity.fork.test.ts |
Locks fork upstream access. |
ccip-sdk/src/api/api.integration.test.ts |
Locks CCIP API access. |
ccip-sdk/package.json |
Adds SDK unit-test filtering. |
ccip-cli/src/commands/show.e2e.test.ts |
Classifies and locks show E2E tests. |
ccip-cli/src/commands/lane.e2e.test.ts |
Classifies and locks lane E2E tests. |
ccip-cli/src/commands/e2e-helpers.test.ts |
Documents and types CLI output capture. |
ccip-cli/package.json |
Adds CLI unit-test filtering. |
.github/workflows/publish.yml |
Updates workflow actions. |
.github/workflows/docs.yml |
Updates documentation workflow actions. |
.github/workflows/ci.yml |
Updates actions and Foundry installation. |
Suppressed comments (3)
scripts/useResource.ts:215
release()is documented as idempotent, but a second call decrements an absent count and removes the directory again. If another process acquired the resource after the first release, that second call deletes its live lock. Track whether this handle has already released before touching shared state.
release: async () => {
for (const resource of [...wanted].reverse()) {
const remaining = (held.get(resource) ?? 0) - 1
if (remaining <= 0) {
held.delete(resource)
rmSync(lockDir(resource), { recursive: true, force: true })
scripts/useResource.ts:201
- Skipping a process-local resource does not increment its refcount for the new handle. Consequently, two
useResource(['x'])calls return two handles backed by count 1, and releasing either handle removes the lock while the other is still active. Record pre-held resources once the full requested set has been acquired.
for (const resource of wanted) {
if (isHeldHere(resource)) continue
if (tryAcquire(resource)) {
held.set(resource, (held.get(resource) ?? 0) + 1)
ccip-cli/src/commands/show.e2e.test.ts:8
- The
showcommand calls the default CCIP API for every 32-byte transaction hash (show.ts:171-176), but this suite does not acquire the sharedapiresource. It can therefore contend with the API and EVM/Solana integration suites despite the new locking scheme.
π‘ Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+153
to
+155
| if (!isStale(resource)) return false | ||
| const holder = describeHolder(resource) | ||
| rmSync(dir, { recursive: true, force: true }) |
Comment on lines
+39
to
+41
| return Object.entries(TAG_SUFFIXES) | ||
| .filter(([, suffix]) => lower.endsWith(suffix)) | ||
| .map(([tag]) => tag) |
Comment on lines
+29
to
+35
| - name: Install Foundry (anvil for fork tests) | ||
| uses: foundry-rs/foundry-toolchain@8789b3e21e6c11b2697f5eb56eddae542f746c10 # v1.7.0 | ||
| run: | | ||
| # The canonical one-liner installs the compiled foundryup binary to | ||
| # $HOME/.config/.foundry/bin (the legacy ~/.foundry path is gone); running it | ||
| # then installs forge/cast/anvil (stable, ethereum β the repo defaults). | ||
| curl -sSL https://foundry.paradigm.xyz | bash | ||
| "$HOME/.config/.foundry/bin/foundryup" |
andrevmatos
force-pushed
the
tests/contention
branch
from
August 25, 2026 16:47
2f8ff5c to
ee7c30b
Compare
- CLI e2e suites renamed to *.e2e.test.ts; test files classified by filename suffix (integration/e2e/fork), excluded from the new npm run test:unit via scripts/run-tests.mjs - add scripts/useResource.ts: OS-level per-network locks (atomic mkdir, pid-liveness staleness) so concurrently running node --test suites never share a live RPC endpoint; suites hold their networks via await useResource([...]) and release on process exit - harden lock paths for the predictable os.tmpdir() names: owner-only mode, symlink/ownership validation (CodeQL js/insecure-temporary-file) - CI: replace the broken foundry-toolchain action (foundryup is now a compiled binary the action still bash-executes) with the official installer; bump checkout/setup-node/artifacts/github-script to current majors (clears the Node 20 action-runtime deprecation warnings) - CHANGELOG entry under Unreleased
andrevmatos
force-pushed
the
tests/contention
branch
from
August 25, 2026 16:48
ee7c30b to
aa1fc4a
Compare
| } | ||
|
|
||
| function tagsOf(filePath) { | ||
| const name = filePath.split('/').at(-1) ?? '' |
Comment on lines
+108
to
+113
| if (ownerPid !== undefined) { | ||
| try { | ||
| process.kill(ownerPid, 0) // same-machine pid: locks are per-machine | ||
| return false // alive β never steal, regardless of age | ||
| } catch (err) { | ||
| return (err as NodeJS.ErrnoException).code === 'ESRCH' // dead β stale |
Comment on lines
+44
to
+47
| import { join } from 'node:path' | ||
|
|
||
| const LOCK_ROOT = | ||
| process.env['CCIP_TOOLS_TEST_LOCK_DIR'] || join(tmpdir(), 'ccip-tools-ts-network-locks') |
Comment on lines
+30
to
+33
| const TAG_SUFFIXES = { | ||
| integration: 'integration.test.ts', | ||
| e2e: 'e2e.test.ts', | ||
| fork: 'fork.test.ts', |
aelmanaa
approved these changes
Aug 25, 2026
Collaborator
Author
|
Addressed all review points in Copilot (suppressed)
Copilot (inline)
CodeQL (github-advanced-security[bot])
First CI attempt hit transient upstream flakiness (staging API HTTP 503 on the latency endpoint; toncenter 429s on the TON v2 walk); the rerun on the same commit is fully green. |
andrevmatos
enabled auto-merge (squash)
August 25, 2026 17:17
This branch was successfully deployed
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.
What
A small test-infra refactor: every networked test suite is classified by filename suffix and serializes its live-network access through OS-level locks, so
node --test's concurrent file runs stop rate-limiting shared public RPCs.Changes
show.test.ts β show.e2e.test.ts,lane.test.ts β lane.e2e.test.ts. Test files are classified purely by suffix (*.integration.test.ts,*.e2e.test.ts,*.fork.test.ts), so a bareintegration.test.tscounts as integration exactly likelogs.integration.test.ts.npm run test:unitβ new script (root + both workspaces) running only offline unit tests viascripts/run-tests.mjs(filename-only filtering;--exclude/--tags/--dry).npm testis unchanged and still runs everything.scripts/useResource.ts. Networked suites declare the networks they talk to:<tmpdir>/ccip-tools-ts-network-locks, created atomically withfs.mkdir; exactly one process holds a tag at a time, and a waiting suite starts the moment the holder finishes β there is no batch/wave scheduling, the filesystem is the queue. Acquisition is all-or-nothing (deadlock-free), locks auto-release on process exit, and crashed holders self-heal via pid-liveness (a lock held by a live process is never stolen). All 13 networked suites are wired with tags verified against the endpoints they actually call.npm teststill runs everything in one job, and network contention is now resolved by the locks themselves (locks are per-machine, so the networked suites intentionally stay on the single CI runner rather than splitting across jobs).CONTRIBUTING.mdgained "Test Suite Layout" + "Networked tests and theuseResourcelock" sections;CHANGELOG.mdUnreleased entry added.Validation
npm run checkclean (lint 0 warnings, typecheck,generate:lintup-to-date)npm run test:unitβ 1313 tests, 0 failuresnpm test(CI parity) β 1327 sdk + 117 cli, 0 failures; lock contention observed and resolved during the run (e.g.waiting for "fuji"heartbeats), zero lock residue afterwards