@wdio/cucumber-framework: Add original coordinates when resetting Cucumber Support Library by nextlevelbeard · Pull Request #11078 · webdriverio/webdriverio

nextlevelbeard · 2023-09-04T08:48:19Z

Proposed changes

export interface ISupportCodeCoordinates {
    requireModules: string[];
    requirePaths: string[];
    importPaths: string[];
}

when resetting the Cucumber Support Library.
In some cases, when a user defines a custom World like so:

import type { ITestCaseHookParameter } from '@cucumber/cucumber';
import { Before, World, setWorldConstructor } from '@wdio/cucumber-framework';

export class Scenario extends World {
  /** My custom World prop */
  myProp!: { myKey: string };
}

setWorldConstructor(Scenario);

Before(async function doStuff(this: Scenario, scenario: ITestCaseHookParameter) {
  this.myProp = { myKey: 'Test!' }
});

Cucumber follows different logic for dealing with the support library, as seen here:

  const supportCoordinates =
    'World' in configuration.support
      ? configuration.support.originalCoordinates
      : configuration.support

This only appears to be a problem when the parallel option is set to a non-zero value.

This fix includes the third argument for the original coordinates in the reset() call of the SuportCodeLibraryBuilder.

Prevents errors such as

TypeError: Cannot read properties of undefined (reading 'getInvocationParameters')
    at Object.run (/Users/user/repos/repo/node_modules/@cucumber/cucumber/lib/runtime/step_runner.js:39:47)
    at TestCaseRunner.invokeStep (/Users/user/repos/repo/node_modules/@cucumber/cucumber/lib/runtime/test_case_runner.js:95:44)
    at TestCaseRunner.runHook (/Users/user/repos/repo/node_modules/@cucumber/cucumber/lib/runtime/test_case_runner.js:200:27)
    at /Users/user/repos/repo/node_modules/@cucumber/cucumber/lib/runtime/test_case_runner.js:171:39
    at TestCaseRunner.aroundTestStep (/Users/user/repos/repo/node_modules/@cucumber/cucumber/lib/runtime/test_case_runner.js:120:38)
    at TestCaseRunner.runAttempt (/Users/user/repos/repo/node_modules/@cucumber/cucumber/lib/runtime/test_case_runner.js:158:24)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async TestCaseRunner.run (/Users/user/repos/repo/node_modules/@cucumber/cucumber/lib/runtime/test_case_runner.js:136:35)
    at async Worker.runTestCase (/Users/user/repos/repo/node_modules/@cucumber/cucumber/lib/runtime/parallel/worker.js:82:9)
    at async Worker.receiveMessage (/Users/user/repos/repo/node_modules/@cucumber/cucumber/lib/runtime/parallel/worker.js:64:13)

Types of changes

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation update

Checklist

I have read the CONTRIBUTING doc
I have added tests that prove my fix is effective or that my feature works
I have added necessary documentation (if appropriate)
I have added proper type definitions for new commands (if appropriate)

Further comments

Reviewers: @webdriverio/project-committers

nextlevelbeard · 2023-09-04T08:59:50Z

@tamil777selvan Mind having a look at this?

tamil777selvan · 2023-09-04T13:09:59Z

@tamil777selvan Mind having a look at this?

hi @nextlevelbeard, Given that this is an options parameter, it shouldn't cause an error. Also, in your cucumberOpts, have you used import instead of require to import step definitions and hooks, especially if you're working with ESM?

I've tested it with the provided custom world, and it worked as expected for me. Could you provide any reproducible examples or scenarios where this issue occurs?

nextlevelbeard · 2023-09-04T13:59:01Z

@tamil777selvan Mind having a look at this?

hi @nextlevelbeard, Given that this is an options parameter, it shouldn't cause an error. Also, in your cucumberOpts, have you used import instead of require to import step definitions and hooks, especially if you're working with ESM?

I've tested it with the provided custom world, and it worked as expected for me. Could you provide any reproducible examples or scenarios where this issue occurs?

Yep, pure ESM and import was specified.

To reproduce try to put parallel as a non-zero integer.
Seems to occur if Cucumber is running in parallel, a worker Coordinator is setup instead of the Runtime, each worker then appears to require/import the support code.

I know we are not supposed to use that feature with WDIO but that's how I found the error.
Would be nice to refactor parallelism to be compatible with Cucumber works.

Another problem I found with the support code library in parallel, when importing the globals from @wdio/globals package and referencing browser in Cucumber hooks (Before, After), it triggers an Error:

Error: No browser instance registered. Don't import @wdio/globals outside of the WDIO testrunner context. Or you have two two different "@wdio/globals" packages installed.
    at Object.get (file:///Users/user/repos/repo/node_modules/@wdio/globals/build/index.js:15:23)
    at Scenario.doStuff (file:///Users/user/repos/repo/e2e/steps/hooks.steps.ts:49:36)

Issue updated.

nextlevelbeard · 2023-09-04T14:22:46Z

Seems like each WDIO worker should correspond to one Cucumber worker when maxInstances is higher than 1.

At the moment, runCucumber is called for each worker, potentially producing separate sets of report/results, publishing a cucumber.io report for each worker, etc.

tamil777selvan · 2023-09-04T15:35:00Z

I know we are not supposed to use that feature with WDIO but that's how I found the error.

In this situation, we shouldn't permit the use of parallel counting to be specified in the cucumberOpts, and we should keep it as an internal feature.

Would be nice to refactor parallelism to be compatible with Cucumber works.

I'm uncertain if we can immediately implement this change because the current functionality is aligned with the worker nodes that were generated by the 'runner' process and is functioning as intended.

tamil777selvan · 2023-09-04T15:48:20Z

Seems like each WDIO worker should correspond to one Cucumber worker when maxInstances is higher than 1.

At the moment, runCucumber is called for each worker, potentially producing separate sets of report/results, publishing a cucumber.io report for each worker, etc.

Then it would be a better approach not to relay on the publish plugin from Cucumber and instead we should work on extending the wdio-reporter interface to create a cucumber message report that can be shared at the end of the process.

christian-bromann · 2023-09-05T05:42:49Z

It seems to me that WebdriverIO currently doesn't support Cucumber's parallel feature. I raised a PR (#11099) to ensure we fail if the parallel option is set.

I suggest to close this PR and raise a new issue to discuss support for Cucumber parallel feature. What do you all think?

nextlevelbeard · 2023-09-05T13:37:42Z

@christian-bromann Agree, I could raise a new issue.
About closing: feel free to do so but do note, this change is harmless and could be merged, it ensures the Cucumber workers properly define/reset the support library and not doing it now will just generate another headache down the line for whoever implements that support for native Cucumber parallelism.

nextlevelbeard · 2023-09-05T14:13:53Z

Created #11101

christian-bromann

LGTM 👍

fix: Add original coordinates to Cucumber Support Librabry

2677232

nextlevelbeard requested a review from christian-bromann September 4, 2023 08:48

christian-bromann mentioned this pull request Sep 5, 2023

(@wdio/cucumber-framework): disable parallel execution and warn user that this feature is not supported #11099

Merged

8 tasks

christian-bromann approved these changes Sep 5, 2023

View reviewed changes

christian-bromann added the PR: Polish 💅 PRs that contain improvements on existing features label Sep 5, 2023

christian-bromann merged commit 5d4b1a4 into webdriverio:main Sep 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

@wdio/cucumber-framework: Add original coordinates when resetting Cucumber Support Library#11078

@wdio/cucumber-framework: Add original coordinates when resetting Cucumber Support Library#11078
christian-bromann merged 1 commit intowebdriverio:mainfrom
nextlevelbeard:fix-cucumber-run-reset

nextlevelbeard commented Sep 4, 2023 •

edited

Loading

Uh oh!

nextlevelbeard commented Sep 4, 2023

Uh oh!

tamil777selvan commented Sep 4, 2023

Uh oh!

nextlevelbeard commented Sep 4, 2023 •

edited

Loading

Uh oh!

nextlevelbeard commented Sep 4, 2023

Uh oh!

tamil777selvan commented Sep 4, 2023

Uh oh!

tamil777selvan commented Sep 4, 2023

Uh oh!

christian-bromann commented Sep 5, 2023

Uh oh!

nextlevelbeard commented Sep 5, 2023

Uh oh!

nextlevelbeard commented Sep 5, 2023

Uh oh!

christian-bromann left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

nextlevelbeard commented Sep 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Types of changes

Checklist

Further comments

Reviewers: @webdriverio/project-committers

Uh oh!

nextlevelbeard commented Sep 4, 2023

Uh oh!

tamil777selvan commented Sep 4, 2023

Uh oh!

nextlevelbeard commented Sep 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nextlevelbeard commented Sep 4, 2023

Uh oh!

tamil777selvan commented Sep 4, 2023

Uh oh!

tamil777selvan commented Sep 4, 2023

Uh oh!

christian-bromann commented Sep 5, 2023

Uh oh!

nextlevelbeard commented Sep 5, 2023

Uh oh!

nextlevelbeard commented Sep 5, 2023

Uh oh!

christian-bromann left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nextlevelbeard commented Sep 4, 2023 •

edited

Loading

nextlevelbeard commented Sep 4, 2023 •

edited

Loading