test: fix node v20 tests #789
Conversation
packages/cli/src/lib/utils.test.ts
Outdated
| it('fall back to use `locally` installed axe-core', () => { | ||
| const axeSource = utils.getAxeSource(); | ||
| const axeSource = utils.getAxeSource(undefined, dirname, dirname); |
There was a problem hiding this comment.
With this update, I feel a bit confused about what this test is trying to actually verify - I'm not sure there's any meaningful real case where cwd === dirname in practice. I think it would be preferable to break this out into separate test cases for:
it('uses axe.js from the working directory if it exists')(where test setup placesaxe.jsin all 3 locations with different version numbers, to verify which version is actually found)it("falls back to axe-core from the working directory's node_modules if axe.js doesn't exist in the working directory")(where test setup placesaxe.jsin both the dirname and cwd based node_modules paths, but not in cwd, again with different versions to verify which one is found)it("falls back to axe-core from our own package's node_modules if no working-directory based implementation exists")(where test setup only populates the dirname based node_modules path)
packages/cli/src/lib/utils.test.ts
Outdated
| 'axe.js': mock.load(require.resolve('axe-core')) | ||
| } | ||
| }); | ||
| describe.only('mock file', () => { |
There was a problem hiding this comment.
(the axe-core repo uses eslint-plugin-mocha-no-only to prevent this class of mistake from being merged accidentally, maybe would be good for a separate PR to add that in this repo as well)
There was a problem hiding this comment.
We use https://www.npmjs.com/package/eslint-plugin-mocha elsewhere, which offers many other useful rules.
| "version": "4.7.1", | ||
| "resolved": "https://registry.npmjs.org/@axe-core/webdriverjs/-/webdriverjs-4.7.1.tgz", | ||
| "integrity": "sha512-Cf3gFDKYkMDo1vO9QQ3uIRP/iyiacxbu6YWFl0LqKG3u4AtUeR64xZ5weexr5xAKixu/sDQmcpjLskkgO9QK9w==", | ||
| "version": "4.7.3", |
There was a problem hiding this comment.
This is an example of the type of stray package-lock.json update I mentioned finding concerning in standup earlier - I can live with including it here, just noting it for ease of referencing in retro tomorrow
AdnoC
dismissed
dbjorge’s stale review
Refined tests to check specific axe.js files
| @@ -38,24 +38,21 @@ | |||
| }, | |||
There was a problem hiding this comment.
Why was this file changed?
| it("falls back to axe-core from the working directory's node_modules if axe.js doesn't exist in the working directory", () => { | ||
| const { cliDirname, cwdDirname } = setupTree(); | ||
| rmSync(join(cwdDirname, 'axe.js')); | ||
| const axeSource = utils.getAxeSource(undefined, cliDirname); |
There was a problem hiding this comment.
This should be cwdDirname shouldn't it (the working directory)?
| const axeSource = utils.getAxeSource(undefined, cliDirname); | |
| const axeSource = utils.getAxeSource(undefined, cwdDirname); |
There was a problem hiding this comment.
Nope. Should be cliDirname because we need a directory with node_modules in it.
Co-authored-by: Steven Lambert <2433219+straker@users.noreply.github.com>
Co-authored-by: Steven Lambert <2433219+straker@users.noreply.github.com>
No description provided.