Consolidate mocha configurations into config file. #1469
Conversation
yifanyang
requested review from
hiranya911,
hsubox76,
mikelehen,
ryanpbrewster,
schmidt-sebastian and
tonymeng
as code owners
| # - https://github.com/karma-runner/karma-mocha for more information on all mocha flags which the | ||
| # karma runner supports. | ||
| --timeout 20000 | ||
| --retry 3 |
There was a problem hiding this comment.
These are coming from mocha.base.js, which gets deleted in this pr.
There was a problem hiding this comment.
Right now our node test and browser test use different mocha options, which leads to mocha.browser.opts and mocha.node.opts coexist in this pr. The discrepancies are about timeout and retry. Does it make sense to use same timeout and retry for both node and browser test? That way we can have only one copy of the config file.
|
@Feiyang1 Can you please invite other folks who you think should also weigh in to review this pr? |
|
I'd be fine with trying a single config for node/browser and see if we run into issues. Just to be clear, before this PR we didn't have a shared config for node, right (mocha.base.js was only being used for browser) and so we were just getting whatever the mocha defaults were (except for the packages that are explicitly passing timeouts on the command-line)? On that note, I think we should remove all of the timeouts / retries being passed on the command-line. I think those were only to override the mocha default, since we didn't have a shared mocha config for node. |
|
Yes. mocha.base.js was only meant for browser tests. For node tests that don't set timeouts or retries on the command line, they will get mocha default. Even with the config file, the test itself can still override it by setting options on the cli explicitly. |
|
I'm fine with sharing the same config for browser and nodejs. To Michael's point, let's move all the config options into the mocha config file. |
yifanyang
force-pushed
the
yifany/mocha
branch
from
4571916 to
22f98db
Compare
yifanyang
force-pushed
the
yifany/mocha
branch
from
22f98db to
a4ddf8d
Compare
|
I have taken a closer look. There are some test options (e.g. So in our case, we probably will have to keep two mocha config files -- one for node and the other for browser. Also, this way we could use different test options for node and browser, which is also the way we have been doing since the beginning. |
|
Okay. I'm okay with 2 config files since node tests and browser tests need different parameters. Thanks. |
No description provided.