test(e2e): wait for batch evaluation before archiving - #2499
Conversation
GitHub Actions
Failure
agentcore-devx-automation
Bot
added
agentcore-harness-reviewing
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
Package TarballHow to installgh release download pr-2499-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.31.0.tgz |
| expect(json).toHaveProperty('success', true); | ||
| expect(json.id).toBeTruthy(); | ||
| expect(json.status).not.toBe('FAILED'); | ||
| expect(['COMPLETED', 'COMPLETED_WITH_ERRORS', 'STOPPED']).toContain(json.status); |
There was a problem hiding this comment.
is completed with errors acceptable?
There was a problem hiding this comment.
Yes, for this archive lifecycle test. COMPLETED_WITH_ERRORS is terminal, and the service explicitly permits deleting it.
We’re testing successful archive and local-record cleanup, not evaluation correctness. The previous assertion only excluded FAILED, so it already accepted COMPLETED_WITH_ERRORS.
A test validating evaluation results should require COMPLETED and inspect the results.
![agentcore-devx-automation[bot]](https://avatars.githubusercontent.com/u/2232217?s=60&v=4){kind=small}
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
Small, well-scoped e2e test change. Adding --wait to the run batch-evaluation invocation is the right fix: the subsequent archive steps need the batch evaluation to be in a terminal state before deletion, so making the test block eliminates a race that not.toBe('FAILED') was masking. The tightened assertion (['COMPLETED', 'COMPLETED_WITH_ERRORS', 'STOPPED']) is strictly stricter than the previous check and correctly excludes FAILED/CANCELLED, which TERMINAL_STATUSES['batch-evaluation'] would otherwise allow.
One minor observation (not blocking): the outer retry(..., 6, 15000) combined with a 600s test timeout and a now-blocking --wait means retries will rarely get a second chance if the first run uses most of the budget — but a successful first attempt is the expected path, and failures surface clearly from vitest's timeout, so this is fine as-is.
agentcore-devx-automation
Bot
removed
the
agentcore-harness-reviewing
This branch had an error being deployed
2 participants
Description
Fix the archive lifecycle E2E test's race with batch evaluation completion.
The test previously submitted an asynchronous batch evaluation and later attempted to archive it without checking whether it had finished. When it was still
IN_PROGRESS, the service correctly returned 409. This also broke the local-record removal and repeated-archive assertions.Use the existing
run batch-evaluation --waitflag and assert a deletable terminal status before continuing. Keep the existing exclusion ofFAILEDevaluations.This is a one-file test-only change: no production behavior, infrastructure, fixed sleeps, or new polling helpers.
Related Issue
Closes #2498
Documentation PR
Not applicable; test synchronization only.
Type of Change
Testing
npm run test:unitandnpm run test:integnpm run typechecknpm run lintsrc/assets/, I rannpm run test:update-snapshotsand committed the updated snapshotsFocused verification:
archive-lifecycle.test.ts: all 15 checks passed using the normal CDK dependency with no local CDK override.Full unit/integration suites were not rerun for this test-only change. Lint exited successfully with 35 existing warnings in unchanged files. No assets were modified.
Checklist
No new documentation or dependent changes are required.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.