search

Meteor-Community-Packages / meteor-browser-tests

A helper package for Meteor test driver packages. Runs client tests in a headless browser.
MIT License
12 stars 21 forks source link

Increase default waitTimeout for Nightmare #28

Closed zodern closed 4 years ago

zodern commented 4 years ago

When .wait times out, meteortesting:mocha will not automatically exit the app after the tests finish. This PR increases the default to 30 minutes to reduce the possibility of an app encountering this.

SimonSimCity commented 4 years ago

I would in one way agree - because this would require less configuration upfront, on another way I'd say it's a false assumption that would throw off on a slower device and already take an opinionated step for the user (https://github.com/meteortesting/meteor-browser-tests/pull/27#issuecomment-596175532). This to me mainly boils down to how the browser is though to be operated. If it's thought to be initiated per test, then your change is quite valid, because we run quite a number of tests in there. Sadly, you can't disable the timeout, but you could set it inappropriately high. Since Mocha already should fail if a test exceeds the timeout, it would be an alternative to set this timeout to an almost infinite number instead of limiting it.

If you think this should be changed, then you should at least also update the documentation: https://github.com/meteortesting/meteor-browser-tests/pull/27/files#diff-04c6e90faac2675aa89e2176d2eec7d8R101

SimonSimCity commented 4 years ago

Personally, I wouldn't remove the section from the readme, but rather update it. Or do you see it as unnecessary to mention it at all? If you'd say so, I'd rather opt in for removing the environment variable from the code as well - otherwise we'll end up with undocumented features 😓

zodern commented 4 years ago

Personally, I wouldn't remove the section from the readme, but rather update it. Or do you see it as unnecessary to mention it at all?

I can't think of a reason why it would be useful. I changed the default to be close to the maximum timeout v8 supports. If the user sets it to a lower value, the only thing that could happen is the timeout is reached and it never detects when the tests finished to automatically exit the app.