fix: report skipped and hook-aborted tests in the CLI flow#67
fix: report skipped and hook-aborted tests in the CLI flow#67kamal-kaur04 wants to merge 3 commits into
Conversation
…he CLI flow In the CLI (gRPC) pipeline, tests that never reach beforeTest/afterTest emitted no events at all: static it.skip, this.skip() inside before/beforeEach hooks, and suites aborted by a failed before hook. The legacy Listener -> api/v1/batch path these flowed through previously is not functional in the CLI pipeline, so such tests were invisible on the dashboard and their Automate sessions were never linked to the build. - add cli/skipReporter: routes skipped tests through the TestFramework tracker using the same INIT_TEST -> TEST PRE -> LOG_REPORT POST -> TEST POST sequence afterTest uses (LOG_REPORT/POST is what loads the result), serialized through a single chain since wdio does not await reporter hooks - reporter.onTestSkip: CLI branch reporting the skipped test with a resolved spec file (events without a location are rejected downstream) and the suite chain passed via ctx for hierarchy extraction - service.afterHook: on a non-passed BEFORE_ALL/BEFORE_EACH/AFTER_EACH hook, report the suite's undetermined tests as skipped (port of the legacy insights-handler cascade); service.beforeTest marks started tests so runtime this.skip() is never double-reported - wdioMochaTestFramework: hook results read `.status`, which does not exist on Frameworks.TestResult, leaving hook_result at 'pending' (coerced to 'passed' downstream) — failed before-hooks produced green builds while CI exited 1. Map passed/skipped/failed from the actual result fields. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- batchAndPostEvents: read the response as text and check response.ok — error responses (401/5xx) and empty bodies are not JSON, and the blind response.json() surfaced every failure as a misleading "Unexpected end of JSON input". Failures now carry the HTTP status and a body snippet. - listener: include the failure reason when a batch is marked failed instead of only the event count. - trackHookEvents: also treat mocha's sync-skip error shape as a skipped hook (wdio v8 delivers this.skip() hooks without a `skipped` flag; harmless on v9, keeps both lines aligned). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
fix: report skipped and hook-aborted tests in the CLI flow
|
✅ Good to go
↻ This verdict comment is the review anchor — it's updated in place on each run (the gate posts its status separately). — SDK PR Review Agent |
|
🟢 SDK PR Review gate is green — the SDK PR Review Agent has given a GTG for this PR (the A native GitHub reviewer approval is still separately required by branch protection before this PR can merge — this check does not substitute for that. |
|
RUN_TESTS |
| // skip, not a failure. | ||
| // guard the sync-skip error shape too (wdio v8 delivers this.skip() hooks without | ||
| // a `skipped` flag; harmless on v9, keeps the lines aligned) | ||
| const skippedHook = testResult?.skipped || !!testResult?.error?.message?.includes('sync skip; aborting execution') |
There was a problem hiding this comment.
.includes('sync skip; aborting execution')
I haven't checked this myself -> Is there a better way to check this than asserting error message string? (Asking because string literals might change in the future without us realising the impact)
There was a problem hiding this comment.
Ideally, skipped check should suffice, if at all string changes, worst case it will marked as failure. Shouldn't be an issues since testResult itself was not a skipped status.
What is this about?
Reports skipped and hook-aborted tests in the CLI (gRPC) flow. Today, tests that never reach
beforeTest/afterTestemit no events at all — staticit.skip,this.skip()insidebefore/beforeEachhooks, and suites aborted by a failedbeforehook. The legacyListener → api/v1/batchpath these flowed through previously is not functional in the CLI pipeline (its own usage stats reportskipped: {triggeredCount: N, sentCount: 0, failedCount: N}), so such tests are invisible on the Test Observability dashboard and their Automate sessions are never linked to the build (the dominant generator ofsession_linking_issue_buildin TRA build-stability metrics).Changes (merged via #55, reviewed there; this PR promotes the staging branch to
main):cli/skipReporter.ts(new): routes skipped tests through the TestFramework tracker using the sameINIT_TEST → TEST PRE → LOG_REPORT POST → TEST POSTsequenceafterTestuses; serialized through a single chain (wdio doesn't await reporter hooks) with started/reported dedup preventing double-reporting runtimethis.skip().reporter.ts:onTestSkipCLI branch with resolved spec file (events without a file location are rejected downstream) and suite chain viactx.service.ts:afterHookCLI branch — on a non-passed BEFORE_ALL/BEFORE_EACH/AFTER_EACH hook, reports the suite's undetermined tests as skipped (port of the legacy insights-handler cascade);beforeTestmarks started tests.cli/frameworks/wdioMochaTestFramework.ts: hook results mapped from realFrameworks.TestResultfields incl. the sync-skip error shape — genuine before-hook failures fail the build, deliberatethis.skip()hooks stay green.util.ts/testOps/listener.ts: batch-upload failures carry HTTP status + body snippet instead of a misleadingUnexpected end of JSON input; listener logs the drop reason.Validation (live end-to-end, 6-spec matrix, verified via the Observability API): before — build
passed,{passed: 3-4}, skipped/aborted suites absent, sessions unlinked. After —{passed: 4, failed: 1, skipped: 8}, all 12 tests reported exactly once, every session attributed, buildfailedonly from the genuinely-thrown hook. Closed-loop proof under TRA stability semantics: 3 without-fix builds FLAGGED assession_linking_issue_build, 3 with-fix builds STABLE.npm run buildgreen; 6 newskipReporterunit tests. Regression coverage in BStackAutomation (SDK/apiwdio_mocha, markersdk_6987). v8 line: #68.Related Jira task/s
N/A
Release (mandatory for every PR — required for the
ready-for-reviewlabel)Version bump: (required — tick exactly one)
Release notes type: (optional)
Release notes (customer-facing): (optional but encouraged)
it.skip,this.skip()in before/beforeEach hooks) and suites aborted by a failed before hook not being reported to Test Observability when using the CLI.Release notes (internal): (required — engineer-facing; what actually changed / why)
cli/skipReporterrouting skipped/aborted tests through the gRPC tracker (INIT_TEST → TEST PRE → LOG_REPORT POST → TEST POST), serialized and deduped against started tests;onTestSkipCLI branch +afterHookcascade port the legacy skip paths that are dead againstapi/v1/batchin CLI mode.TestResultfields (incl. the sync-skip error shape) so failed before-hooks fail the build and deliberate skips don't.response.okguard (failures carry HTTP status instead ofUnexpected end of JSON input); listener logs batch-drop reasons.Checklist
PR Validations
Run Tests: Comment RUN_TESTS to trigger sanity tests.
🤖 Generated with Claude Code