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

tests(smoke): convert core tests to single-expectations format #12819

Merged
merged 4 commits into from
Jul 23, 2021
Merged

tests(smoke): convert core tests to single-expectations format #12819

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4de7358
tests(smoke): convert core tests to single-expectations format
brendankenny Jul 21, 2021
1cf9d39
feedback
brendankenny Jul 22, 2021
53df9a1
csp IDs
brendankenny Jul 22, 2021
9c3886b
stdout/stderr ordering
brendankenny Jul 22, 2021
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: 1 addition & 1 deletion .github/workflows/smoke.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
runs-on: ubuntu-latest
env:
# The smokehouse tests run by job `test set 1`. `test set 2` will run the rest.
SMOKE_BATCH_1: a11y oopif pwa pwa2 pwa3 dbw redirects errors offline
SMOKE_BATCH_1: a11y oopif pwa dbw redirects errors offline
# Job named e.g. "Chrome stable, test set 1".
name: Chrome ${{ matrix.chrome-channel }}, batch ${{ matrix.smoke-test-invert == false && '1' || '2' }}

Expand Down
4 changes: 1 addition & 3 deletions lighthouse-cli/test/smokehouse/frontends/lib.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,14 @@
const cloneDeep = require('lodash.clonedeep');
const smokeTests = require('../test-definitions/core-tests.js');
const {runSmokehouse} = require('../smokehouse.js');
const {updateTestDefnFormat} = require('./back-compat-util.js');

/**
* @param {Smokehouse.SmokehouseLibOptions} options
*/
async function smokehouse(options) {
const {urlFilterRegex, skip, modify, ...smokehouseOptions} = options;

const updatedCoreTests = updateTestDefnFormat(smokeTests);
const clonedTests = cloneDeep(updatedCoreTests);
const clonedTests = cloneDeep(smokeTests);
const modifiedTests = [];
for (const test of clonedTests) {
if (urlFilterRegex && !test.expectations.lhr.requestedUrl.match(urlFilterRegex)) {
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-cli/test/smokehouse/smokehouse.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ async function runSmokehouse(smokeTestDefns, smokehouseOptions) {
const failingDefns = testResults.filter(result => result.failed);
if (failingDefns.length) {
const testNames = failingDefns.map(d => d.id).join(', ');
console.error(log.redify(`We have ${failingDefns.length} failing smoketest(s): ${testNames}`));
console.log(log.redify(`We have ${failingDefns.length} failing smoketest(s): ${testNames}`));
return {success: false, testResults};
}

Expand Down
8 changes: 3 additions & 5 deletions lighthouse-cli/test/smokehouse/test-definitions/a11y/expectations.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,10 @@
/* eslint-disable max-len */

/**
* @type {Array<Smokehouse.ExpectedRunnerResult>}
* @type {Smokehouse.ExpectedRunnerResult}
* Expected Lighthouse audit values for byte efficiency tests
*/
const expectations = [
{
const expectations = {
lhr: {
requestedUrl: 'http://localhost:10200/a11y/a11y_tester.html',
finalUrl: 'http://localhost:10200/a11y/a11y_tester.html',
Expand Down Expand Up @@ -692,7 +691,6 @@ const expectations = [
},
},
},
},
];
};

module.exports = expectations;
24 changes: 15 additions & 9 deletions lighthouse-cli/test/smokehouse/test-definitions/byte-efficiency/expectations.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,10 @@
'use strict';

/**
* @type {Array<Smokehouse.ExpectedRunnerResult>}
* Expected Lighthouse audit values for byte efficiency tests
* @type {Smokehouse.ExpectedRunnerResult}
* Expected Lighthouse results for byte efficiency audits.
*/
const expectations = [
{
const efficiency = {
artifacts: {
ScriptElements: [
{
Expand Down Expand Up @@ -282,8 +281,13 @@ const expectations = [
},
},
},
},
{
};

/**
* @type {Smokehouse.ExpectedRunnerResult}
* Expected Lighthouse results testing gzipped requests.
*/
const gzip = {
lhr: {
requestedUrl: 'http://localhost:10200/byte-efficiency/gzip.html',
finalUrl: 'http://localhost:10200/byte-efficiency/gzip.html',
Expand Down Expand Up @@ -316,7 +320,9 @@ const expectations = [
},
},
},
},
];
};

module.exports = expectations;
module.exports = {
efficiency,
gzip,
};
208 changes: 181 additions & 27 deletions lighthouse-cli/test/smokehouse/test-definitions/core-tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,71 +5,217 @@
*/
'use strict';

/** @type {ReadonlyArray<Smokehouse.BackCompatTestDefn>} */
const errors = require('./errors/error-expectations.js');
const pwa = require('./pwa/pwa-expectations.js');
const pwa2 = require('./pwa/pwa2-expectations.js');
const redirects = require('./redirects/expectations.js');
const seo = require('./seo/expectations.js');
const offline = require('./offline-local/offline-expectations.js');
const byte = require('./byte-efficiency/expectations.js');
const perf = require('./perf/expectations.js');
const diagnostics = require('./perf-diagnostics/expectations.js');
const lantern = require('./lantern/lantern-expectations.js');
const metrics = require('./tricky-metrics/expectations.js');
const csp = require('./csp/csp-expectations.js');

/** @type {ReadonlyArray<Smokehouse.TestDfn>} */
const smokeTests = [{
id: 'a11y',
expectations: require('./a11y/expectations.js'),
config: require('./a11y/a11y-config.js'),
}, {
id: 'errors',
expectations: require('./errors/error-expectations.js'),
id: 'errors-infinite-loop',
expectations: errors.infiniteLoop,
config: require('./errors/error-config.js'),
runSerially: true,
}, {
id: 'errors-expired-ssl',
expectations: errors.expiredSsl,
config: require('./errors/error-config.js'),
runSerially: true,
}, {
id: 'errors-iframe-expired-ssl',
expectations: errors.iframeBadSsl,
config: require('./errors/error-config.js'),
runSerially: true,
}, {
id: 'oopif',
expectations: require('./oopif/oopif-expectations.js'),
config: require('./oopif/oopif-config.js'),
}, {
id: 'pwa',
expectations: require('./pwa/pwa-expectations.js'),
id: 'pwa-airhorner',
expectations: pwa.airhorner,
config: require('./pwa/pwa-config.js'),
}, {
id: 'pwa2',
expectations: require('./pwa/pwa2-expectations.js'),
id: 'pwa-chromestatus',
expectations: pwa.chromestatus,
config: require('./pwa/pwa-config.js'),
}, {
id: 'pwa3',
expectations: require('./pwa/pwa3-expectations.js'),
id: 'pwa-svgomg',
expectations: pwa2.svgomg,
config: require('./pwa/pwa-config.js'),
}, {
id: 'pwa-caltrain',
expectations: pwa2.caltrain,
config: require('./pwa/pwa-config.js'),
}, {
id: 'pwa-rocks',
expectations: require('./pwa/pwa3-expectations.js').pwarocks,
config: require('./pwa/pwa-config.js'),
}, {
id: 'dbw',
expectations: require('./dobetterweb/dbw-expectations.js'),
config: require('./dobetterweb/dbw-config.js'),
runSerially: true, // Need access to network request assertions.
}, {
id: 'redirects',
expectations: require('./redirects/expectations.js'),
id: 'issues-mixed-content',
expectations: require('./issues/mixed-content.js'),
}, {
id: 'redirects-single-server',
Copy link
Collaborator

@connorjclark connorjclark Jul 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

how about going one step further and moving this entire object to what expectations.js exports? there is overhead in separating "this is what the test does" and "this is the id we give it and the config it uses"

then core-tests.js is just bringing it all together as an array.

(separate PR would be best)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it's a little silly to have filenames for separate files and then IDs for them in a different file. That'll be in the above-mentioned (but undescribed, sorry) next PR.

expectations: redirects.singleServer,
config: require('./redirects/redirects-config.js'),
}, {
id: 'redirects-multiple-server',
expectations: redirects.multipleServer,
config: require('./redirects/redirects-config.js'),
}, {
id: 'redirects-client-paint-server',
expectations: redirects.clientPaintServer,
config: require('./redirects/redirects-config.js'),
}, {
id: 'redirects-single-client',
expectations: redirects.singleClient,
config: require('./redirects/redirects-config.js'),
}, {
id: 'redirects-history-push-state',
expectations: redirects.historyPushState,
config: require('./redirects/redirects-config.js'),
}, {
id: 'seo',
expectations: require('./seo/expectations.js'),
id: 'seo-passing',
expectations: seo.passing,
config: require('./seo/seo-config.js'),
}, {
id: 'seo-failing',
expectations: seo.failing,
config: require('./seo/seo-config.js'),
}, {
id: 'seo-status-403',
expectations: seo.status403,
config: require('./seo/seo-config.js'),
}, {
id: 'offline',
expectations: require('./offline-local/offline-expectations.js'),
id: 'seo-tap-targets',
expectations: seo.tapTargets,
config: require('./seo/seo-config.js'),
}, {
id: 'offline-online-only',
expectations: offline.onlineOnly,
config: require('./offline-local/offline-config.js'),
runSerially: true,
}, {
id: 'offline-ready',
expectations: offline.ready,
config: require('./offline-local/offline-config.js'),
runSerially: true,
}, {
id: 'byte',
expectations: require('./byte-efficiency/expectations.js'),
id: 'offline-sw-broken',
expectations: offline.swBroken,
config: require('./offline-local/offline-config.js'),
runSerially: true,
}, {
id: 'offline-sw-slow',
expectations: offline.swSlow,
config: require('./offline-local/offline-config.js'),
runSerially: true,
}, {
id: 'byte-efficiency',
expectations: byte.efficiency,
config: require('./byte-efficiency/byte-config.js'),
runSerially: true,
}, {
id: 'byte-gzip',
expectations: byte.gzip,
config: require('./byte-efficiency/byte-config.js'),
runSerially: true,
}, {
id: 'perf',
expectations: require('./perf/expectations.js'),
id: 'perf-preload',
expectations: perf.preload,
config: require('./perf/perf-config.js'),
runSerially: true,
}, {
id: 'perf-diagnostics',
expectations: require('./perf-diagnostics/expectations.js'),
id: 'perf-budgets',
expectations: perf.budgets,
config: require('./perf/perf-config.js'),
runSerially: true,
}, {
id: 'perf-fonts',
expectations: perf.fonts,
config: require('./perf/perf-config.js'),
runSerially: true,
}, {
id: 'perf-trace-elements',
expectations: perf.traceElements,
config: require('./perf/perf-config.js'),
runSerially: true,
}, {
id: 'perf-frame-metrics',
expectations: perf.frameMetrics,
config: require('./perf/perf-config.js'),
runSerially: true,
}, {
id: 'perf-diagnostics-animations',
expectations: diagnostics.animations,
config: require('./perf-diagnostics/perf-diagnostics-config.js'),
}, {
id: 'lantern',
expectations: require('./lantern/lantern-expectations.js'),
id: 'perf-diagnostics-third-party',
expectations: diagnostics.thirdParty,
config: require('./perf-diagnostics/perf-diagnostics-config.js'),
}, {
id: 'perf-diagnostics-unsized-images',
expectations: diagnostics.unsizedImages,
config: require('./perf-diagnostics/perf-diagnostics-config.js'),
}, {
id: 'lantern-online',
expectations: lantern.online,
config: require('./lantern/lantern-config.js'),
}, {
id: 'lantern-settimeout',
expectations: lantern.setTimeout,
config: require('./lantern/lantern-config.js'),
}, {
id: 'lantern-fetch',
expectations: lantern.fetch,
config: require('./lantern/lantern-config.js'),
}, {
id: 'lantern-xhr',
expectations: lantern.xhr,
config: require('./lantern/lantern-config.js'),
}, {
id: 'lantern-idle-callback-short',
expectations: lantern.idleCallbackShort,
config: require('./lantern/lantern-config.js'),
}, {
id: 'metrics',
expectations: require('./tricky-metrics/expectations.js'),
id: 'lantern-idle-callback-long',
expectations: lantern.idleCallbackLong,
config: require('./lantern/lantern-config.js'),
}, {
id: 'metrics-tricky-tti',
expectations: metrics.trickyTti,
config: require('./tricky-metrics/no-throttling-config.js'),
}, {
id: 'metrics-tricky-tti-late-fcp',
expectations: metrics.trickyTtiLateFcp,
config: require('./tricky-metrics/no-throttling-config.js'),
}, {
id: 'metrics-delayed-lcp',
expectations: metrics.delayedLcp,
config: require('./tricky-metrics/no-throttling-config.js'),
}, {
id: 'metrics-delayed-fcp',
expectations: metrics.delayedFcp,
config: require('./tricky-metrics/no-throttling-config.js'),
}, {
id: 'metrics-debugger',
expectations: metrics.debuggerStatement,
config: require('./tricky-metrics/no-throttling-config.js'),
}, {
id: 'legacy-javascript',
Expand All @@ -89,8 +235,16 @@ const smokeTests = [{
expectations: require('./screenshot/expectations.js'),
config: require('./screenshot/screenshot-config.js'),
}, {
id: 'csp',
expectations: require('./csp/csp-expectations.js'),
id: 'csp-allow-all',
expectations: csp.allowAll,
config: require('./csp/csp-config.js'),
}, {
id: 'csp-block-all-m91',
expectations: csp.blockAllM91,
config: require('./csp/csp-config.js'),
}, {
id: 'csp-block-all',
expectations: csp.blockAll,
config: require('./csp/csp-config.js'),
}];

Expand Down
Loading