Skip to content
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

chore(KarmaConf): read CLI browser option to run tests #6166

Closed
wants to merge 1 commit into from

Conversation

ghybs
Copy link
Collaborator

@ghybs ghybs commented May 22, 2018

Hi,

This PR (re)makes it possible to specify a browser when executing a test command (e.g. npm run test-nolint -- --ff for Firefox).

So that it is easier to run tests in local browser (other than PhantomJS).

This CLI argument browser option disappeared when Jake has been removed from scripts.

This PR will make it easier to locally debug to re-enable PR's #5845 and #5831

so that it is easier to run tests in local browser (other than PhantomJS).
This CLI argument browser option disappeared when Jake has been removed from scripts.
@ghybs ghybs requested a review from cherniavskii May 22, 2018 10:22
Copy link
Collaborator

@cherniavskii cherniavskii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, IIRC it's possible to do this right now, e.g.:
npm test -- --browsers Firefox,Chrome,Safari,IE

@ghybs
Copy link
Collaborator Author

ghybs commented May 22, 2018

mmm I got caught by the same gotcha as in my PR:
https://github.com/ghybs/Leaflet/blob/ecd1c009ce4420f1d96b285343bc42560431eb19/spec/karma.conf.js#L134-L136

=> use either npm test -- -- --browsers Firefox (twice -- flag before --browsers)
or npm run test-nolint -- --browsers Firefox

…because of the nested npm script, we have to use multiple times the end of args flag "--"

Thanks for the quick review!

@ghybs ghybs closed this May 22, 2018
@ghost

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants