feat(SDK-7063): report bail-skipped tests as skipped - #122
Draft
harshit-browserstack wants to merge 6 commits into
Draft
feat(SDK-7063): report bail-skipped tests as skipped#122harshit-browserstack wants to merge 6 commits into
harshit-browserstack wants to merge 6 commits into
Conversation
Tests that never run because of `bail` were absent from the Test Run entirely. Report them as `skipped` so the report accounts for every declared test. WebdriverIO has two unrelated bail mechanisms, handled separately: - `mochaOpts.bail` halts the failing spec. `service.afterTest` walks `test.ctx.test.parent` to the spec's root suite and reuses the existing `reportSuiteSkipped` cascade, so sibling describes are covered. Gated strictly on mocha's own option -- under wdio's `bail` every in-spec test still runs, so triggering on it would report tests that are about to execute. - wdio's `bail: <n>` counts failed spec files and stops scheduling more. `launcher.onComplete` diffs dispatched specs against the run's spec list and enumerates the rest via mocha's declaration pass, without executing any test body. Launcher-side reporting only happens when bail demonstrably fired (`failedRunners >= bail`, counted the way wdio counts it -- a failed spec with retries left is requeued, not counted). It is skipped entirely under sharding, when capabilities define their own specs/exclude, and for excluded specs. A spec can fail to run for many reasons unrelated to bail, and reporting one of those as skipped would corrupt the report; under-reporting is recoverable, phantom skips are not. Events are relayed through the binary's EnqueueTestEvent RPC. The launcher's TESTHUB_JWT is the binary's account token on the CLI path and carries no build claim, so posting to the collector directly returns 401; the binary holds the build-scoped credential. The RPC already existed in the proto -- only the client wrapper was missing. Reported identity matches `getUniqueIdentifier` so a test reported skipped and the same test on a later run share one identity. Covers the CLI/gRPC path only. Enumerating never-started specs re-executes their top-level code.
Wire evidence showed a test that runs emits `name` and `identifier` as the bare title, with only `scope` carrying the parent: identifier: 'MA-TC1 passes' scope: 'Multi Spec A - MA-TC1 passes' Skipped tests reported for never-launched specs were setting `identifier` to the parent-qualified form, so the same test reported skipped and later run would not have matched on that field. Verified on build bpt0jvulwfonuoucepdu892xhzborork2hy7tgqb: executed and skipped tests now emit identical conventions for all three fields.
The customer's config is `bail: 0` with `mochaOpts.bail: true`, so only Mocha's in-spec bail applies to them. WebdriverIO's spec-file-level bail is disabled in their setup and the launcher-side reporting never runs. Drop the never-launched-spec work from this PR so the change that unblocks them is small and built on the already-proven `reportSuiteSkipped` path. That work is preserved on `feat/sdk-7063-unlaunched-specs` and can land separately on its own review. Removes: specEnumerator, unlaunchedSpecReporter, the launcher hooks, the EnqueueTestEvent client wrapper, and the mocha devDependency they needed. README and changeset narrowed to match.
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
1 participant
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 is this about?
With
mochaOpts: { bail: true }, Mocha halts a spec on its first failure and the remaining tests in that file never run. Today those tests are not reported in any state — they simply disappear from the Test Run, so the report silently under-counts. This PR reports them asskipped.Tests in sibling
describeblocks in the same file are covered too, since Mocha's bail aborts the whole spec rather than just the failing block.How it works
afterTestalready fires for the failing test. From there we reach the live Mocha suite viatest.ctx.test.parent, walk up to the spec's root suite, and hand it to the existingreportSuiteSkippedcascade — the same code path already used when abeforehook fails and Mocha silently drops the rest of the suite. That is the identical shape of problem, so this is a second trigger on a proven path rather than new machinery.Mocha builds its full suite tree at file-load time, before running anything, so tests that never execute are already known and carry
state === undefined.Deliberately gated on
mochaOpts.bailonlyWebdriverIO's top-level
bailis a different setting: it counts failed spec files and stops scheduling further ones, and is never forwarded to Mocha. Under it, every test in the running spec still executes — so triggering on it would report tests that are about to run. Reporting is also skipped while a retry is still queued, for the same reason.Not covered here
Tests in spec files that never started (WebdriverIO's
bail) are not reported. That is a separate, launcher-side change and is kept out of this PR to keep the blast radius small; the work exists onfeat/sdk-7063-unlaunched-specsand can land on its own review.Also scoped to the CLI/gRPC reporting path — the legacy direct-HTTP path and multi-remote runs are unchanged.
Related Jira task/s
Release (mandatory for every PR — required for the
ready-for-reviewlabel)Version bump:
Release notes type:
Release notes (customer-facing):
bailhalted a spec are now reported asskipped, instead of being left out of the report entirely — so a Test Run accounts for every test in that spec.Release notes (internal):
service.afterTest: whenmochaOpts.bailis set and a test fails, resolve the spec's root suite fromtest.ctx.test.parentand run the existingreportSuiteSkippedcascade over it. Root-walk (rather than starting at the failing test's own suite) is what covers siblingdescribeblocks.mochaOpts.bailwith Mocha's own truthiness rather thanisTrue(), which is a strict'true'string compare and would miss a numericbail: 1.config.bail: in-spec tests still run under it, so reporting them would double-count. Also skipped whileresults.retries.attempts < limit.Checklist
PR Validations
Run Tests: Comment RUN_TESTS to trigger sanity tests.
Testing evidence
Verified against real BrowserStack builds with Observability assertions, not just console output. Each scenario was run twice — once on the published SDK, once with the fix.
describe, tests in a siblingdescribeThe sibling-
describerun also confirmed the reportedscopesstay per-suite (['Suite B']) rather than being flattened, and that skipped tests carry the same Automate session id as the tests that ran — so they attribute to the session instead of orphaning.Identity fields match what an executed test emits (
name/identifier= bare title,scope=parent - title), so a test reported skipped and the same test on a later successful run are recognised as one test.4 unit tests added;
service.test.tsfailure count unchanged frommain(those are pre-existing and environmental —vi.mock('fetch')is a no-op, so they make real network calls with dummy credentials).