-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Use new API for running Cucumber #3995
base: main
Are you sure you want to change the base?
Conversation
Thanks for sending this. On a quick glance, I'm thinking that we should keep the existing implementation for Cucumber version pre-10 so that we are backwards compatible. |
@beatfactor no probem, I've updated so that both integrations are there with fairly decent code reuse, and we check the version exported by Cucumber to decide which to run. There's also an env var override which we use in the tests so we check every test case with both integrations. We'll probably remove the |
Ok, so now it will look for the installed cucumber version and decide which integration strategy to use? Regarding the env var override, I'd suggest to use the existing config dictionary and add a new field there, as in extend the {
test_runner: {
// set cucumber as the runner
type: 'cucumber',
// define cucumber specific options
options: {
//set the feature path
feature_path: 'examples/cucumber-js/*/*.feature',
// start the webdriver session automatically (enabled by default)
auto_start_session: true,
// use parallel execution in Cucumber
// set number of workers to use (can also be defined in the cli as --parallel 2
parallel: 2
}
},
src_folders: ['examples/cucumber-js/features/step_definitions']
} https://nightwatchjs.org/guide/writing-tests/using-cucumberjs.html |
Thanks @beatfactor, I was a bit hesitant to add an option because it feels like this should just need to be an internal override for test purposes, and an env var was the least worst way to do it I could see. But thinking about it, if we make it an option it at least gives people the control if they need to switch because of some niche issue or something. I'll make that change. |
38be81f
to
e5219cd
Compare
e5219cd
to
e748fb2
Compare
e748fb2
to
dc02a6c
Compare
This PR changes the Cucumber runner integration to use Cucumber's JavaScript API rather than the now-deprecated
Cli
class. Closes #3317.The size of the code change is pretty small since we can now pass in an argv array to Cucumber's
loadConfiguration
function without having to convert it to an options object.Some of the tests from
test/cucumber-integration-tests
are failing for me locally, but are hitting issues at the Nightwatch level and fail in the same way onmain
- I'll try to get these fixed up if I can, and perhaps add some more.One thing I wasn't sure about was - given this will only work with cucumber-js 10.3.1 and above - is whether I should keep the existing
Cli
-based implementation in place for now and check the version from Cucumber to decide which one to use. I'd be happy to add this in if it's considered valuable.(Raising as draft so as to get some eyes on this early - feedback welcome.)