Switch to jsdom for tests. #935
Conversation
DylanPiercey
force-pushed
the
jsdom-tests
branch
4 times, most recently
from
f5f70d8
to
84d074a
Compare
DylanPiercey
force-pushed
the
jsdom-tests
branch
from
84d074a
to
1509ad8
Compare
1 similar comment
Codecov Report
@@ Coverage Diff @@
## master #935 +/- ##
=======================================
Coverage 90.57% 90.57%
=======================================
Files 309 309
Lines 11390 11390
=======================================
Hits 10317 10317
Misses 1073 1073Continue to review full report at Codecov.
|
.travis.yml
Outdated
| @@ -1,8 +1,8 @@ | |||
| sudo: false | |||
| node_js: | |||
| - "4" | |||
There was a problem hiding this comment.
I realize jsdom doesn't support Node 4, but Marko 4 still does. This means we will either have to strengthen our linting rules to ensure we aren't breaking Node 4, or this is a major version.
There was a problem hiding this comment.
Yeah, Michael and I were talking about this. I agree it should be under a major. We were debating if it would make sense to just release a Marko v5 (and start releasing mostly non breaking majors more often) or I may just switch back to jsdom 9. We can hash this out tomorrow morning though.
test/__runner__/browser/index.js
Outdated
| runner.on('end', function () { | ||
| cleanup(); | ||
| runner.stats.failures.length | ||
| ? reject(new Error('')) |
There was a problem hiding this comment.
Should we report the errors? Perhaps .join them?
test/__runner__/browser/index.js
Outdated
| return new Promise(function (resolve, reject) { | ||
| window.addEventListener('load', function () { | ||
| var runner = window.MOCHA_RUNNER; | ||
| runner.on('end', function () { |
There was a problem hiding this comment.
If the runner never emits end, will the tests infinitely hang?
test/__runner__/browser/index.js
Outdated
| runner.on('end', function () { | ||
| if (shouldCover) { | ||
| var coverageFile = getCoverageFile(options.testsFile); | ||
| fs.writeFileSync(coverageFile, JSON.stringify(window.__coverage__)); |
There was a problem hiding this comment.
If writeFileSync throws, the tests will hang because the promise will never reject. Additionally, since this code is already async, why not just use fs.writeFile instead?
There was a problem hiding this comment.
Good catch. I thought of switching it but it was just left over from copy pasting.
I’ll clean that up.
test/__runner__/browser/index.js
Outdated
| @@ -124,23 +118,35 @@ function generate(options) { | |||
|
|
|||
| function runTests(options) { | |||
| return generate(options).then(generated => { | |||
| console.log(`Running ${generated.url} using mocha-phantomjs...`); | |||
| var mochaPhantomJSOptions = { useColors: true }; | |||
| var cleanup = JSDOM(fs.readFileSync(generated.url, 'utf-8'), { | |||
There was a problem hiding this comment.
Also this does not have to be sync either
2 similar comments
1 similar comment
Description
Switches test suite from phantomjs to jsdom. This decreases test run time from ~7:30 to ~1:30 on my machine. Also several (large) dev dependencies were able to be removed.
Motivation and Context
Improve amount of time it takes to run tests.
Checklist: