feat(v8): platformise CucumberJS on the CLI/binary flow - #207
Open
AdityaHirapara wants to merge 12 commits into
Open
AdityaHirapara wants to merge 12 commits into
AdityaHirapara wants to merge 12 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>
|
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: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
…platformisation-v8 # Conflicts: # packages/browserstack-service/src/cli/modules/testHubModule.ts
AdityaHirapara
marked this pull request as ready for review
September 18, 2026 05:59
AdityaHirapara
requested review from
vivianludrick and
yashdsaraf
and removed request for
a team
September 18, 2026 05:59
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
Collaborator
Author
Per-File Confidence
Change map (generated deterministically from the diff)graph LR
subgraph nnode_agent["node-agent"]
npackages_browserstack_service_src_cli_frameworks_wdioCucumberTestFramework_ts["wdioCucumberTestFramework.ts<br/>~651 lines"]
npackages_browserstack_service_tests_cli_frameworks_wdioCucumberTestFramework_test_ts["wdioCucumberTestFramework.test.ts<br/>~266 lines"]
npackages_browserstack_service_src_service_ts["service.ts<br/>~215 lines"]
npackages_browserstack_service_src_cli_modules_automateModule_ts["automateModule.ts<br/>~128 lines"]
npackages_browserstack_service_tests_cli_modules_automateModule_test_ts["automateModule.test.ts<br/>~128 lines"]
npackages_browserstack_service_src_cli_modules_testHubModule_ts["testHubModule.ts<br/>~17 lines"]
npackages_browserstack_service_src_cli_index_ts["index.ts<br/>~15 lines"]
npackages_browserstack_service_src_cli_eventDispatcher_ts["⚠ eventDispatcher.ts<br/>~13 lines"]
n_changeset_pr_207_md["pr-207.md<br/>~7 lines"]
npackages_browserstack_service_src_cli_modules_accessibilityModule_ts["accessibilityModule.ts<br/>~6 lines"]
npackages_browserstack_service_src_cli_cliUtils_ts["cliUtils.ts<br/>~2 lines"]
npackages_browserstack_service_src_cli_frameworks_constants_testFrameworkConstants_ts["testFrameworkConstants.ts<br/>~1 lines"]
end
↻ 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 |
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?
Moves CucumberJS + WebdriverIO on the WDIO v8 line off the legacy in-SDK reporting flow onto the CLI/binary gRPC flow, to functional parity with legacy.
Cucumber joins
CLISupportedFrameworksand reports through a newWdioCucumberTestFrameworkclass, with the shared dispatch gates widened for it. Two parity defects found during verification are fixed here as well.⚠ The gate flip alone is unsafe and is not what this PR does.
service.ts's cucumber hooks are unguarded where mocha's are guarded, so flippingCLISupportedFrameworkswithout addingisRunning()guards in the same change reports every scenario twice — over two transports, under two uuids, with no error and a build that looks populated. The gate and the guards land together here.Zero binary changes — the thick layer already supported this combination.
Verification (7 checkpoints, 50 parity rows, full BSA suite vs matched legacy)
Known limits, stated rather than smoothed: turboscale could not actually be exercised (no grid provisioned on the test account, so runs degrade to regular Automate); Percy matches legacy on what its two tests assert but that is not a broader Percy claim; and the legacy comparison arm ran 8.51.0 in three of five directories, a confound in legacy's favour.
Related Jira task/s
SDK-7606 (epic SDK-7053)
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)
sessionNameFormatbeing ignored, so custom session names now apply on the CLI flow.Release notes (internal): (required — engineer-facing; what actually changed / why)
CLISupportedFrameworksnow includescucumber; scenarios and hooks report via a newWdioCucumberTestFramework, and the shared dispatch gates inservice.tsare widened for it. TheisRunning()guards ship in the same change — without them the unguarded cucumber hooks double-report every scenario across both transports.sessionNameFormatwas silently dropped on the CLI flow: it is a function, soJSON.stringifyremoved it from the config round-tripped through the binary, andautomateModulethen overwrote the correct name with the raw suite title.automateModulenow resolves the formatter from the live in-process service options (injected, not imported — importingcli/index.tsback closes an ESM cycle), keeping all session naming in one place. Also fixes the same loss forwdio_mocha, which never wrote the formatted name at all.beforeFeatureawaited a session-update REST call before emitting the CLI feature-start event, so any rejection of that call (a 4xx/5xx) aborted the hook and Observability lost the feature path. The event is now emitted first; it raises no wire event, so ordering is unaffected.Checklist
PR Validations
Run Tests: Comment RUN_TESTS to trigger sanity tests.