Merge SDK-7606/wdio-cucumber-platformisation-v8 - #232
Open
bsautomation wants to merge 19 commits into
Open
bsautomation wants to merge 19 commits into
bsautomation wants to merge 19 commits into
Conversation
Add 'cucumber' to CLIUtils.CLISupportedFrameworks, give setupTestFramework an explicit 'webdriverio-cucumber' branch (previously no else arm, so testFramework stayed null and every event was dropped without an error), and widen the field to the TestFramework base type every consumer already uses. Membership alone is not safe to ship. The legacy InsightsHandler -> Listener -> api/v1/batch transport is gated only on TESTOPS_BUILD_COMPLETED and BROWSERSTACK_TESTHUB_JWT, both of which the CLI flow sets itself, and none of the cucumber lifecycle hooks were CLI-guarded. Opening the gate alone would keep the legacy path POSTing TestRunStarted/TestRunFinished/CBTSessionCreated under the binary-issued JWT alongside the tracker, reporting every scenario twice under two uuids with no error. The guards for the emitting hooks therefore land here, not in a follow-up. beforeStep/afterStep are deliberately left unguarded: they emit nothing and only build the step list afterScenario reads back via hasTestStepFailures(), a read that is not CLI-gated and feeds the process exit code.
…rk class WdioCucumberTestFramework goes from an inert stub to the cucumber event tracker. It extends the base TestFramework, not the mocha one: WDIO never calls beforeTest/afterTest for cucumber and its hook invocations carry no title, so mocha's INIT_TEST/TEST/hook boundaries have no source here. A scenario raises TEST/PRE at beforeScenario and TEST/POST at afterScenario. That is forced rather than chosen — the binary's WDIO language index dispatches only on TEST and ^(BEFORE_|AFTER_), and silently ignores anything else, so a scenario-specific state would have produced a green build with no tests. Raising TEST is also what fills automateModule's sessionMap, which is why the session is now named and marked. Hooks are classified from a state machine over the class's own cucumber bookkeeping, never from a title. util.ts getHookType() is left alone: it matches Mocha's quoted titles and widening it would change mocha and jasmine too, so service.beforeHook/afterHook discriminate on the framework instead. Before this, every cucumber hook boundary threw inside an awaited WDIO hook and the hook was lost with a logged stack trace in a green build. The step-depth counter is deliberately not reset per scenario, matching the legacy handler: one missed afterStep classifies every later AFTER_EACH as unreported for the rest of the run, and reproducing that is parity. bdd_meta_info.feature.path is sent ABSOLUTE. The binary re-bases it against the project path for file_name/location and the git root for vc_filepath; pre-relativising on this side makes both fields depend on the binary's own cwd (SDK-7233). A failed BeforeAll abandons the whole feature, so every scenario it never reached (Rule-nested included) is reported skipped. Those rows are built as detached instances and sent straight to TestHub — routing them through the observers would rename the session, stop accessibility and run a Percy teardown per row. A hook finish with no recorded start drops both the finish and the cascade, as the legacy path does. accessibilityModule now owns the per-scenario Web A11y scan on this flow. It observes the TEST states raised above, and leaving the legacy handler live alongside it ran sendTestStopEvent twice per scenario against the same test run uuid — while that handler is only half-initialised here, since service.before() never calls its before(). preferScenarioName was a silent no-op on this flow: the only _updateJob carrying the name is gated off while the binary is up, and the session had already been named after the feature. It is written explicitly now, after the EXECUTE/POST tracker call rather than racing it. Unreachable for mocha and jasmine, whose scenario array is never populated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…on and session verdict Hook-scoped logs now carry their hook's state on the log record, so the binary keys them to hook_run_uuid instead of stamping a hook uuid into test_run_uuid. automateModule gains a build-level hook observer, so a failed BeforeAll/AfterAll fails the session as legacy's after() did; keys per-scenario results on the scenario rather than the feature name; and treats a skipped scenario as legacy's _failureStatuses does. All cucumber-gated — mocha's path is unchanged. accessibilityModule's scan decision passes the world through, restoring the tag-aware filter cucumber has on legacy.
…alse setSessionName suppresses the session NAME, not its registration. Skipping registration left onAfterExecute with nothing to status-mark, so a failing run under setSessionName: false reported no status at all where legacy marked it — its after() status block gates on setSessionStatus alone. Applies to every framework on the CLI flow, wdio_mocha included: a mocha session that is unmarked today becomes status-marked. The name stays suppressed, since onAfterExecute's naming call is guarded on both the flag and a non-empty name.
…observer failures An Automate-only CLI run (every TestHub product off) never registered its driver: service.before() raised AutomationFrameworkState.CREATE/POST only inside the shouldProcessEventForTesthub block, so webdriverIOModule never recorded the session id or capabilities. automateModule's sessionMap stayed empty and the Automate session was left unmarked, where the legacy flow marked it. Session tracking does not depend on TestHub, so the raise is now gated on the CLI being up and nothing else. On cucumber the same gap also failed every scenario: testHubModule's session event dereferenced the missing session id, re-threw, and — because the event was neither awaited nor caught — surfaced as an unhandled rejection inside the user's own cucumber Before hook, skipping every step. The event is now awaited, and eventDispatcher gives each observer its own boundary so one module's failure neither aborts the observers registered after it nor escapes into the framework hook that raised the state. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…call beforeFeature awaited _setSessionName before reaching onFeatureStart, so any rejection of that session-update call aborted the rest of the hook and the CLI framework never saw the feature. Observability then lost the feature path for that feature (root file_path empty). Reordering is safe because the two statements are independent: onFeatureStart only assigns local bookkeeping from its own arguments and raises no wire event, and _setSessionName sets nothing it reads. Cucumber-only hook, so mocha and jasmine are unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
sessionNameFormat is a function, so JSON.stringify drops it and it never reaches the binary. automateModule, which is written to apply it, therefore fell back to the raw suite title and PUT that over the correctly formatted name the SDK had already written. Carry a sessionNameFormatProvided boolean across the boundary instead, and have automateModule defer its name write when a formatter exists, leaving the SDK — which holds the live function — as the naming authority. beforeSuite now also names mocha on the CLI flow in that case, where previously nothing applied the format at all. Nothing changes when sessionNameFormat is unset. The guard sits at the single write site, so lastTestName, the cucumber result key and percy's session-name read are unaffected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Supersedes the previous approach, which made the legacy-side _setSessionName the namer whenever sessionNameFormat was set. That split session naming across two code paths on the same flow and reached into the legacy path to do CLI work. automateModule runs in the SDK process, so it never needed the formatter to survive the gRPC/JSON round trip -- it only needed to stop reading the round-tripped copy, where function-valued keys are silently dropped. It now takes the live service options by injection and resolves sessionNameFormat from them, so every naming decision stays in one place. Injected rather than imported: cli/index.ts constructs this module, so importing it back closes an ESM cycle (it surfaced as "Class extends value is not a constructor" in wdioMochaTestFramework). Adds two tests. The first shapes testContextOptions as the binary really returns it -- with no sessionNameFormat key -- and asserts the formatted name still reaches the session; it fails against the previous code, so it pins the behaviour rather than merely passing alongside it. The second asserts the suite title is used when no formatter is configured, guarding the claim that nothing changes for the unset case. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…platformisation-v8 # Conflicts: # packages/browserstack-service/src/cli/modules/testHubModule.ts
service.ts decided this on the CLI path and wrote the name itself, which split session naming across two code paths on the same flow -- the module named the session at TEST/PRE from the feature title, and service.ts renamed it afterwards from its own scenario list. automateModule now counts non-skipped cucumber scenarios and applies the rename at EXECUTE/POST, the first point where "exactly one ran" is knowable. service.ts carries preferScenarioName on the cucumber TEST/POST event and keeps the rename only for the legacy path. This matches the shape the v9 line already uses. The scenario is tracked before the skipSessionStatus return: setSessionStatus false opts out of the status, not of the rename. Also completes the TestFramework test mock with getState. isCucumberInstance was previously reached only through a short-circuit, so the missing mock method never surfaced; it is called unconditionally now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…sation-v8' into SDK-7606/wdio-cucumber-platformisation-v8
Addresses two findings from the PR review.
service.ts gained the cucumber CLI dispatch without direct unit coverage —
tests/service.test.ts never mocked BrowserstackCLI, so all of its existing
cases ran the legacy branch only. Adds the CLI side:
- beforeScenario / afterScenario raise TEST/PRE and TEST/POST on the
cucumber framework and leave the legacy handlers alone
- afterScenario still runs the legacy Percy teardown when the binary is up
but holds a non-cucumber framework
- the BEFORE_ALL skip cascade fires on a failed BEFORE_ALL only, not on a
passing one and not on BEFORE_EACH
- _cucumberTestResult / _cucumberTestView mapping, folded into the existing
afterScenario suite since that is their only call site
Also documents why the !isRunning() guard was dropped from afterScenario's
legacy Percy call: the CLI branch returns before it, so percyModule's
observer and this teardown are mutually exclusive. The guard additionally
skipped Percy when the binary ran a non-cucumber framework, which raises no
TEST/POST either — the teardown was lost there rather than duplicated.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e rows Two parity defects found in PR review. preferScenarioName never renamed the session on the CLI flow. onAfterTest read the scenario name from `test.title`, which _cucumberTestView leaves undefined on purpose — legacy calls _setSessionName(feature.name) with no test argument, so sessionNameFormat has to receive undefined as its fourth argument. Populating `title` would have changed what every sessionNameFormat callback is handed, so the name is read off the live `world` instead; this observer runs in-process, so the object has not been through the binary's JSON round-trip. Automate results keyed on `test.fullName`, the raw pickle name, which every Examples row of an outline shares when the outline title carries no placeholder. Those rows collapsed last-write-wins, so a failing row followed by a passing one reported the session passed. Cucumber now keys on the scenario's own uuid; mocha's key is unchanged. The existing preferScenarioName test passed a fabricated `test.title`, bypassing _cucumberTestView, so it stayed green while the integrated path was dead. It now uses the real view shape and reads the name from `world`, and the blanket getState mock is keyed so it stops answering every state identically. Adds an outline-collision regression test. Both tests fail against the unfixed module. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
SDK-4177. browserCommand resolves the active test through insightsHandler's `_tests` map and returns before the SDK-6277 screenshot branch when it finds no entry. Legacy seeded that map from insights-handler's beforeScenario, which the CLI branch skips, and TestReporter's map is keyed on `testStats.fullTitle` / `getHookIdentifier` — neither matches `getUniqueIdentifierForCucumber(world)`. So screenshots were dropped for cucumber on the binary flow where legacy forwarded them. beforeScenario now reads the scenario uuid after the TEST/PRE event — the uuid is minted by loadScenarioData during that event, so reading earlier would seed the previous scenario's — and hands it to setTestData. The lookup is guarded: getState dereferences the instance, so an absent tracked instance would have thrown out of the customer's hook rather than just losing the attribution. setTestData accepts a world and lets it through to seed `_tests`. The key moves to getIdentifier(), which for every non-pickle test returns getUniqueIdentifier(test, framework) — the previous key unchanged, so mocha and jasmine keep their behaviour. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
On the CLI flow service.before() merges the capabilities the binary hands back, and those carry the SDK's own options. NOT_ALLOWED_KEYS_IN_CAPS is the only strip between that merge and the hub, and it did not list preferScenarioName, so the hub rejected the session outright — `additional properties ["preferScenarioName"] outside of the schema` — and the run exited 1 with no session where the legacy flow, which never calls the binary, exited 0. The leak is framework-agnostic, so this changes mocha and jasmine too: an option that is a no-op for them stops crashing the run. preferScenarioName is cucumber-only — _scenariosThatRan is pushed only in afterScenario, so the `length === 1` gate can never fire for mocha. Unblocks the three cucumber session-naming parity rows, which could not be measured while no session was ever created. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The earlier strip covered preferScenarioName only. sessionNamePrependTopLevelSuiteTitle
reaches the outgoing capabilities by the same route and the hub rejects it the same way
("additional properties [...] outside of the schema"), which is what kept the cucumber
session-name parity row unmeasurable. sessionNameOmitTestTitle is listed alongside it:
same family, same route into caps, not a W3C capability.
Both are read from testContextOptions, so stripping them from the outgoing capabilities
does not disable either option.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The binary re-bases test_file_path and location itself but never touches the bdd_meta blob, so the absolute path we put there reached the dashboard verbatim, home directory included — where the legacy flow published a repo-relative path read off the cucumber world's gherkinDocument.uri. Adds featureUriForMeta(), used only for the meta blob; the file-path pair stays absolute because the binary owns re-basing those. The existing test asserted the meta path equalled the absolute feature uri, which encoded the defect; it now asserts cwd-relative and pins the file path as still absolute. Verified at unit level and by source inspection. The O11Y read-back that produced the original evidence for this defect was not reproduced — the testRuns endpoint returned no rows for the fix run — so dashboard confirmation is still outstanding. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…lob too The BEFORE_ALL cascade builds its own bdd_meta blob inline rather than going through buildBddMetaInfo, so the previous fix missed it and cascade rows kept sending the absolute feature path into a blob the binary never re-bases. Per-field contract, now consistent at all five emit sites: test_file_path and location go out absolute because the binary re-bases them; every bdd_meta feature.path goes out cwd-relative because the binary leaves that blob alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
bsautomation
requested review from
dandonarahul2002 and
shivam5643
and removed request for
a team
September 24, 2026 15:14
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited), Workspace UI (inherited) Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
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
2 participants
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.
Merge into sdk_v8_pre_prod :by automationbs