-
Notifications
You must be signed in to change notification settings - Fork 9
SDK 7414 wdio cucumber platformisation #191
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
447c4d3
78a90f2
7299385
71fa9bb
e6596eb
bdf9cb1
650ca23
28aa160
e7f74f5
f1444f6
7573393
ed012cf
a16ef31
f8f5f6a
1382903
e35a3d1
8c550bd
17ed99f
f45497d
f84fc39
59a03de
31c5cc7
8d63e5f
5fd444d
e9f9aaa
6a73c32
bb7eacf
343cc78
bc28ca6
72fbc49
615130a
6844406
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| --- | ||
| "@wdio/browserstack-service": minor | ||
| --- | ||
|
|
||
| - WebdriverIO + CucumberJS now runs on the BrowserStack CLI flow, the same path Mocha already uses. Reporting, accessibility, Percy and session naming behave as before — no config change is needed. | ||
| - Fixed: sessions were left unmarked pass/fail when setSessionName: false was set. Naming and status are independent options again. | ||
| - Fixed: the accessibility extension was not applied on non-BrowserStack infrastructure, leaving scans empty on otherwise green runs. |
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -89,10 +89,14 @@ export default class AccessibilityModule extends BaseModule { | |
| if (!this.accessibility) { | ||
| return | ||
| } | ||
| // Open the scan gate for the hook window so DOM-changing commands issued inside | ||
| // before/beforeEach/afterEach/after hooks trigger scans (web per-command path). The | ||
| // following onBeforeTest re-computes the per-test gate, so this only affects the hook. | ||
| if (this.autoScanning && sessionId !== undefined && sessionId !== null) { | ||
| // Open the scan gate for the hook window, so commands issued inside hooks are scanned. | ||
| // Mocha-only, as legacy gates the identical write (`this._framework === 'mocha'`): it | ||
| // relies on the following onBeforeTest re-computing the gate, which holds only where | ||
| // beforeEach precedes beforeTest. Cucumber inverts that ordering, so there the write | ||
| // would land last and force the gate permanently open. | ||
| const frameworkName = String(TestFramework.getState(testInstance, TestFrameworkConstants.KEY_TEST_FRAMEWORK_NAME) || '') | ||
| const isMocha = frameworkName.toLowerCase().includes('mocha') | ||
| if (this.autoScanning && isMocha && sessionId) { | ||
|
Comment on lines
+98
to
+99
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| this.accessibilityMap.set(sessionId, true) | ||
| } | ||
| } catch (error) { | ||
|
|
@@ -343,7 +347,13 @@ export default class AccessibilityModule extends BaseModule { | |
|
|
||
| const sessionId = AutomationFramework.getState(autoInstance, AutomationFrameworkConstants.KEY_FRAMEWORK_SESSION_ID) | ||
| const accessibilityOptions = this.config.accessibilityOptions | ||
| const shouldScanTest = this.autoScanning && shouldScanTestForAccessibility(suiteTitle, test.title || '', accessibilityOptions as Record<string, string> | undefined) && this.accessibility | ||
| // Cucumber filters scans by gherkin tag, which needs the world object and the 6-arg | ||
| // form of shouldScanTestForAccessibility; the 3-arg form matches include/exclude tags | ||
| // against the test title instead and so silently scans every scenario. `args.world` is | ||
| // only ever populated on the cucumber path, so mocha and jasmine keep the exact 3-arg | ||
| // behaviour — both extra args arrive undefined/false and the tag branch is not taken. | ||
| const world = args.world as { [key: string]: unknown } | undefined | ||
| const shouldScanTest = this.autoScanning && shouldScanTestForAccessibility(suiteTitle, test.title || '', accessibilityOptions as Record<string, string> | undefined, world, Boolean(world)) && this.accessibility | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| this.accessibilityMap.set(sessionId, shouldScanTest) | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Problem
The new fall-through log is unconditional: any
testFrameworkDetail.namethat is notwebdriverio-mochaorwebdriverio-cucumberreaches it and emits at error level.launcher.ts:345sets that name mechanically asWDIO_NAMING_PREFIX + config.framework, so the three WDIO runner values producewebdriverio-mocha,webdriverio-cucumberandwebdriverio-jasmine. Jasmine is a first-class WDIO framework this service supports, and it has no CLI test-framework class by design — so after this change every Jasmine run on the CLI/binary flow logssetupTestFramework: no CLI test framework registered for name=webdriverio-jasmine; test events will not be trackedat error level, on a path where nothing is actually broken relative to the shipped behaviour.Two consequences:
The intent of the added log is right; only its severity and its indiscriminate reach are wrong.
Suggested Fix
Separate the known, expected unregistered frameworks from the unexpected ones, and drop the expected case to debug:
If keeping a single branch is preferred, at minimum demote the level to
debug/warn— an unsupported-by-design framework is not an error condition in the customer's run.Confidence: 🟢 cards/_shared.md SH-12: a feature that cannot run must disable itself loudly at debug level, leaving the test unaffected; packs/default.md DEF-02 requires the signal be surfaced, not shouted on an expected path
— SDK PR Review Agent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not valid — jasmine never reaches this line.
setupTestFramework()is called fromloadModules(), which runs insidebootstrap(), and both entry points tobootstrap()are gated on the CLI allow-list:launcher.ts:343—if (CLIUtils.checkCLISupportedFrameworks(config.framework) && !isMultiremote)service.ts:176— the same gate in the workerwith
CLISupportedFrameworks = ['mocha', 'cucumber'](cliUtils.ts:50). Jasmine is not in that list, so the CLI never starts for a jasmine run and this method is never entered.The comment's own phrasing points at it: "every Jasmine run on the CLI/binary flow" — jasmine has no CLI/binary flow. The log therefore fires only for the genuinely unexpected name, which is the case it was written for, and error level is appropriate there.