Skip to content

test: trim scheduled e2e matrix and harden dev-server process cleanup#1122

Open
tejaskash wants to merge 2 commits intomainfrom
worktree-test-cleanup-day7
Open

test: trim scheduled e2e matrix and harden dev-server process cleanup#1122
tejaskash wants to merge 2 commits intomainfrom
worktree-test-cleanup-day7

Conversation

@tejaskash
Copy link
Copy Markdown
Contributor

@tejaskash tejaskash commented May 5, 2026

Summary

  • Scheduled Monday e2e run now targets 6 priority suites (CodeZip, Container, custom JWT auth, evals, import, local dev). Manual dispatch still runs the full e2e directory to exercise every framework/model-provider combo — halves the weekly AWS test burn
  • Replace fire-and-forget SIGTERM in `integ-tests/dev-server.test.ts` with a process-group terminator: signal via `-pid`, wait on exit event, SIGKILL after 5s timeout. Spawn uses `detached:true` so the group signal reaches subprocesses

Paired PR

The CDK construct test isolation fix (`setupMocks()` mock pollution in `evaluator.test.ts`) lives in the `agentcore-l3-cdk-constructs` repo as a separate PR, since it's a different package.

Test plan

  • Integ tests still pass — `npm run test:integ`
  • `e2e-tests-full.yml` YAML is valid (validated locally with Python yaml)
  • Manual workflow_dispatch of `e2e-tests-full.yml` still runs the full suite (sanity-check before relying on the schedule split)

@tejaskash tejaskash requested a review from a team May 5, 2026 18:59
@github-actions github-actions Bot added size/s PR size: S agentcore-harness-reviewing AgentCore Harness review in progress labels May 5, 2026
@agentcore-cli-automation
Copy link
Copy Markdown

Reviewed — LGTM. The dev-server process-group teardown logic looks correct (detached: true ensures the child is a PG leader so process.kill(-pid, ...) targets the group, ESRCH is swallowed, and the 5s SIGKILL fallback is sensible). The workflow conditional correctly leaves workflow_dispatch and push on the full suite while trimming only the scheduled Monday run, and the six files listed in SCHEDULED_TESTS all exist under e2e-tests/. The npx vitest run --project e2e <files> filter pattern is already proven in e2e-tests.yml.

Two minor observations (non-blocking, feel free to ignore):

  • With 6 files × 6 shards on scheduled runs, each shard runs exactly one test file, so the per-job setup overhead (checkout, npm ci, build, CDK tarball, global install) is amortized over a single deployment. If you want to squeeze more out of this, you could drop shard to 3/3 or 2/2 for scheduled runs. Not a correctness issue.
  • In terminateDevProcess, if the dev process has already exited before afterEach runs, the proc.on('exit', ...) listener is attached post-event and never fires, so the promise only resolves via the 5s timer. A quick if (proc.exitCode !== null || proc.signalCode !== null) { resolve(); return; } guard before attaching the listener would avoid that. In practice the only test here calls terminateDevProcess() inline, so afterEach sees devProcess === null and returns immediately — no real impact today.

@github-actions github-actions Bot removed the agentcore-harness-reviewing AgentCore Harness review in progress label May 5, 2026
@tejaskash tejaskash force-pushed the worktree-test-cleanup-day7 branch from de1bdce to 2f27d6c Compare May 5, 2026 19:29
@github-actions github-actions Bot added size/s PR size: S and removed size/s PR size: S labels May 5, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 43.1% 9015 / 20912
🔵 Statements 42.39% 9573 / 22582
🔵 Functions 39.91% 1553 / 3891
🔵 Branches 39.98% 5808 / 14527
Generated in workflow #2472 for commit 609f73e by the Vitest Coverage Report Action

# Scheduled runs cover 6 priority suites spanning CodeZip, Container, Auth,
# Evals, Import, and local Dev paths. Manual workflow_dispatch runs the
# full e2e directory to exercise every framework/model-provider combo.
SCHEDULED_TESTS: >-
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

whats the advantage of scheduled tests if we run them on every commit?

tejaskash added 2 commits May 5, 2026 19:18
Scheduled Monday e2e run now targets 6 priority suites covering CodeZip,
Container, custom JWT auth, evals, import, and local dev paths. Manual
workflow_dispatch still runs the full e2e directory to exercise every
framework/model-provider combo. Halves weekly AWS test burn from 6 shards
of the full suite to 6 shards of the priority subset.

Replace fire-and-forget SIGTERM in integ-tests/dev-server.test.ts with a
process-group-aware terminator that: signals the whole group via -pid,
waits on the child's exit event, and SIGKILLs the group after a 5s
timeout if the process hangs. Spawn now uses detached:true so the
process-group signal actually reaches subprocesses of the dev server.
@tejaskash tejaskash force-pushed the worktree-test-cleanup-day7 branch from 0a2c8b3 to 609f73e Compare May 5, 2026 23:18
@github-actions github-actions Bot added size/s PR size: S and removed size/s PR size: S labels May 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/s PR size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants