Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

test: fix node v20 tests #789

Merged
merged 9 commits into from
Aug 22, 2023
Merged

test: fix node v20 tests #789

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
170db6b
test: fix node v20 tests
AdnoC Aug 9, 2023
5169f3b
fix tests to actually use the tempy
AdnoC Aug 9, 2023
a54c006
merge cwd and dirname args
AdnoC Aug 9, 2023
8569edd
test for axe.js in cwd
AdnoC Aug 9, 2023
c649639
refine tests
AdnoC Aug 9, 2023
89c8b70
remove mock-fs typ3es
AdnoC Aug 9, 2023
aa42f35
bump chromedriver
AdnoC Aug 10, 2023
d74a895
Update packages/cli/src/lib/utils.test.ts
AdnoC Aug 22, 2023
1d7ef6d
Update packages/cli/src/lib/utils.test.ts
AdnoC Aug 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 7 additions & 23 deletions packages/cli/package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions packages/cli/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,9 @@
"chai": "^4.3.6",
"execa": "5.1.0",
"mocha": "^10.0.0",
"mock-fs": "^5.1.4",
"nyc": "^15.1.0",
"rimraf": "^3.0.2",
"tempy": "1.0.0",
"tempy": "^1.0.0",
"ts-node": "^10.9.1",
"typescript": "^4.8.4"
},
Expand Down
36 changes: 23 additions & 13 deletions packages/cli/src/lib/utils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import 'mocha';
import { assert } from 'chai';
import mock = require('mock-fs');
import tempy from 'tempy';
import { join } from 'path';
import { mkdirSync, writeFileSync } from 'fs';
import { dependencies } from '../../package.json';
import * as utils from './utils';

Expand Down Expand Up @@ -73,25 +75,33 @@ describe('utils', () => {
});

describe('getAxeSource', () => {
describe('mock file', () => {
beforeEach(() => {
mock({
'/node_modules/axe-core': {},
'../node_modules/axe-core': {
'axe.js': mock.load(require.resolve('axe-core'))
}
});
describe.only('mock file', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stray .only

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(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)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use https://www.npmjs.com/package/eslint-plugin-mocha elsewhere, which offers many other useful rules.

let dirname = '<NOT A FILE>';
before(() => {
const axeVersionCheck = dependencies['axe-core'].replace('^', '');
const tempDir = tempy.directory();
mkdirSync(join(tempDir, 'node_modules'));
mkdirSync(join(tempDir, 'node_modules', 'axe-core'));
writeFileSync(
join(tempDir, 'node_modules', 'axe-core', 'axe.js'),
`"${axeVersionCheck}"`
);

mkdirSync(join(tempDir, 'packages'));
mkdirSync(join(tempDir, 'packages', 'cli'));
mkdirSync(join(tempDir, 'packages', 'cli', 'node_modules'));
mkdirSync(join(tempDir, 'packages', 'cli', 'node_modules', 'axe-core'));
dirname = join(tempDir, 'packages', 'cli', 'lib');
mkdirSync(dirname);
});

afterEach(() => {
mock.restore();
});
it('fall back to use `locally` installed axe-core', () => {
const axeSource = utils.getAxeSource();
const axeSource = utils.getAxeSource(undefined, dirname, dirname);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 places axe.js in 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 places axe.js in 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)

const axeVersionCheck = dependencies['axe-core'].replace('^', '');
assert.include(axeSource, axeVersionCheck);
});
});

it('given no axe source use local source', () => {
const axeSource = utils.getAxeSource();
assert.isNotNull(axeSource);
Expand Down
20 changes: 16 additions & 4 deletions packages/cli/src/lib/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,28 +70,40 @@ export const parseBrowser = (browser?: string): string | Error => {
}
};

export const getAxeSource = (axePath?: string): string | void => {
export const getAxeSource = (
axePath?: string,
cwd?: string,
dirname?: string
): string | void => {
// Abort if axePath should exist, and it isn't
if (axePath && !fs.existsSync(axePath)) {
return;
}

if (!dirname) {
dirname = __dirname;
}

if (!cwd) {
cwd = process.cwd();
}

// Look for axe in current working directory
if (!axePath) {
axePath = path.join(process.cwd(), 'axe.js');
axePath = path.join(cwd, 'axe.js');
}

if (!fs.existsSync(axePath)) {
// Look for axe in CWD ./node_modules
axePath = path.join(process.cwd(), 'node_modules', 'axe-core', 'axe.js');
axePath = path.join(cwd, 'node_modules', 'axe-core', 'axe.js');
}

if (!fs.existsSync(axePath)) {
// `__dirname` is /@axe-core/cli/dist/src/lib when installed globally
// to access the locally installed axe-core package we need to go up 3 levels
// if all else fails, use the locally installed axe
axePath = path.join(
__dirname,
dirname,
'..',
'..',
'..',
Expand Down
Loading