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.

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

tests: start replacing jest with mocha, starting with unit-cli #13568

Closed
wants to merge 39 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
e15d486
initial. external snapshots
connorjclark Nov 17, 2021
b57b6d6
inline snapshot
connorjclark Nov 17, 2021
5f1f6be
one snapshot state per test file
connorjclark Nov 18, 2021
2bd3231
maintain map of snapshot states by test file
connorjclark Nov 18, 2021
f92351a
simplify
connorjclark Nov 18, 2021
e10c0ac
use testdouble to mock modules
connorjclark Nov 23, 2021
2f7f891
yarn unit-cli-mocha mostly works
connorjclark Nov 23, 2021
c4d591b
comment
connorjclark Nov 23, 2021
d4d4830
Merge remote-tracking branch 'origin/master' into mocha
connorjclark Jan 13, 2022
5b2b8b0
fix beforeAll/afterAll timeouts
connorjclark Jan 13, 2022
5b03510
fix config test
connorjclark Jan 13, 2022
b553be9
update
connorjclark Jan 13, 2022
e7eb722
fix types
connorjclark Jan 13, 2022
8f8eaea
update
connorjclark Jan 13, 2022
b084502
ci
connorjclark Jan 13, 2022
939e1d0
todo, rm log
connorjclark Jan 13, 2022
dbb8e60
comment
connorjclark Jan 13, 2022
2b3e56b
patch mocha globals
connorjclark Jan 13, 2022
4f96ce4
dev
connorjclark Jan 13, 2022
d00d94a
typecheck
connorjclark Jan 13, 2022
071ae8e
update
connorjclark Jan 13, 2022
b311566
update
connorjclark Jan 13, 2022
71198a5
Merge remote-tracking branch 'origin/master' into mocha
connorjclark Feb 7, 2022
0cebb33
print instruction on snapshot failure
connorjclark Feb 7, 2022
7f0a250
run-tests.js
connorjclark Feb 7, 2022
1092625
update snapshhots
connorjclark Feb 7, 2022
9bea135
Merge remote-tracking branch 'origin/master' into mocha
connorjclark May 17, 2022
7e3912d
update testdouble and rm hacky workaround
connorjclark May 17, 2022
e4d3a62
--testMatch
connorjclark May 17, 2022
b3c9ecf
update
connorjclark May 17, 2022
39eef45
gitignore
connorjclark May 17, 2022
4cff085
mochaPassThruArgs
connorjclark May 17, 2022
8eedc78
ok
connorjclark May 17, 2022
6656a0b
--parallel
connorjclark May 17, 2022
74336c5
yml
connorjclark May 17, 2022
c8c69a0
lint
connorjclark May 17, 2022
b497806
add custom expect matchers to mocha
connorjclark May 17, 2022
8ab1d52
filter based on abs path
connorjclark May 17, 2022
bc1c959
Merge remote-tracking branch 'origin/master' into mocha
connorjclark May 21, 2022
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
2 changes: 2 additions & 0 deletions .github/workflows/devtools.yml
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ jobs:
- name: Download DevTools Frontend
run: bash $GITHUB_WORKSPACE/lighthouse/lighthouse-core/test/chromium-web-tests/download-devtools.sh

- run: npx patch-package # TODO: remove when mocha replaces jest
working-directory: ${{ github.workspace }}/lighthouse
- name: Roll Lighthouse + build DevTools
run: bash $GITHUB_WORKSPACE/lighthouse/lighthouse-core/test/chromium-web-tests/roll-devtools.sh

Expand Down
3 changes: 1 addition & 2 deletions .github/workflows/unit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,7 @@ jobs:
- run: yarn install --frozen-lockfile --network-timeout 1000000
- run: yarn build-report

- name: yarn unit-cli
run: yarn unit-cli
- run: yarn unit-cli
- run: yarn diff:sample-json

# Fail if any changes were written to any source files or generated untracked files (ex, from -GA).
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ lcov.info
lighthouse-cli/results
results.html
last-run-results.html
/lighthouse-plugin-simple

*.trace.json
*.devtoolslog.json
Expand Down
1 change: 0 additions & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ module.exports = {
testEnvironment: 'node',
testMatch: [
'**/lighthouse-core/**/*-test.js',
'**/lighthouse-cli/**/*-test.js',
'**/report/**/*-test.js',
'**/lighthouse-core/test/fraggle-rock/**/*-test-pptr.js',
'**/treemap/**/*-test.js',
Expand Down
618 changes: 313 additions & 305 deletions lighthouse-cli/cli-flags.js

Large diffs are not rendered by default.

54 changes: 26 additions & 28 deletions lighthouse-cli/test/cli/bin-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,42 +6,40 @@

Copy link
Member

Choose a reason for hiding this comment

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

eslint-env mocha now?

import fs from 'fs';

import {jest} from '@jest/globals';
import * as td from 'testdouble';
import jestMock from 'jest-mock';

import {LH_ROOT} from '../../../root.js';
import {LH_ROOT, readJson} from '../../../root.js';

const mockRunLighthouse = jest.fn();

jest.unstable_mockModule('../../run.js', () => {
return {runLighthouse: mockRunLighthouse};
});

const mockGetFlags = jest.fn();
jest.unstable_mockModule('../../cli-flags.js', () => {
return {getFlags: mockGetFlags};
});

const mockAskPermission = jest.fn();
jest.unstable_mockModule('../../sentry-prompt.js', () => {
return {askPermission: mockAskPermission};
});

const mockSentryInit = jest.fn();
jest.mock('../../../lighthouse-core/lib/sentry.js', () => {
return {init: mockSentryInit};
});

const mockLoggerSetLevel = jest.fn();
jest.unstable_mockModule('lighthouse-logger', () => {
return {default: {setLevel: mockLoggerSetLevel}};
});
const mockRunLighthouse = jestMock.fn();
const mockGetFlags = jestMock.fn();
const mockAskPermission = jestMock.fn();
const mockSentryInit = jestMock.fn();
const mockLoggerSetLevel = jestMock.fn();

/** @type {import('../../bin.js')} */
let bin;
beforeAll(async () => {
await td.replaceEsm('../../run.js', {
runLighthouse: mockRunLighthouse,
});
await td.replaceEsm('../../cli-flags.js', {
getFlags: mockGetFlags,
});
await td.replaceEsm('../../sentry-prompt.js', {
askPermission: mockAskPermission,
});
await td.replace('../../../lighthouse-core/lib/sentry.js', {
init: mockSentryInit,
});
await td.replaceEsm('lighthouse-logger', undefined, {setLevel: mockLoggerSetLevel});
bin = await import('../../bin.js');
});

afterAll(async () => {
await td.reset();
});

/** @type {LH.CliFlags} */
let cliFlags;

Expand Down Expand Up @@ -177,7 +175,7 @@ describe('CLI bin', function() {
await bin.begin();

expect(getRunLighthouseArgs()[1]).toMatchObject({
precomputedLanternData: (await import(lanternDataFile)).default,
precomputedLanternData: readJson(lanternDataFile),
precomputedLanternDataPath: lanternDataFile,
});
});
Expand Down
11 changes: 4 additions & 7 deletions lighthouse-cli/test/cli/cli-flags-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@
import {strict as assert} from 'assert';
import fs from 'fs';

import yargs from 'yargs';

import {getFlags} from '../../cli-flags.js';
import {getFlags, getYargsParser} from '../../cli-flags.js';
import {LH_ROOT} from '../../../root.js';

/**
Expand Down Expand Up @@ -37,18 +35,17 @@ function snapshot(flags) {

describe('CLI flags', function() {
it('all options should have descriptions', () => {
getFlags('chrome://version');
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

with jest import yargs from 'yargs here and in the test file results in yargs.getGroups() working in the test file. That function is added to the module dynamically after being called, which is why this test did this seemingly no-op call to getFlags.

But, that property is only added to the module object in jest because ... reasons? The modules aren't really es modules, I guess, and es module objects are read-only, IIRC.

Anyhow, the getGroups method is also added to the yargs parser object, so to get to that I split up the implementation across two functions.


const parser = getYargsParser();
// @ts-expect-error - getGroups is private
const optionGroups = yargs.getGroups();
const optionGroups = parser.getGroups();
/** @type {string[]} */
const allOptions = [];
Object.keys(optionGroups).forEach(key => {
allOptions.push(...optionGroups[key]);
});
const optionsWithDescriptions =
// @ts-expect-error - getUsageInstance is private
Object.keys(yargs.getInternalMethods().getUsageInstance().getDescriptions());
Object.keys(parser.getInternalMethods().getUsageInstance().getDescriptions());

allOptions.forEach(opt => {
assert.ok(optionsWithDescriptions.includes(opt), `cli option '${opt}' has no description`);
Expand Down
4 changes: 3 additions & 1 deletion lighthouse-cli/test/cli/printer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import {strict as assert} from 'assert';
import fs from 'fs';

import * as Printer from '../../printer.js';
import sampleResults from '../../../lighthouse-core/test/results/sample_v2.json';
import {readJson} from '../../../root.js';

const sampleResults = readJson('lighthouse-core/test/results/sample_v2.json');

describe('Printer', () => {
it('accepts valid output paths', () => {
Expand Down
12 changes: 10 additions & 2 deletions lighthouse-cli/test/cli/run-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,15 @@ describe('CLI run', function() {
let fileResults;

beforeAll(async () => {
try {
fs.symlinkSync(
`${LH_ROOT}/lighthouse-core/test/fixtures/config-plugins/lighthouse-plugin-simple`,
`${LH_ROOT}/lighthouse-plugin-simple`
);
} catch {
// Might exist already because process was killed before test finished.
}

const url = 'http://localhost:10200/dobetterweb/dbw_tester.html';
// eslint-disable-next-line max-len
const samplev2ArtifactsPath = LH_ROOT + '/lighthouse-core/test/results/artifacts/';
Expand All @@ -38,8 +47,6 @@ describe('CLI run', function() {
const flags = getFlags([
'--output=json',
`--output-path=${filename}`,
// Jest allows us to resolve this module with no setup.
// https://github.com/GoogleChrome/lighthouse/pull/13045#discussion_r708690607
'--plugins=lighthouse-plugin-simple',
// Use sample artifacts to avoid gathering during a unit test.
`--audit-mode=${samplev2ArtifactsPath}`,
Expand All @@ -59,6 +66,7 @@ describe('CLI run', function() {

afterAll(() => {
fs.unlinkSync(filename);
fs.unlinkSync(`${LH_ROOT}/lighthouse-plugin-simple`);
});

it('returns results that match the saved results', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ describe('ExecutionContext', () => {
expect(executionContext.getContextId()).toEqual(undefined);
});

it.todo('should cache native objects in page');
it.todo('TODO: should cache native objects in page');
});

describe('.evaluateAsync', () => {
Expand Down
51 changes: 40 additions & 11 deletions lighthouse-core/test/jest-setup/setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,25 @@
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
*/

const {default: {toBeCloseTo}} = require('expect/build/matchers.js');
const expect = require('expect');

const format = require('../../../shared/localization/format.js');

// TODO: the `message` value of these matchers seems to be ignored. Ex:
//
// expect({a: 'A'}).toMatchObject({
// a: expect.toBeDisplayString('B'),
// });
//
// Error: expect(received).toMatchObject(expected)
// - Expected - 1
// + Received + 1
//
// Object {
// - "a": toBeDisplayString<B>,
// + "a": "A",
// }

expect.extend({
toBeDisplayString(received, expected) {
if (!format.isIcuMessage(received)) {
Expand Down Expand Up @@ -38,17 +53,31 @@ expect.extend({

return {message, pass};
},
toBeApproximately(received, expected, precision = 2) {
let pass = false;
let expectedDiff = 0;
let receivedDiff = 0;

if (received === Infinity && expected === Infinity) {
pass = true; // Infinity - Infinity is NaN
} else if (received === -Infinity && expected === -Infinity) {
pass = true; // -Infinity - -Infinity is NaN
} else {
expectedDiff = Math.pow(10, -precision) / 2;
receivedDiff = Math.abs(expected - received);
pass = receivedDiff < expectedDiff;
}

const message = () =>
[
`${this.utils.matcherHint('.toBeDisplayString')}\n`,
`Expected number to be close to:`,
` ${this.utils.printExpected(expected)}`,
`Received:`,
` ${this.utils.printReceived(received)}`,
].join('\n');

// Expose toBeCloseTo() so it can be used as an asymmetric matcher.
toBeApproximately(...args) {
// If called asymmetrically, a fake matcher `this` object needs to be passed
// in (see https://github.com/facebook/jest/issues/8295). There's no effect
// because it's only used for the printing of full failures, which isn't
// done for asymmetric matchers anyways.
const thisObj = (this && this.utils) ? this :
{isNot: false, promise: ''};
// @ts-expect-error
return toBeCloseTo.call(thisObj, ...args);
return {message, pass};
},
/**
* Asserts that an inspectable promise created by makePromiseInspectable is currently resolved or rejected.
Expand Down
Loading