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

Protractor CLI rejects `cucumberOpts` as invalid #3978

Closed
jan-molak opened this Issue Jan 16, 2017 · 17 comments

Comments

Projects
None yet
10 participants
@jan-molak
Contributor

jan-molak commented Jan 16, 2017

Hello there!

It looks like the whitelist defined in the cli.ts rejects the cucumberOpts key which the protractor-cucumber-framework uses.

  • Node Version: 6.9.4
  • Protractor Version: 5.0.0

Running protractor with the below config file results in this output:

Error: Found extra flags: cucumberOpts
exports.config = {
    seleniumServerJar: path.resolve(node_modules, 'protractor/node_modules/webdriver-manager/selenium/selenium-server-standalone-2.53.1.jar'),
        
    framework: 'custom',
    frameworkPath: require.resolve('protractor-cucumber-framework'),
    specs: [ 'features/**/*.feature' ],
    cucumberOpts: {
        require:    [
            'features/**/*.ts'
        ],
        format:     'pretty',
        compiler:   'ts:ts-node/register',
    }
};

I guess there are several ways to resolve this issue:

  1. run the protractor/cucumber combo with a --disableChecks switch (that's more of a workaround, though)
  2. change the whitelist to include cucumberOpts, which allows people to run their cucumber tests same as they used to, but requires the Protractor Team to amend the whitelist whenever a new framework comes along or changes the way it's configured (which breaks "half of SOLID" ;-))
  3. change the CLI to only perform the checks on keys recognised and supported by Protractor, ignoring any additional keys, which to me seems the most sensible approach.

Looking forward to hearing your thoughts!
Jan

@heathkit

This comment has been minimized.

Show comment
Hide comment
@heathkit

heathkit Jan 17, 2017

Member

The goal of the flag checks is to help catch when people misspell a flag name, so ignoring additional keys would defeat the purpose. However, there should be a way for protractor-cucumber-framework to add to the whitelist, we can work on that.

Member

heathkit commented Jan 17, 2017

The goal of the flag checks is to help catch when people misspell a flag name, so ignoring additional keys would defeat the purpose. However, there should be a way for protractor-cucumber-framework to add to the whitelist, we can work on that.

@jan-molak

This comment has been minimized.

Show comment
Hide comment
@jan-molak

jan-molak Jan 17, 2017

Contributor

I guess there could be an API to allow plugin authors to register additional commands perhaps?

Contributor

jan-molak commented Jan 17, 2017

I guess there could be an API to allow plugin authors to register additional commands perhaps?

@igniteram

This comment has been minimized.

Show comment
Hide comment
@igniteram

igniteram Jan 18, 2017

Contributor

@jan-molak This is strange you should not face this issue. Are you sure you are not passing cucumberOpts flag through command line?

As you see protractor's config interface was recently updated to support any optional key i.e. it allows us to use cucumberOpts with minimal dependencies. Coming to new changes to protractor's CLI, I agree with @mgiambalvo ignoring additional flags would not help users know the protractor's actual cli commands.
If you want to use your custom flag you would have to use --disableChecks flag.

But since you are passing cucumberOpts in config itself you should not get this issue. I have been using this setup and did not face any issues.

For your reference also pls check protractor-cookbook protractor-typescript-cucumber example which was recently upgraded to Protractor 5.0 with no issues.

Contributor

igniteram commented Jan 18, 2017

@jan-molak This is strange you should not face this issue. Are you sure you are not passing cucumberOpts flag through command line?

As you see protractor's config interface was recently updated to support any optional key i.e. it allows us to use cucumberOpts with minimal dependencies. Coming to new changes to protractor's CLI, I agree with @mgiambalvo ignoring additional flags would not help users know the protractor's actual cli commands.
If you want to use your custom flag you would have to use --disableChecks flag.

But since you are passing cucumberOpts in config itself you should not get this issue. I have been using this setup and did not face any issues.

For your reference also pls check protractor-cookbook protractor-typescript-cucumber example which was recently upgraded to Protractor 5.0 with no issues.

@jan-molak

This comment has been minimized.

Show comment
Hide comment
@jan-molak

jan-molak Jan 18, 2017

Contributor

Thanks @igniteram, I didn't realise the behaviour is different when the option is specified on the command line.

Now that you pointed that out it might be relevant that I'm doing both:

  • I set the cucumberOpts in the protractor config file to register ts-node
  • I pass the cucumberOpts.name flag to run a specific test

A complete example to demonstrate the behaviour I'm seeing would be this:

// protractor.conf.js

exports.config = {
    seleniumServerJar: path.resolve(node_modules, 'protractor/node_modules/webdriver-manager/selenium/selenium-server-standalone-2.53.1.jar'),
        
    framework: 'custom',
    frameworkPath: require.resolve('protractor-cucumber-framework'),
    specs: [ 'features/**/*.feature' ],
    cucumberOpts: {
        require:    [
            'features/**/*.ts'
        ],
        format:     'pretty',
        compiler:   'ts:ts-node/register',
    }
};
// package.json

{
  // ...
  "scripts": {
    "pree2e": "npm run webdriver:update -- --standalone",
    "e2e": "protractor ./protractor.conf.js",
    "e2e-single": "protractor ./protractor.conf.js --cucumberOpts.name",
    "webdriver-manager": "webdriver-manager",
    "webdriver:update": "npm run webdriver-manager update",
    // ...
  }
}

Now, although running npm run e2e (without specifying the cucumberOpts.name on the command line) works fine, running the e2e-single results in an error:

npm run e2e-single "Subtract two numbers"

/some/path/node_modules/protractor/built/cli.js:172
        throw new Error('Found extra flags: ' + unknownKeys.join(', '));
        ^

Error: Found extra flags: cucumberOpts
    at Object.<anonymous> (/some/path/node_modules/protractor/built/cli.js:172:15)

I suppose this means that the issue still stands, but thanks to your suggestion it can be narrowed down to the CLI rather than the config. Thanks!

Contributor

jan-molak commented Jan 18, 2017

Thanks @igniteram, I didn't realise the behaviour is different when the option is specified on the command line.

Now that you pointed that out it might be relevant that I'm doing both:

  • I set the cucumberOpts in the protractor config file to register ts-node
  • I pass the cucumberOpts.name flag to run a specific test

A complete example to demonstrate the behaviour I'm seeing would be this:

// protractor.conf.js

exports.config = {
    seleniumServerJar: path.resolve(node_modules, 'protractor/node_modules/webdriver-manager/selenium/selenium-server-standalone-2.53.1.jar'),
        
    framework: 'custom',
    frameworkPath: require.resolve('protractor-cucumber-framework'),
    specs: [ 'features/**/*.feature' ],
    cucumberOpts: {
        require:    [
            'features/**/*.ts'
        ],
        format:     'pretty',
        compiler:   'ts:ts-node/register',
    }
};
// package.json

{
  // ...
  "scripts": {
    "pree2e": "npm run webdriver:update -- --standalone",
    "e2e": "protractor ./protractor.conf.js",
    "e2e-single": "protractor ./protractor.conf.js --cucumberOpts.name",
    "webdriver-manager": "webdriver-manager",
    "webdriver:update": "npm run webdriver-manager update",
    // ...
  }
}

Now, although running npm run e2e (without specifying the cucumberOpts.name on the command line) works fine, running the e2e-single results in an error:

npm run e2e-single "Subtract two numbers"

/some/path/node_modules/protractor/built/cli.js:172
        throw new Error('Found extra flags: ' + unknownKeys.join(', '));
        ^

Error: Found extra flags: cucumberOpts
    at Object.<anonymous> (/some/path/node_modules/protractor/built/cli.js:172:15)

I suppose this means that the issue still stands, but thanks to your suggestion it can be narrowed down to the CLI rather than the config. Thanks!

@igniteram

This comment has been minimized.

Show comment
Hide comment
@igniteram

igniteram Jan 19, 2017

Contributor

@jan-molak This is expected behaviour! since protractor 5.0, there is a check for unidentified command line flags, i.e. it would throw the above error if you use any flag which is not part of protractor's command line options which can be found here .

You could see that cucumberOpts doesn't belong to the list of identified command line flags in protractor, so in case you are passing this as a custom flag, as I said earlier you have to use --disableChecks flag along with this. So in your case you would have to do this-

"e2e-single": "protractor ./protractor.conf.js --cucumberOpts.name --disableChecks"

Now your script should run successfully, The job of disableChecks flags is to ignore any unidentified flags just as it was working in the earlier versions.

In the next version of protractor you would get a better error message for these scenarios. I hope this clarifies your query!

Contributor

igniteram commented Jan 19, 2017

@jan-molak This is expected behaviour! since protractor 5.0, there is a check for unidentified command line flags, i.e. it would throw the above error if you use any flag which is not part of protractor's command line options which can be found here .

You could see that cucumberOpts doesn't belong to the list of identified command line flags in protractor, so in case you are passing this as a custom flag, as I said earlier you have to use --disableChecks flag along with this. So in your case you would have to do this-

"e2e-single": "protractor ./protractor.conf.js --cucumberOpts.name --disableChecks"

Now your script should run successfully, The job of disableChecks flags is to ignore any unidentified flags just as it was working in the earlier versions.

In the next version of protractor you would get a better error message for these scenarios. I hope this clarifies your query!

@jan-molak

This comment has been minimized.

Show comment
Hide comment
@jan-molak

jan-molak Jan 19, 2017

Contributor

Thank you for getting back to me so quickly!

If I may make a suggestion, perhaps allowing plugin authors to register additional flags for the CLI and configuration options for the protractor.conf.js via the plugin API might be a nice way of avoiding having to specify the --disableChecks, which, granted, gets rid of the warning, but also disables other, potentially useful checks?

This way the white list we discussed earlier could stay as it is, but optionally get expanded via plugins.

Looking forward to hearing your thoughts!

Contributor

jan-molak commented Jan 19, 2017

Thank you for getting back to me so quickly!

If I may make a suggestion, perhaps allowing plugin authors to register additional flags for the CLI and configuration options for the protractor.conf.js via the plugin API might be a nice way of avoiding having to specify the --disableChecks, which, granted, gets rid of the warning, but also disables other, potentially useful checks?

This way the white list we discussed earlier could stay as it is, but optionally get expanded via plugins.

Looking forward to hearing your thoughts!

@igniteram

This comment has been minimized.

Show comment
Hide comment
@igniteram

igniteram Jan 19, 2017

Contributor

Expanding the white list via plugin api should be a viable option in future.

@cnishina what are your thoughts on it? you are the right person here 😄

Contributor

igniteram commented Jan 19, 2017

Expanding the white list via plugin api should be a viable option in future.

@cnishina what are your thoughts on it? you are the right person here 😄

jan-molak added a commit to jan-molak/serenity-js that referenced this issue Jan 19, 2017

test(protractor/cucumber): --disableChecks is only needed when invoki…
…ng protractor with --cucubmerOpts

As per our conversation at angular/protractor#3978

heathkit added a commit that referenced this issue Jan 23, 2017

fix(cli): Allow frameworks to specify flags they recognize. (#3994)
Fix for #3978.

Our initial plan to allow setting --disableChecks with an environment variable is insufficient, since the custom framework isn't even require()'d until after the config is parsed. This moves the unknown flag check into the runner, and gives frameworks a way to specify extra flags they accept.
@heathkit

This comment has been minimized.

Show comment
Hide comment
@heathkit

heathkit Jan 23, 2017

Member

There might be a better way to do this that we could do in the future. In the meantime, #3994 will allow frameworks to specify extra flags that they accept in the call to runTestPreparer().

This should go out in Protractor 5.1

Member

heathkit commented Jan 23, 2017

There might be a better way to do this that we could do in the future. In the meantime, #3994 will allow frameworks to specify extra flags that they accept in the call to runTestPreparer().

This should go out in Protractor 5.1

@heathkit heathkit added this to the 5.1 milestone Jan 23, 2017

@jan-molak

This comment has been minimized.

Show comment
Hide comment
@jan-molak

jan-molak Jan 23, 2017

Contributor

Awesome, thanks for your help @mgiambalvo 👍

Contributor

jan-molak commented Jan 23, 2017

Awesome, thanks for your help @mgiambalvo 👍

@juliemr juliemr closed this Feb 3, 2017

jan-molak added a commit to jan-molak/serenity-js that referenced this issue Feb 11, 2017

feat(serenity-protractor): Support for Protractor 5.1.x
This change adds support for using Serenity/JS with Protractor 5.0.0, 5.1.0 and 5.1.1:
- `ProtractorNotifier` deals with Protractor 5.1.0 requirement for test frameworks to invoke `runner.afterEach` after every test,
so that the browser is correctly restarted between the tests. See angular/protractor#4087
- `TestFrameworkDetector` registers `mochaOpts` and `cucumberOpts` as valid command line options,
benefiting from angular/protractor#3994 and dealing with a bug introduced in Protractor 5.0.0, which prevents command line arguments
from being passed on to custom test frameworks. See angular/protractor#3978
- Updated the examples and documentation to work with Protractor 5.1.x
- Enables #18

Related issues: angular/protractor#3978 angular/protractor#3994 angular/protractor#4087 #18

igniteram added a commit to igniteram/protractor that referenced this issue Feb 21, 2017

fix(cli): Allow frameworks to specify flags they recognize. (#3994)
Fix for #3978.

Our initial plan to allow setting --disableChecks with an environment variable is insufficient, since the custom framework isn't even require()'d until after the config is parsed. This moves the unknown flag check into the runner, and gives frameworks a way to specify extra flags they accept.
@AlexMarchini

This comment has been minimized.

Show comment
Hide comment
@AlexMarchini

AlexMarchini Mar 28, 2017

@mgiambalvo
I use the default jasmine framework, but I define additional CLI options in my project, such as account options for the app I'm running tests against. These get picked up as unknown flags.

Is there currently any way for me to plug these into the call to runTestPreparer to get rid of the warning?

AlexMarchini commented Mar 28, 2017

@mgiambalvo
I use the default jasmine framework, but I define additional CLI options in my project, such as account options for the app I'm running tests against. These get picked up as unknown flags.

Is there currently any way for me to plug these into the call to runTestPreparer to get rid of the warning?

@pawelus

This comment has been minimized.

Show comment
Hide comment
@pawelus

pawelus Jun 8, 2017

I'm facing the same thing as @AlexMarchini
We pass a bunch of parameters from our CI jobs - things like environment to run on, build numbers, etc.

It would be good to have a way to tell protractor those are to be expected.

pawelus commented Jun 8, 2017

I'm facing the same thing as @AlexMarchini
We pass a bunch of parameters from our CI jobs - things like environment to run on, build numbers, etc.

It would be good to have a way to tell protractor those are to be expected.

@igniteram

This comment has been minimized.

Show comment
Hide comment
@igniteram

igniteram Jun 8, 2017

Contributor

This feature is already available in protractor, Since v5.1 you have 2 options-

  • Either set disableChecks option to true which would disable the console options check. i.e. Any extra flag you pass it would ignore.

Please check the config.ts interface on how this could be set.

  • You can specify your custom flags in unknownFlags option in config file, you can pass in your flags as array of strings.

it is also available in config.ts
I know there is a lack of documentation on this, If you have time please feel free to raise a PR. cheers!

Contributor

igniteram commented Jun 8, 2017

This feature is already available in protractor, Since v5.1 you have 2 options-

  • Either set disableChecks option to true which would disable the console options check. i.e. Any extra flag you pass it would ignore.

Please check the config.ts interface on how this could be set.

  • You can specify your custom flags in unknownFlags option in config file, you can pass in your flags as array of strings.

it is also available in config.ts
I know there is a lack of documentation on this, If you have time please feel free to raise a PR. cheers!

@djom20

This comment has been minimized.

Show comment
Hide comment
@djom20

djom20 Jun 8, 2017

Hi @igniteram,

I have the same issue but my problem is that I have a lot of arguments in the cucumberOpts something like that:

cucumberOpts: {
        require: ['step_definitions/**/*_steps.js', 'support/*.js'],
        format: 'pretty', // format: 'json',
        tags: ['@complete', '~@displays_date_question_results', '~@analyzingSurveyExpression', '~@stressTests', '~@widgets', '~@assuring', '~@todo', '~@tests', '~@noRunLocal'],
        keepAlive: false
    }

is possible to pass all those params by the cli at the same time?. Also I'm not using typeScripts

djom20 commented Jun 8, 2017

Hi @igniteram,

I have the same issue but my problem is that I have a lot of arguments in the cucumberOpts something like that:

cucumberOpts: {
        require: ['step_definitions/**/*_steps.js', 'support/*.js'],
        format: 'pretty', // format: 'json',
        tags: ['@complete', '~@displays_date_question_results', '~@analyzingSurveyExpression', '~@stressTests', '~@widgets', '~@assuring', '~@todo', '~@tests', '~@noRunLocal'],
        keepAlive: false
    }

is possible to pass all those params by the cli at the same time?. Also I'm not using typeScripts

@djom20

This comment has been minimized.

Show comment
Hide comment
@djom20

djom20 Jun 8, 2017

Hi @igniteram,

I tried to run this:

protractor cucumber_conf.js --cucumberOpts.require="step_definitions/**/*_steps.js" --cucumberOpts.tags="@surveyMakingImagesDesktopSmokeTest" --cucumberOpts.tags="~@todo" --cucumberOpts.tags="~@noRunLocal" --disableChecks

but the steps were not found by protractor.

djom20 commented Jun 8, 2017

Hi @igniteram,

I tried to run this:

protractor cucumber_conf.js --cucumberOpts.require="step_definitions/**/*_steps.js" --cucumberOpts.tags="@surveyMakingImagesDesktopSmokeTest" --cucumberOpts.tags="~@todo" --cucumberOpts.tags="~@noRunLocal" --disableChecks

but the steps were not found by protractor.

@drew-royster

This comment has been minimized.

Show comment
Hide comment
@drew-royster

drew-royster Jul 13, 2017

Where can I adjust the default cucumber timeout? I haven't found a list of all available cucumberOpts.

drew-royster commented Jul 13, 2017

Where can I adjust the default cucumber timeout? I haven't found a list of all available cucumberOpts.

@wswebcreation

This comment has been minimized.

Show comment
Hide comment
@wswebcreation

wswebcreation Jul 14, 2017

Collaborator

@drew-royster

You can't adjust it through the command line. See the CucumberJS timeouts-docs to adjust this.

Maybe if you create something custom you can add it like for example using yargs and do something like this

// You CucumberJS timeout file that you load through your `cucumberOpts.require` in your
// Protractor conf
var {defineSupportCode} = require('cucumber');
// Add yargs
var argv = require('yargs').argv

defineSupportCode(function({setDefaultTimeout}) {
  // Then add the property `defaultTimeout` that will be passed through the cli with
  // npm run protractor -- --defaultTimeout=60000
  setDefaultTimeout(argv.defaultTimeout);
});
Collaborator

wswebcreation commented Jul 14, 2017

@drew-royster

You can't adjust it through the command line. See the CucumberJS timeouts-docs to adjust this.

Maybe if you create something custom you can add it like for example using yargs and do something like this

// You CucumberJS timeout file that you load through your `cucumberOpts.require` in your
// Protractor conf
var {defineSupportCode} = require('cucumber');
// Add yargs
var argv = require('yargs').argv

defineSupportCode(function({setDefaultTimeout}) {
  // Then add the property `defaultTimeout` that will be passed through the cli with
  // npm run protractor -- --defaultTimeout=60000
  setDefaultTimeout(argv.defaultTimeout);
});
@andreasmarkussen

This comment has been minimized.

Show comment
Hide comment
@andreasmarkussen

andreasmarkussen Apr 24, 2018

Warning goes away when upgrading protractor-cucumber-framework from 4.2.0 to 5.0.0 on my machine. So yes! closed.

andreasmarkussen commented Apr 24, 2018

Warning goes away when upgrading protractor-cucumber-framework from 4.2.0 to 5.0.0 on my machine. So yes! closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment