move test suite to jest #156
Conversation
MasterOdin
force-pushed
the
chore-test-suite
branch
4 times, most recently
from
b4a0aef to
a4f99ec
Compare
Signed-off-by: Matthew Peveler <[email protected]>
MasterOdin
force-pushed
the
chore-test-suite
branch
from
a4f99ec to
026df04
Compare
|
|
||
| - name: Update npm | ||
| if: matrix.node-version == '6.x' | ||
| run: npm install -g npm@^5 |
There was a problem hiding this comment.
node 6.x installs npm@3 which had issues installing all dependencies of jest. Updating to npm@5 fixes the issue, and didn't think it worth spending time trying to figure out why it wasn't working.
| run: npm install --save-dev typescript | ||
|
|
||
| - run: node_modules/.bin/tsc --strict lib/portfinder.d.ts | ||
| - run: node_modules/.bin/tsc --strict --lib es2015 lib/portfinder.d.ts |
There was a problem hiding this comment.
Needed to add --lib es2015 here as some of the dependencies that jest brings in uses es2015 features (e.g. Set, Map, etc.) and without this would get an error. Node 6+ is largely es2015 complete, so not really a compatibility risk.
| "types": "./lib/portfinder.d.ts", | ||
| "scripts": { | ||
| "test": "vows test/*-test.js --spec" | ||
| "test": "jest --runInBand" |
There was a problem hiding this comment.
Jest will try to run the test files in parallel via workers, which breaks the suite given that tests re-use the same ports. Given how quickly it runs (< 1s), I think the lack of parallelization doesn't matter.
There was a problem hiding this comment.
Jest does not allow specifying an order for test files to run, and will run them randomly. In testing, I didn't find that it would matter when this particular test would run though.
|
👀 tomorrow am. Thanks again. This should help new people a lot. |
eriktrom
left a comment
eriktrom
left a comment
There was a problem hiding this comment.
Very good work. Thank you again.
2 participants
Closes #155
PR moves the test suite over from vows (which is defunct and doesn't work properly on Node >= 10) to jest. I did have to use a somewhat older version of jest to run against older EOL versions of node. Based on #153 (comment), I chose the newest version that would still allow for testing and specifying node >= 6 in the engines field (once
asyncis updated).This does make
npm auditcomplain about a number of vulnerabilities in installed packages, but given this is just within the test suite, I think that's fine. There's also the downside that adding jest does add quite a bit of dependency tree (whereasvowswas relatively small), but I think using vitest or mocha would add similar amount of weight to the repo.I did also have to update the test files names so that the suffix was
.test.jsinstead of-test.jswhich allows jest to automatically pick up the files without having to modify thetestMatchsetting.