SDK 7414 wdio cucumber platformisation - #191
Open
AdityaHirapara wants to merge 27 commits into
Open
Conversation
Adds 'cucumber' to CLISupportedFrameworks, which is the single gate both the launcher and the worker read. The binary side already registers 'WebdriverIO-cucumber' and the name reaches it unchanged: setFrameworkDetail takes WDIO_NAMING_PREFIX + config.framework verbatim, so no session-start branch is needed here. setupTestFramework's if had no else, so an unmatched name left testFramework null and every CLI event no-opped without an error anywhere. The framework class lands next, so cucumber takes that arm for now — log it rather than leave the silence. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… own states Cucumber's unit of work is the scenario, and every cli/modules/* observer subscribes to TestFrameworkState.TEST — so a scenario raises TEST/PRE at beforeScenario and TEST/POST at afterScenario, and the whole module set works unchanged. Extends the base TestFramework, not WdioMochaTestFramework: WDIO never calls beforeTest/afterTest or titled hooks for cucumber, so mocha's INIT_TEST/TEST/hook boundary semantics have no source here. - Hooks classify via a _cucumberData state machine, not util.ts getHookType() — a cucumber hook carries no title and BeforeAll/AfterAll pass no hook object at all, so getHookType would throw the moment the flow gate opened. Step-scoped hooks stay unreported. - Scenario results set test_result_at. Without it testHubModule marks the test deferred and waits on LOG_REPORT, a state cucumber never emits. - Duration comes from cucumber's protobuf Duration, not an ended_at - started_at delta; the failure backtrace is one element, not mocha's two; tags keep their leading '@'; identifier stays the raw pickle name while name/scope carry the examples qualifier. - The feature path is sent absolute — the binary re-bases it (SDK-7233). - Logs route to the open hook's uuid when one is in flight, else the scenario's. Mocha's path is unchanged: both service.ts hook edits add an instanceof arm ahead of the existing block, and the factory's mocha branch still returns first. SDK-7414
Phase 7 of SDK-7414. 26 shared dispatch sites were enumerated before any edit; 19 needed no change and are recorded as such. accessibilityModule.onBeforeTest now calls shouldScanTestForAccessibility in its 6-arg form, passing the cucumber world and the tag-filter flag. The 3-arg form matches include/exclude tags against the test title, so a cucumber user's tag filters were silently ignored and every scenario was scanned. Only the call arity changed; the helper itself is untouched, and args.world is populated solely on the cucumber path, so mocha and jasmine keep the exact title-matching behaviour. wdioCucumberTestFramework stamps hook_scope, hook_retries and hook_duration onto the hook record. The binary cannot derive any of them from the event: a hook's scope is the feature name while the event carries the examples-qualified scenario name, and BEFORE_ALL/AFTER_ALL fire on an instance with no scenario data at all.
On the CLI flow the Automate session status comes solely from the result view service.afterScenario builds — service.after()'s _failReasons path is gated on the binary not running. That view ignored testObservabilityOptions.ignoreHooksStatus, so a scenario that failed only in a hook marked the session failed where the legacy flow marked it passed. Reuses the framework class's own hasStepFailures(), the same predicate the observability result already applies, so both surfaces of the flag agree. InsightsHandler.hasTestStepFailures is unusable here: it reads _tests, which the CLI branch never populates. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…and custom tags Four product fixes on the CLI/binary flow, all in cli/modules/*. Turboscale sessions were marked against the Automate REST API: the URL was a two-way ternary (app-automate / automate) and the verb was always PUT, where turboscale needs PATCH against /automate-turboscale/v1/sessions. Both markers now share one three-way resolver, so the path and the verb cannot drift apart. wdio_mocha carried the same defect and is repaired by the same change. A failing BeforeAll/AfterAll produced no scenario result, so it could never enter the per-test map onAfterExecute aggregates and the session came back passed. Cucumber-gated, and it honours ignoreHooksStatus the same way the scenario surface does. The scenarios a failed BeforeAll abandons now reach Test Observability as skipped rather than vanishing. The cascade publishes straight to TestHub, as the legacy listener did, so it does not rename the session or fire a scan or a Percy teardown per skipped row. TestHub's v2 pipeline builds the test row from the start event, so each row sends a start followed by the skip; a lone TestRunSkipped is accepted and counted in no bucket. accessibilityModule.onHookStart re-opened the scan gate for every framework, resting on beforeEach preceding beforeTest. Cucumber inverts that ordering, so the write landed last and scanned every scenario regardless of the tag filters. Narrowed to mocha, matching the legacy handler. setCustomTags had no framework gate and had quietly started working for cucumber, where the legacy handler warns and no-ops. Gated back. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…th no scenarios A failing BeforeAll under ignoreHooksStatus left the Automate session unmarked and so invisible on the dashboard. Legacy marks it failed through the !_specsRan arm of after(), which the flag never reaches. Keys the skip on a scenario result having been recorded, not on the flag. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… the order Two defects collapsed a cucumber feature's scenario results into a single verdict, so a feature whose last scenario passed reported a passed Automate session however many earlier scenarios had failed. Legacy marks it failed. automateModule.onAfterTest derived one `name` and used it for two different jobs: the session name and the testResults accumulator key. Cucumber's test view carries a fullName, so `name` stayed the Feature title — shared by every scenario — and the Map collapsed N scenarios into one last-write-wins entry. Key the accumulator on fullName where the framework supplies one. Mocha leaves fullName undefined, so its key is unchanged and its path is byte-identical. _cucumberTestResult also read the observability passed/failed collapse for session status. The two views are not the same: UNDEFINED / AMBIGUOUS / UNKNOWN fail the session on legacy while still reporting to Observability as skipped, and PENDING joins them under cucumberOpts.strict. Read _failureStatuses. Verified with an ordering probe — the same two scenarios, order the only variable. Both arms now report failed with legacy's own reason for the shared failure mode, where the failure-first arm previously reported passed. SDK-7414 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The binary types AccessibilityCapability.value as a proto string, so the object-valued goog:chromeOptions capability reaches the SDK as "[object Object]" on the CLI flow where the HTTP launch response delivers a real object. AccessibilityScripts.update() stored that string verbatim, and the non-BrowserStack-infra accessibility path in the launcher then wrote it into a W3C capability, which the hub rejects outright: The property '#/alwaysMatch/goog:chromeOptions' of type String did not match the following type: object No session is created, so every assertion downstream fails for want of one. Accept both shapes and drop anything that is not an object, so a value that cannot become a capability is never written as one. Applying this in update() rather than at the response site also covers the value read back from a commands.json poisoned by an earlier run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…of caps Parity row 40 was failing twice over, independently. The option reached bstack:options. cliUtils.getBinConfig passes the service options through verbatim, and the binary's getBstackOptions() copies every config key it does not recognise into the outgoing W3C payload, so the hub rejected the session before it existed: The property '#/alwaysMatch/bstack:options' contains additional properties ["preferScenarioName"] outside of the schema when none are allowed Excluded on the SDK side rather than in the binary's EXCLUDED_CAPS. preferScenarioName is a wdio-service option, not a BrowserStack capability, so NOT_ALLOWED_KEYS_IN_CAPS is both the narrower blast radius (WDIO frameworks, not every language SDK) and the more correct home - includeTagsInTestingScope is already there for exactly this reason. turboScaleOptions belonged in EXCLUDED_CAPS because that key genuinely is a capability. This half is not cucumber-specific: the same leak broke preferScenarioName on wdio_mocha, and fixing it here repairs that too. And the gate had no implementation on this flow. service.after() sets _fullTitle, but every _updateJob call site that consumes it is gated !BrowserstackCLI.isRunning(), so the name never moved; automateModule owns the name here and was still applying the feature title. after() now pushes the rename to a new automateModule.overrideSessionName(), which writes sessionMap and re-flushes - flushSessionName's appliedName de-dupe keeps a no-op override free, and skipSessionName still wins, matching legacy omitting `name` from its _updateJob payload under setSessionName: false. Legacy's `=== 1` exactness is reproduced, not widened: the new branch sits inside the existing guard. wdio_mocha cannot reach it - _scenariosRanCount and _lastScenarioName are written only by cucumber's afterScenario - which the discriminating test pins on identical input. Separately, _cucumberTestResult() now mirrors legacy afterScenario()'s failure message. The statuses that only fail a session via _failureStatuses carry no world.result.message - PENDING under cucumberOpts.strict, equally UNDEFINED and AMBIGUOUS - so automateModule fell back to 'Unknown Error' where legacy reports `Some steps/hooks are pending for scenario "..."`. A failure that carries a real message is unaffected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Readability only; no behaviour change beyond the session-id guard noted below. `reopensGateForHook` named the effect rather than the condition — renamed to `isMocha`, matching the file's neighbours (`isBrowserstackSession`, `isPreTestWindow`). `sessionId !== undefined && sessionId !== null` collapsed to a truthiness check. Line 287 of this file already guards the same value that way, so the long form was the outlier. The substring match stays. `KEY_TEST_FRAMEWORK_NAME` holds the vendor-qualified name — `WebdriverIO-mocha`, observed in the wire payload — so `=== 'mocha'` would never match and the mocha hook window would silently stop scanning. `testHubModule` gates on the same value the same way. Comment cut from eleven lines to five, keeping only what the code cannot say: that this mirrors legacy's `_framework === 'mocha'` gate, and why it has to be mocha-only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comments only; no code change. The BEFORE_ALL cascade doc dropped its parity-row and ticket citations and the "one call site" note, keeping what the code cannot say: what cucumber does to a feature when BeforeAll throws, and why the cascade goes straight to TestHub instead of through trackEvent. Two comments overstated a constraint. Both said automateModule "cannot read" an option, which is only true of `this.config` — the binary-supplied one. A module can reach service options via `BrowserstackCLI.getInstance().options`, as accessibilityModule already does. They now give the actual reason for the placement instead of implying an access restriction that does not exist. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The rename lived in service.after(), which reached into the module through a public overrideSessionName(). It does not need to: the module already sees every scenario event with its status, so it can keep the tally itself and apply the name on the path that already flushes it. onAfterTest counts non-skipped scenarios behind the existing isCucumberInstance gate; onAfterExecute applies the scenario name just before the final flushSessionName sweep, so skipSessionName keeps working through that guard. service.ts loses the module import, the try/catch and the cross-component ordering dependency on after() running before the session closes. The flag rides the scenario event rather than being read from the module. A module CAN reach service options via BrowserstackCLI.getInstance().options — accessibilityModule does — but importing the CLI singleton here is a cycle (automateModule -> index -> testHubModule -> wdioMochaTestFramework), and it breaks class construction at load. Riding the event is the route ignoreHooksStatus already takes. Legacy's `=== 1` exactness is reproduced, not widened; both tests were falsified against a deliberately broken guard. The five service-level cases move to the module suite, three new ones cover the event seam, and the four _cucumberTestResult reason cases that shared the old file are kept. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comments only; no code change. Six comments pointed at the SDK-7414 parity table by row number. That table is a migration working artifact, not something a future reader of this file will have, so the pointers were about to become dangling references. Each now states the constraint directly instead of citing where it was recorded. Jira ticket references are kept — SDK-7233 is durable and lookupable, which a row number is not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comment only; no code change. Fourteen lines to seven, keeping why the instances are detached and why they bypass the observer set. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`featurePath()` fed two consumers with one absolute value, and only one of them
wants it.
`test_file_path` must stay absolute: the binary's cucumber module re-bases it
(`path.relative(session.pathProject, absoluteTestFilePath)`), so a
pre-relativised value there is resolved against cwd first and both `file_name`
and `vc_filepath` come out wrong (SDK-7233). Unchanged.
`bdd_meta_info` is never read anywhere in the binary's node path, so it reaches
the dashboard verbatim. Legacy builds it as `feature = { path:
gherkinDocument.uri, … }` — the raw uri — while relativising separately for
`file_name`/`location`. Ours sent the absolute path, so the dashboard showed
`/Users/<name>/…/features/x.feature` where legacy showed `features/x.feature`,
publishing the developer's home directory.
Both bdd-meta sites now read `cucumberData.uri` directly rather than taking a
path parameter, so no caller can pass the absolute value back in.
Tests assert both shapes and were falsified: restoring the absolute value fails
the two meta assertions and leaves the test_file_path one green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Renamed wdioCucumberTestFramework.featurePath.test.ts to wdioCucumberTestFramework.test.ts. No content change. The class had no test file at all, so a concern-scoped name was the wrong shape: the repo uses <subject>.test.ts as the main suite and adds <subject>.<concern>.test.ts alongside it, and the next test for this class now has an obvious home instead of spawning a second file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Renamed automateModule.phase8.test.ts to automateModule.sessionMarking.test.ts and removed the "Phase 8", "8-A", "8-B" describe labels. The file covers two halves of one subject — which API a session mark is sent to, and what verdict a build-level hook failure produces — so the subject names it better than the phase that happened to introduce it. Also stripped parity-row citations from six test files. Same reasoning as the source comments: the row numbers point at a migration working artifact a future reader will not have, and were about to become dangling references. The assertions already state what they check. No test content changed; 1271 still pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`1382903` swapped `bdd_meta_info.feature.path` from the absolute `featurePath()`
to the raw `cucumberData.uri` on the premise that WDIO supplies a cwd-relative
uri. It does not. WDIO hands `beforeFeature` an ABSOLUTE path — verified on the
wire:
onFeatureStart: uri=/Users/…/automate-wdio_cucumber/features/cfg-one.feature
so `path.resolve(cwd, uri) === uri` and the swap was a no-op at runtime. The
dashboard still showed the developer's home directory. The unit test stayed
green only because its fixture fed `onFeatureStart` a relative uri, a shape WDIO
never produces.
Legacy never reads WDIO's uri: `insights-handler` builds the blob off the
cucumber world's `gherkinDocument.uri`, which is cwd-relative. Both bdd-meta
sites now go through `featureUriForMeta()`, which relativises against cwd and
reproduces that value exactly.
`test_file_path` is untouched and stays ABSOLUTE — the binary re-bases it itself,
and pre-relativising it corrupts `file_name` and `vc_filepath` (SDK-7233).
Verified on the O11Y dashboard, CLI against a published-9.35.1 legacy control on
the same feature file:
meta.feature.path features/cfg-one.feature == legacy (was an absolute path)
file_name features/cfg-one.feature == legacy (unchanged)
location features/cfg-one.feature (unchanged)
vc_filepath '' == legacy (unchanged)
No `/Users/` string survives anywhere in the CLI run's payload — 2 occurrences
before, 0 after.
The fixture now runs both uri shapes. Falsified: restoring `cucumberData.uri`
fails the two meta assertions on the absolute-uri arm only and leaves every
`test_file_path` assertion green — the same falsification 1382903 claimed, which
its relative-only fixture could not actually perform.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comment only; no code change. Nine lines to four on the specsRan guard in onBuildLevelHookEnd, keeping why it keys on the absence of scenario results rather than on ignoreHooksStatus. Also removes two parity-table references my earlier sweep missed — it was case-sensitive and these read "Parity row". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Opting out of session NAMING silently opted you out of status MARKING: the session showed `done`/unmarked on the dashboard instead of passed or failed. sessionMap registration was gated on skipSessionName in both places that do it — onBeforeTest returned before registering, and onAfterTest's repair carried the same conjunct — so testResults was never populated and the onAfterExecute sweep had nothing to mark. Registration is now independent of the flag; the name is what it suppresses. flushSessionName already hard-returns on skipSessionName and on an empty lastTestName, so a registered session cannot leak a name. Legacy gates its after() status block on setSessionStatus alone — setSessionName never enters the condition. Measured on both frameworks' legacy arms: with setSessionName:false, cucumber (9.35.1) and mocha (9.20.1, the last release before mocha was platformised) both report name '' and status passed, while both CLI arms report unmarked. So this regressed every framework on the CLI path, not just cucumber. G7: this changes wdio_mocha's behaviour too, and deliberately — mocha is equally broken today and the fix repairs it. The inverse-leak guard test asserted the opposite, having encoded the CLI's own prior behaviour rather than parity with legacy. Rewritten to assert exactly one status call carrying no name field; falsified against the reverted code. Verified at unit level only. Dashboard verification pending. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
With observability, accessibility and Percy all off, the Automate session went
unnamed and unmarked and browser.setCustomTags was never defined — a call to it
threw, where legacy defines the method and warns.
service.ts raised the CREATE/POST driver-registration event only inside
`if (shouldProcessEventForTesthub(''))`. That predicate is a disjunction over
the three product flags, called with an empty eventType, so accessibility or
Percy being on holds it open and only all-three-off closes it. With it closed
the event never fires: webdriverIOModule.onDriverCreated never runs, the driver
is unregistered, isBrowserstackSession() is falsy, and automateModule skips both
naming and status marking. customTagsModule.onBeforeExecute never assigns
setCustomTags either — the module IS constructed, so its absence is not a
construction problem but a missing event.
Measured over 8 runs, both flows: with a11y or Percy on, CLI is
indistinguishable from legacy. With everything off, CLI gives name '' / status
done / setCustomTags undefined against legacy's named, marked, defined.
Deliberately raised only where the gate would have swallowed it, rather than
hoisted out of the block. Hoisting reads better and is the right refactor later,
but it would reorder the event ahead of `new InsightsHandler(...)` on every
configuration that already works. This shape cannot execute when the gate is
open, so no working configuration changes and verification narrows to the
all-products-off arm.
Tests falsified in both directions: reverting the fix fails the all-off case;
making the new branch unconditional fails both double-fire cases.
Scope: service side only. The binary also creates an empty TestHub build in this
configuration where legacy creates none — out of scope here, still open.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Moved the three cases out of service.driverRegistration.cli.test.ts and deleted that file. service.test.ts is the main suite for service.ts and already mocks cli/index.js in the same shape, so the standalone file added a second copy of that setup for no benefit. Comment condensed from ten lines to five. No test content changed; still falsified in both directions — reverting the fix fails one case, making the new branch unconditional fails the other two. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…est.ts Second and last of the per-concern service suites. All its imports and its cli/index.js mock already existed in service.test.ts, so the separate file was duplicated setup around seven tests — three on the preferScenarioName event seam and four on _cucumberTestResult's failure reason. Repaired a comment my earlier parity-row sweep had mangled: removing "parity row 40:" left "the two halves of automateModule decides the rename", which no longer parsed. Rewritten and condensed. Also dropped an "escape class 3" reference the sweep missed — same migration-plan category as the row numbers. SDK-7047 kept; a ticket outlives the plan. No test content changed. 1277 pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Merges automateModule.sessionMarking.test.ts and automateModule.preferScenarioName.test.ts into automateModule.test.ts and deletes both. All three mocked the identical six modules, so the satellites were duplicated setup. Trimmed 21 cases to 15 rather than concatenating. Dropped: the BROWSERSTACK_TURBOSCALE_INTERNAL variant and the name/status-agree check (one resolver, already covered by the PATCH/PUT pair); a second mocha guard on the zero-scenario case; an all-passed baseline; a no-rename side-effect assertion; and a no-cucumber-scenarios case that repeats the scenariosRan != 1 branch. Every discriminating pair is kept, confirmed by falsifying the trimmed suite against all three shipped fixes: === 1 -> >= 1 fails 1, reverting the resultKey collapse fails 2, re-gating registration on skipSessionName fails 1. 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 |
Collaborator
Author
|
RUN_TESTS |
`npm run lint` exited 1 on no-extra-semi in service.test.ts. The leading `;`
guarded against ASI before a `(`-initial line, but the preceding token is the
`{` of an if-block, so there is nothing to guard.
Pre-existing, but in scope for this branch: merging the per-concern suites into
this file is what shifted the reported line to 2851.
eslint clean over src and tests; 1276 tests pass on the post-merge tree.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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?
Related Jira task/s
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)
Release notes (internal): (required — engineer-facing; what actually changed / why)
Checklist
PR Validations
Run Tests: Comment RUN_TESTS to trigger sanity tests.