test(e2e): add insights and online-insights lifecycle tests#1599
test(e2e): add insights and online-insights lifecycle tests#1599notgitika wants to merge 2 commits into
Conversation
Adds end-to-end coverage for the Lens/Insights feature shipped in the NYS summit release. Two independent sequential suites in e2e-tests/insights-lifecycle.test.ts, each owning its own deployed agent and CFN stack so a deploy failure in one suite does not blank the other: - online-insights lifecycle: add online-insights -> deploy -> invoke -> pause -> resume -> teardown. Verifies live executionStatus toggling through the control plane. - run-insights and recommendation chain: deploy -> invoke -> run insights (async) -> view list -> view detail -> archive -> run insights --wait -> run recommendation --from-insights -> teardown. Covers async submission, local job storage, view/archive round-trip, and the chain from a completed insights job into a system-prompt recommendation. The chain step accepts either success or a service error indicating the upstream job had no usable sessions, since real trace volume is not guaranteed seconds after invoke; flag-parsing errors fail hard.
github-actions
Bot
added
the
agentcore-harness-reviewing
agentcore-devx-automation
Bot
added
the
claude-security-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-1599-tarball --repo aws/agentcore-cli --pattern "*.tgz" --dir /tmp/pr-tarball
npm install -g /tmp/pr-tarball/aws-agentcore-0.20.2.tgz |
agentcore-cli-automation
left a comment
agentcore-cli-automation
left a comment
There was a problem hiding this comment.
Test additions look thorough and follow the existing evals-lifecycle.test.ts pattern. One blocker before merge: the view insights <id> test asserts a region field that the CLI doesn't actually emit. Otherwise the wiring against the real commands looks correct.
| expect(json.success).toBe(true); | ||
| expect(json.id).toBe(asyncJobId); | ||
| expect(json.status).toBeTruthy(); | ||
| expect(json.region).toBeTruthy(); |
There was a problem hiding this comment.
This assertion will fail. view insights <id> --json outputs serializeResult({ success: true, ...record, ...extra }) (src/cli/commands/view/command.tsx:70), where record is an InsightsJobRecord. Per src/cli/operations/jobs/shared/types.ts:50 (JobRecordBase) the comment is explicit:
Fields every job record carries, regardless of type. Note: region is NOT stored — parse from
arn.
So the serialized record has arn but no region field, and json.region will be undefined. The InsightsJobJson interface at the top of the file declares region: string, which is similarly misleading — that's a runtime cast and TypeScript doesn't validate it.
Options to fix:
- Drop the assertion (recommended —
id+statusalready prove the detail call succeeded) and removeregionfrom theInsightsJobJsoninterface. - Assert on
json.arninstead (which is present) and optionally check it starts witharn:aws:bedrock-agentcore:. - If
regionis genuinely intended to be in the JSON output, that's a CLI bug to fix inview/command.tsx(and probablyrun/command.tsxtoo) rather than the test — but that's out of scope for this PR.
There was a problem hiding this comment.
Good catch — confirmed JobRecordBase only stores arn and notes "region is NOT stored — parse from arn". Went with option 2 from your suggestion: dropped region from the InsightsJobJson interface and changed the detail assertion to expect(json.arn).toMatch(/^arn:[^:]+:bedrock-agentcore:/), which proves the field is present and well-formed across partitions. Fixed in f2611c6.
github-actions
Bot
removed
the
agentcore-harness-reviewing
JobRecordBase does not store region — region is parsed from arn. The view insights <id> --json output therefore has no region field. Switch the detail-call assertion to match arn against the bedrock-agentcore ARN shape and update the InsightsJobJson interface accordingly.
agentcore-devx-automation
Bot
added
the
claude-security-reviewing
|
Claude Security Review: no high-confidence findings. (run) |
agentcore-devx-automation
Bot
removed
the
claude-security-reviewing
2 participants
This PR adds e2e tests for insights feature that was recently shipped. two independent
describe.sequentialsuites in a single file (e2e-tests/insights-lifecycle.test.ts), each owning its own deployed agent and CFN stack so a deploy failure in one suite does not blank the other.Suite A:
online-insightslifecycleadd online-insights → deploy → invoke → pause → resume → teardown. Mirrorsevals-lifecycle.test.ts. Verifies liveexecutionStatustoggling through the control plane.Suite B:
run insights+ recommendation chaindeploy → invoke → run insights (async) → view list → view detail → archive → run insights --wait → run recommendation --from-insights → teardown. Covers:run insights(no--wait) and local job recordview insightslist + per-id detailarchive insightsround-trip (job no longer appears inview insights)run insights --waitreaching a terminal statusrun recommendation --from-insightschaining off the completed insights jobThe chain step accepts either
success: trueor a service error containingno sessions/completed_with_errors/failed/empty, since real trace volume is not guaranteed seconds after invoke. Flag-parsing errors (e.g.Unknown option,--from-insights) still fail the test hard, so the wiring is verified even when the upstream job has nothing to learn from.