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: move cli/core to jest #5386

Merged
merged 13 commits into from
Jun 29, 2018
4 changes: 3 additions & 1 deletion .appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ test_script:
- yarn --version
- which yarn
- yarn lint
- yarn unit
- yarn unit-core --runInBand
- yarn unit-cli
- yarn unit-viewer
- yarn type-check
# FIXME: Exclude Appveyor from running `perf` smoketest until we fix the flake.
# https://github.com/GoogleChrome/lighthouse/issues/5053
Expand Down
22 changes: 22 additions & 0 deletions jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/**
* @license Copyright 2018 Google Inc. All Rights Reserved.
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
* 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.
*/
'use strict';

module.exports = {
Copy link
Member

Choose a reason for hiding this comment

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

gotta disable eslint

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

Copy link
Collaborator

@wardpeet wardpeet Jun 3, 2018

Choose a reason for hiding this comment

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

we could go for /* eslint-env jest */

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ya @wardpeet I made that change 👍 :)

collectCoverage: true,
coverageReporters: ['none'],
collectCoverageFrom: [
'**/lighthouse-core/**/*.js',
'**/lighthouse-cli/**/*.js',
'!**/test/',
'!**/scripts/',
],
testEnvironment: 'node',
testMatch: [
'**/lighthouse-core/**/*-test.js',
'**/lighthouse-cli/**/*-test.js',
],
};
Copy link
Member

Choose a reason for hiding this comment

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

let's set the timeout to 2000ms, since that's what we use in mocha. (otherwise it'll default to 5000ms)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

how important is this one? do we think 2s was a good feature? :)

in jest you do this via jest.setTimeout which requires a bootstrap file, and AFAICT cannot be done through config https://facebook.github.io/jest/docs/en/configuration.html

Copy link
Member

Choose a reason for hiding this comment

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

oic. hmmmmmmmm well nevermind then.

Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@
*/
'use strict';

/* eslint-env mocha */
/* eslint-env jest */
const assert = require('assert');

require('../../bin.js');
const getFlags = require('../../cli-flags').getFlags;

describe('CLI bin', function() {
it('all options should have descriptions', () => {
getFlags('chrome://version');
const yargs = require('yargs');

const optionGroups = yargs.getGroups();
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-cli/test/cli/index-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/
'use strict';

/* eslint-env mocha */
/* eslint-env jest */
const assert = require('assert');
const childProcess = require('child_process');
const path = require('path');
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-cli/test/cli/printer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/
'use strict';

/* eslint-env mocha */
/* eslint-env jest */
const Printer = require('../../printer.js');
const assert = require('assert');
const fs = require('fs');
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-cli/test/cli/run-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/
'use strict';

/* eslint-env mocha */
/* eslint-env jest */
const assert = require('assert');
const path = require('path');
const fs = require('fs');
Expand Down Expand Up @@ -44,7 +44,7 @@ describe('CLI run', function() {

fs.unlinkSync(filename);
});
}).timeout(20 * 1000);
}, 20 * 1000);
});

describe('flag coercing', () => {
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-cli/test/global-mocha-hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const assert = require('assert');
* Please keep them in sync.
*/

/* eslint-env mocha */
/* eslint-env jest */
let currTest;

// monkey-patch all assert.* methods
Expand Down
16 changes: 10 additions & 6 deletions lighthouse-core/report/html/renderer/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -288,12 +288,16 @@ class Util {
* @return {{file: string, hostname: string, origin: string}}
*/
static parseURL(url) {
const parsedUrl = new URL(url);
return {
file: Util.getURLDisplayName(parsedUrl),
hostname: parsedUrl.hostname,
origin: parsedUrl.origin,
};
try {
Copy link
Member

Choose a reason for hiding this comment

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

why was this needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

well we only need this or a fix on the _renderTextURL to be looser on its try/catch (we check instanceof TypeError), but it seemed like a good catch on passing invalid URLs to parseURL. We pass through several 'Unknown' strings to this function which throw

Copy link
Member

Choose a reason for hiding this comment

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

hmm, but why was it needed for this PR? If it's throwing shouldn't it have caused a failure before, too? Or is the problem something else.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

well again it isn't strictly needed, but seemed like a good idea anyhow.

full details:
in jest's jsdom env, the URL class throws an error that isn't instanceof TypeError
_renderTextURL which normally catches TypeErrors thrown by this method was then throwing

this isn't needed anymore because along the way I turned off default jsdom environment, but it seemed like a good idea to keep, it seems like you don't think so though so I'll revert :)

const parsedUrl = new URL(url);
return {
file: Util.getURLDisplayName(parsedUrl),
hostname: parsedUrl.hostname,
origin: parsedUrl.origin,
};
} catch (_) {
return {file: 'Unknown', hostname: 'Unknown', origin: 'Unknown'};
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const Audit = require('../../../audits/accessibility/accesskeys.js');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: accesskeys audit', () => {
it('generates an audit output', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const Audit = require('../../../audits/accessibility/aria-allowed-attr.js');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: aria-allowed-attr audit', () => {
it('generates an audit output', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const Audit = require('../../../audits/accessibility/aria-required-attr.js');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: aria-required-attr audit', () => {
it('generates an audit output', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const Audit = require('../../../audits/accessibility/aria-required-children.js');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: aria-required-children audit', () => {
it('generates an audit output', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const Audit = require('../../../audits/accessibility/aria-required-parent.js');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: aria-required-parent audit', () => {
it('generates an audit output', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const Audit = require('../../../audits/accessibility/aria-roles.js');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: aria-roles audit', () => {
it('generates an audit output', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const Audit = require('../../../audits/accessibility/aria-valid-attr.js');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: aria-valid-attr audit', () => {
it('generates an audit output', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const Audit = require('../../../audits/accessibility/aria-valid-attr-value.js');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: aria-valid-attr-value audit', () => {
it('generates an audit output', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const Audit = require('../../../audits/accessibility/audio-caption.js');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: audio-caption audit', () => {
it('generates an audit output', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const AxeAudit = require('../../../audits/accessibility/axe-audit');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: axe-audit', () => {
describe('audit()', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const Audit = require('../../../audits/accessibility/button-name.js');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: button-name audit', () => {
it('generates an audit output', () => {
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/test/audits/accessibility/bypass-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const Audit = require('../../../audits/accessibility/bypass.js');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: bypass audit', () => {
it('generates an audit output', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const Audit = require('../../../audits/accessibility/color-contrast.js');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: color-contrast audit', () => {
it('generates an audit output', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const Audit = require('../../../audits/accessibility/definition-list.js');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: definition-list audit', () => {
it('generates an audit output', () => {
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/test/audits/accessibility/dlitem-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const Audit = require('../../../audits/accessibility/dlitem.js');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: dlitem audit', () => {
it('generates an audit output', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const Audit = require('../../../audits/accessibility/document-title.js');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: document-title audit', () => {
it('generates an audit output', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const Audit = require('../../../audits/accessibility/duplicate-id.js');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: duplicate-id audit', () => {
it('generates an audit output', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const Audit = require('../../../audits/accessibility/frame-title.js');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: frame-title audit', () => {
it('generates an audit output', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const Audit = require('../../../audits/accessibility/html-has-lang.js');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: html-has-lang audit', () => {
it('generates an audit output', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const Audit = require('../../../audits/accessibility/html-lang-valid.js');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: html-lang-valid audit', () => {
it('generates an audit output', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const Audit = require('../../../audits/accessibility/image-alt.js');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: image-alt audit', () => {
it('generates an audit output', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const Audit = require('../../../audits/accessibility/input-image-alt.js');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: input-image-alt audit', () => {
it('generates an audit output', () => {
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/test/audits/accessibility/label-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const Audit = require('../../../audits/accessibility/label.js');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: label audit', () => {
it('generates an audit output', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const Audit = require('../../../audits/accessibility/layout-table.js');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: layout-table audit', () => {
it('generates an audit output', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const Audit = require('../../../audits/accessibility/link-name.js');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: link-name audit', () => {
it('generates an audit output', () => {
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/test/audits/accessibility/list-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const Audit = require('../../../audits/accessibility/list.js');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: list audit', () => {
it('generates an audit output', () => {
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/test/audits/accessibility/listitem-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const Audit = require('../../../audits/accessibility/listitem.js');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: listitem audit', () => {
it('generates an audit output', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const Audit = require('../../../audits/accessibility/meta-refresh.js');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: meta-refresh audit', () => {
it('generates an audit output', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const Audit = require('../../../audits/accessibility/meta-viewport.js');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: meta-viewport audit', () => {
it('generates an audit output', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const Audit = require('../../../audits/accessibility/object-alt.js');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: object-alt audit', () => {
it('generates an audit output', () => {
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/test/audits/accessibility/tabindex-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const Audit = require('../../../audits/accessibility/tabindex.js');
const assert = require('assert');

/* eslint-env mocha */
/* eslint-env jest */

describe('Accessibility: tabindex audit', () => {
it('generates an audit output', () => {
Expand Down
Loading