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: remove protoc roundtrip check from update:sample-json #10557

Merged
merged 14 commits into from
Apr 22, 2020
15 changes: 15 additions & 0 deletions .github/workflows/basics.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,20 @@ jobs:
with:
node-version: 10.x

- name: Setup protoc
uses: arduino/setup-protoc@master
with:
version: '3.7.1'

- name: Set up Python
uses: actions/setup-python@v1
with:
python-version: 2.7
- name: Install Python dependencies
run: |
python -m pip install --upgrade pip
pip install protobuf==3.7.1

# Cache yarn deps. thx https://github.com/actions/cache/blob/master/examples.md#node---yarn
- name: Get yarn cache directory path
id: yarn-cache-dir-path
Expand All @@ -41,5 +55,6 @@ jobs:
- run: yarn build-all
- run: yarn diff:sample-json
- run: yarn lint
- run: yarn test-proto # Run before unit-core because the roundtrip json is needed for proto tests.
- run: yarn unit-core
- run: yarn tsc -p .
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,5 @@ proto/scripts/*_processed.json

# require any lock file to be checked in explicitly
yarn.lock

proto/sample_v2_round_trip.json
8 changes: 3 additions & 5 deletions lighthouse-core/test/report/html/renderer/psi-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const fs = require('fs');

const jsdom = require('jsdom');

const testUtils = require('../../../test-utils.js');
const prepareLabData = require('../../../../report/html/renderer/psi.js');
const Util = require('../../../../report/html/renderer/util.js');
const I18n = require('../../../../report/html/renderer/i18n.js');
Expand All @@ -19,11 +20,8 @@ const DetailsRenderer = require('../../../../report/html/renderer/details-render
const CriticalRequestChainRenderer =
require('../../../../report/html/renderer/crc-details-renderer.js');

const {itIfProtoExists, sampleResultsRoundtripStr} = testUtils.getProtoRoundTrip();
const sampleResultsStr = fs.readFileSync(__dirname + '/../../../results/sample_v2.json', 'utf-8');
const sampleResultsRoundtripStr = fs.readFileSync(
__dirname + '/../../../../../proto/sample_v2_round_trip.json',
'utf-8'
);

const TEMPLATE_FILE = fs.readFileSync(
__dirname + '/../../../../report/html/templates.html',
Expand Down Expand Up @@ -64,7 +62,7 @@ describe('DOM', () => {

describe('psi prepareLabData helpers', () => {
describe('prepareLabData', () => {
it('succeeds with LHResult object (roundtrip) input', () => {
itIfProtoExists('succeeds with LHResult object (roundtrip) input', () => {
const roundTripLHResult = /** @type {LH.Result} */ JSON.parse(sampleResultsRoundtripStr);
const result = prepareLabData(roundTripLHResult, document);

Expand Down
9 changes: 6 additions & 3 deletions lighthouse-core/test/report/proto-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,16 @@
*/
'use strict';

const testUtils = require('../test-utils.js');
const sampleJson = require('../results/sample_v2.json');
const roundTripJson = require('../../../proto/sample_v2_round_trip.json');
const preprocessor = require('../../lib/proto-preprocessor.js');

/* eslint-env jest */

describe('round trip JSON comparison subsets', () => {
const {describeIfProtoExists, sampleResultsRoundtripStr} = testUtils.getProtoRoundTrip();
const roundTripJson = sampleResultsRoundtripStr && JSON.parse(sampleResultsRoundtripStr);

describeIfProtoExists('round trip JSON comparison subsets', () => {
let processedLHR;

beforeEach(() => {
Expand Down Expand Up @@ -51,7 +54,7 @@ describe('round trip JSON comparison subsets', () => {
});
});

describe('round trip JSON comparison to everything', () => {
describeIfProtoExists('round trip JSON comparison to everything', () => {
let processedLHR;

beforeEach(() => {
Expand Down
36 changes: 36 additions & 0 deletions lighthouse-core/test/test-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

/* eslint-env jest */

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

Expand Down Expand Up @@ -41,3 +42,38 @@ expect.extend({
return toBeCloseTo.call(thisObj, ...args);
},
});

function getProtoRoundTrip() {
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
let sampleResultsRoundtripStr;
let describeIfProtoExists;
let itIfProtoExists;
try {
sampleResultsRoundtripStr =
fs.readFileSync(__dirname + '/results/sample_v2_round_trip.json', 'utf-8');
describeIfProtoExists = describe;
itIfProtoExists = it;
} catch (err) {
if (process.env.GITHUB_ACTIONS) {
throw new Error('sample_v2_round_trip must be generated for CI proto test');
}
// Otherwise no proto roundtrip for tests, so skip them.
// This is fine for running the tests locally.

itIfProtoExists = (...args) => {
// eslint-disable-next-line no-console
console.warn('Skipping test - you need to run yarn test-proto first.');
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
it.skip(...args);
};
describeIfProtoExists = describe.skip;
}

return {
itIfProtoExists,
describeIfProtoExists,
sampleResultsRoundtripStr,
};
}

module.exports = {
getProtoRoundTrip,
};
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"test-lantern": "bash lighthouse-core/scripts/test-lantern.sh",
"test-legacy-javascript": "bash lighthouse-core/scripts/test-legacy-javascript.sh",
"test-docs": "yarn --cwd docs/recipes/auth && jest docs/recipes/integration-test && yarn --cwd docs/recipes/custom-gatherer-puppeteer test",
"test-proto": "yarn compile-proto && yarn build-proto-roundtrip",
"unit-core": "jest \"lighthouse-core/\"",
"unit-cli": "jest \"lighthouse-cli/\"",
"unit-viewer": "jest \"lighthouse-viewer/.*-test.js\"",
Expand Down Expand Up @@ -64,7 +65,7 @@
"i18n:checks": "./lighthouse-core/scripts/i18n/assert-strings-collected.sh",
"i18n:collect-strings": "node lighthouse-core/scripts/i18n/collect-strings.js",
"update:sample-artifacts": "node lighthouse-core/scripts/update-report-fixtures.js",
"update:sample-json": "yarn i18n:collect-strings && node ./lighthouse-cli -A=./lighthouse-core/test/results/artifacts --config-path=./lighthouse-core/test/results/sample-config.js --output=json --output-path=./lighthouse-core/test/results/sample_v2.json && node lighthouse-core/scripts/cleanup-LHR-for-diff.js ./lighthouse-core/test/results/sample_v2.json --only-remove-timing && yarn compile-proto && yarn build-proto-roundtrip",
"update:sample-json": "yarn i18n:collect-strings && node ./lighthouse-cli -A=./lighthouse-core/test/results/artifacts --config-path=./lighthouse-core/test/results/sample-config.js --output=json --output-path=./lighthouse-core/test/results/sample_v2.json && node lighthouse-core/scripts/cleanup-LHR-for-diff.js ./lighthouse-core/test/results/sample_v2.json --only-remove-timing",
"diff:sample-json": "yarn i18n:checks && bash lighthouse-core/scripts/assert-golden-lhr-unchanged.sh",
"ultradumbBenchmark": "./lighthouse-core/scripts/benchmark.js",
"mixed-content": "./lighthouse-cli/index.js --chrome-flags='--headless' --preset=mixed-content",
Expand Down
Loading