-
Notifications
You must be signed in to change notification settings - Fork 6.8k
build: add karma task for easier cross-browser debugging #7796
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
Conversation
devversion
left a comment
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.
LGTM other than the one comment
tools/gulp/tasks/unit-test.ts
Outdated
| const server = new karma.Server({...({ | ||
| configFile: join(projectDir, 'test/karma.conf.js'), | ||
| autoWatch: false, | ||
| singleRun: false |
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.
We can maybe move the "default options" as a constant to the top of the file.
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.
I think that if it's moved out, it implies that it's also relevant for the test:single-run task which it's not.
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.
The only difference for the single-run task is the singleRun property. We can just have those options (e.g configFile, autoWatch) as default options and overwrite the singleRun option later as well.
That way we repeat less code and it's more obvious which options are used (also more clean)
new karma.Server({...defaultKarmaOptions, ...overwrites});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.
Makes sense. It's added.
Adds the `gulp test:static` task that is identical to `gulp test`, however it doesn't launch Chrome automatically, which is convenient for debugging any browser that isn't Chrome. Currently the debugging flow for other browsers is running the Gulp task which launches Chrome, waiting for it to finish running the tests, connect the other browser, wait for the new browser to finish and then start debugging. With the new task you can just run the Gulp task and connect and disconnect only the relevant browsers.
1124da4 to
5e5e2c1
Compare
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Adds the
gulp test:statictask that is identical togulp test, however it doesn't launch Chrome automatically, which is convenient for debugging any browser that isn't Chrome. Currently the debugging flow for other browsers is running the Gulp task which launches Chrome, waiting for it to finish running the tests, connect the other browser, wait for the new browser to finish and then start debugging. With the new task you can just run the Gulp task and connect and disconnect only the relevant browsers.