Skip to content

Commit

Permalink
Fix build, upgrade to v1.1.0 (#202)
Browse files Browse the repository at this point in the history
  • Loading branch information
jburger424 authored Jun 4, 2020
1 parent 1c60780 commit 30a9c37
Show file tree
Hide file tree
Showing 12 changed files with 107 additions and 239 deletions.
2 changes: 1 addition & 1 deletion lighthouse-plugin-publisher-ads/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "lighthouse-plugin-publisher-ads",
"version": "1.0.0",
"version": "1.1.0",
"description": "A Lighthouse plugin to improve ad speed and overall quality through a series of automated audits.",
"author": "Google Ads",
"license": "Apache-2.0",
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-plugin-publisher-ads/test/smoke/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,6 @@ module.exports = {
settings: {
onlyCategories: ['lighthouse-plugin-publisher-ads'],
// TODO(jburger): Use simulation once properly implemented.
throttlingMethod: 'devtools',
throttlingMethod: 'simulate',
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ module.exports = [
},
'viewport-ad-density': {
score: 0,
numericValue: '.452 +/- .001',
numericValue: '.408 +/- .001',
},
'ads-in-viewport': {
score: 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ module.exports = [
},
'first-ad-render': {
// TODO(jburger): Update when https://github.com/GoogleChrome/lighthouse/issues/9417 is implemented.
numericValue: '4.25 +/- 1',
numericValue: '5.5 +/- 1',
},
'loads-gpt-from-sgdn': {
score: 0,
Expand All @@ -38,7 +38,7 @@ module.exports = [
items: [
{
script: 'http://localhost:8081/long-tasks.html',
duration: '1010 +/- 10',
duration: '4025 +/- 25',
},
],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,10 @@ module.exports = [
scoreDisplayMode: 'informative',
// It's important that the critical path does not include the render
// blocking resources.
numericValue: 4,
numericValue: 2,
details: {
items: [
{url: 'http://localhost:8081/slowly-inject-gpt.js'},
{url: 'http://securepubads.g.doubleclick.net/tag/js/gpt.js'},
{url: 'https://securepubads.g.doubleclick.net/tag/js/gpt.js'},
// Use a Regexp to ignore the version of pubads_impl.
{url: /.*securepubads.g.doubleclick.net.*pubads_impl.*/},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,19 @@ module.exports = [
},
},
'loads-ad-tag-over-https': {
score: 0,
numericValue: 1,
score: 1,
details: {
numAdTagHttpsReqs: 1,
numAdTagHttpReqs: 0,
},
},
'ad-blocking-tasks': {
score: 0,
details: {
items: [
{
script: 'http://localhost:8081/slowly-inject-gpt.js',
duration: '1010 +/- 10',
duration: '4050 +/- 50',
},
],
},
Expand All @@ -56,8 +59,6 @@ module.exports = [
/(.*\/gpt\/pubads_impl([a-z_]*)((?<!rendering)_)\d+\.js)/, 'g'),
},
{url: 'https://securepubads.g.doubleclick.net/tag/js/gpt.js'},
{url: 'http://securepubads.g.doubleclick.net/tag/js/gpt.js'},
{url: 'http://localhost:8081/slowly-inject-gpt.js'},
],
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,5 @@ function longTask(duration) {

longTask(1000);
const gpt = document.createElement('script'); // eslint-disable-line
gpt.setAttribute('src', 'http://securepubads.g.doubleclick.net/tag/js/gpt.js');
gpt.setAttribute('src', 'https://securepubads.g.doubleclick.net/tag/js/gpt.js');
document.querySelector('head').appendChild(gpt); // eslint-disable-line
199 changes: 23 additions & 176 deletions lighthouse-plugin-publisher-ads/test/smoke/run-smoke.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2019 Google LLC
// Copyright 2020 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand All @@ -16,17 +16,9 @@
/* eslint-disable no-console */
const {promisify} = require('util'); // eslint-disable-line
const execAsync = promisify(require('child_process').exec);
const log = require('lighthouse-logger');
const StaticServer = require('static-server');
const {execSync} = require('child_process');

const SMOKEHOUSE_PATH = require.resolve('lighthouse/lighthouse-cli/test/smokehouse/smokehouse.js');
const CLI_ROOT = SMOKEHOUSE_PATH.replace('/test/smokehouse/smokehouse.js', '');
const TMP_DIR = `${CLI_ROOT}/test/smokehouse/.tmp`;
const CORE_DFNS_PATH = `${CLI_ROOT}/test/smokehouse/smoke-test-dfns.js`;
const CUSTOM_DFNS_PATH = require.resolve('./smoke-test-dfns.js');


const server = new StaticServer({
rootPath: `${__dirname}/fixtures`,
port: 8081,
Expand All @@ -37,183 +29,38 @@ const server = new StaticServer({
*/
function init() {
const cmd = [
// Backup core dfns file.
`mv ${CORE_DFNS_PATH} ${CORE_DFNS_PATH}.backup`,
// Symlink custom dfns file.
`ln -sf ${CUSTOM_DFNS_PATH} ${CORE_DFNS_PATH}`,
// Create and symlink local `.tmp` directory.
`mkdir -p .tmp`,
`ln -sf ./.tmp ${TMP_DIR}`,
// Symlink `lighthouse-cli` in root.
`ln -sf ${CLI_ROOT} .`,
'cd node_modules/lighthouse',
'ln -sf ../../lighthouse-plugin-publisher-ads ' +
'lighthouse-plugin-publisher-ads',
];
execSync(cmd.join(' && '));
}

/**
* Revert changes from init().
*/
function cleanUp() {
const cmd = [
// Remove symlinked dfns file.
`rm ${CORE_DFNS_PATH}`,
// Restore backup dfns file.
`mv ${CORE_DFNS_PATH}.backup ${CORE_DFNS_PATH}`,
// Remove local and core `.tmp` dirs.
`rm -r .tmp`,
`rm ${TMP_DIR}`,
// Remove symlinked `lighthouse-cli`.
`rm lighthouse-cli`,
];
execSync(cmd.join(' && '));
}

init();

/** @param {string} str */
const purpleify = (str) => `${log.purple}${str}${log.reset}`;
const smokeTests = require('./smoke-test-dfns.js');

/**
* Display smokehouse output from child process
* @param {{id: string, stdout: string, stderr: string, error?: Error}} result
*/
function displaySmokehouseOutput(result) {
console.log(`\n${purpleify(result.id)} smoketest results:`);
if (result.error) {
console.log(result.error.message);
}
process.stdout.write(result.stdout);
process.stderr.write(result.stderr);
console.timeEnd(`smoketest-${result.id}`);
console.log(`${purpleify(result.id)} smoketest complete. \n`);
return result;
}

/**
* Run smokehouse in child processes for the selected smoke tests
* Display output from each as soon as they finish, but resolve function when
* ALL are complete
* @param {Array<Smokehouse.TestDfn>} smokeTests
* @return {Promise<Array<{id: string, error?: Error}>>}
*/
async function runSmokehouse(smokeTests) {
const cmdPromises = [];
for (const {id} of smokeTests) {
console.log(`${purpleify(id)} smoketest starting…`);
console.time(`smoketest-${id}`);
const cmd = [
`node ${SMOKEHOUSE_PATH}`,
`--smoke-id=${id}`,
].join(' ');

// The promise ensures we output immediately, even if the process errors
const p = execAsync(cmd, {timeout: 6 * 60 * 1000, encoding: 'utf8'})
.then((cp) => ({id, ...cp}))
.catch(
(err) => ({id, stdout: err.stdout, stderr: err.stderr, error: err}))
.then((result) => displaySmokehouseOutput(result));

// If the machine is terribly slow, we'll run all smoketests in succession,
// not parallel
if (process.env.APPVEYOR) {
await p;
}
cmdPromises.push(p);
}

return Promise.all(cmdPromises);
}

/**
* Determine batches of smoketests to run, based on argv
* @param {string[]} argv
* @return {Map<string|undefined, Array<Smokehouse.TestDfn>>}
*/
function getSmoketestBatches(argv) {
let smokes = [];
const usage = ` ${log.dim}yarn smoke ${smokeTests.map((t) => t.id)
.join(' ')}${log.reset}\n`;

if (argv.length === 0) {
smokes = smokeTests;
console.log('Running ALL smoketests. Equivalent to:');
console.log(usage);
} else {
smokes = smokeTests.filter((test) => argv.includes(test.id));
console.log(`Running ONLY smoketests for: ${smokes.map((t) => t.id)
.join(' ')}\n`);
}

const unmatchedIds = argv.filter((requestedId) =>
!smokeTests.map((t) => t.id).includes(requestedId));
if (unmatchedIds.length) {
console.log(
log.redify(`Smoketests not found for: ${unmatchedIds.join(' ')}`));
console.log(usage);
}

// Split into serial batches that will run their tests concurrently
const batches = smokes.reduce((map, test) => {
const batch = map.get(test.batch) || [];
batch.push(test);
return map.set(test.batch, batch);
}, new Map());

return batches;
}

/**
* Main function. Run webservers, smokehouse, then report on failures
*/
async function cli() {
async function run() {
console.log('Running Smoke Tests');
init();
server.start(() => {
console.log('Server listening to', server.port);
console.log('Fixtures hosted on port :' + server.port);
});

const argv = process.argv.slice(2);
const batches = getSmoketestBatches(argv);

const smokeDefns = new Map();
const smokeResults = [];
for (const [batchName, batch] of batches) {
console.log(`Smoketest batch: ${batchName || 'default'}`);
for (const defn of batch) {
smokeDefns.set(defn.id, defn);
}

const results = await runSmokehouse(batch);
smokeResults.push(...results);
}

let failingTests = smokeResults.filter((result) => !!result.error);

// Automatically retry failed tests in CI to prevent flakes
if (failingTests.length && (process.env.RETRY_SMOKES || process.env.CI)) {
console.log('Retrying failed tests...');
for (const failedResult of failingTests) {
/** @type {number} */
const resultIndex = smokeResults.indexOf(failedResult);
const smokeDefn = smokeDefns.get(failedResult.id);
smokeResults[resultIndex] = (await runSmokehouse([smokeDefn]))[0];
}
}

failingTests = smokeResults.filter((result) => !!result.error);

if (failingTests.length) {
const testNames = failingTests.map((t) => t.id).join(', ');
console.error(log.redify(
`We have ${failingTests.length} failing smoketests: ${testNames}`));
cleanUp();
process.exit(1);
}
cleanUp();
process.exit(0);
execAsync(
'cd node_modules/lighthouse && ' +
'node lighthouse-cli/test/smokehouse/frontends/smokehouse-bin.js --tests-path ../../lighthouse-plugin-publisher-ads/test/smoke/smoke-test-dfns.js --retries 3'
)
.then((result) => {
process.stdout.write(result.stdout);
process.exit(0);
})
.catch((e) => {
process.stdout.write(e.stdout);
process.stderr.write(e.stderr);
process.exit(1);
});
}

cli().catch((e) => {
console.error(e);
cleanUp();
run().catch((e) => {
process.stderr.write(e.stderr);
process.exit(1);
});
7 changes: 0 additions & 7 deletions lighthouse-plugin-publisher-ads/test/smoke/smoke-test-dfns.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,43 +19,36 @@ const smokeTests = [
id: 'duplicate-tags',
expectations: require('./expectations/duplicate-tags.js'),
config: require('./config.js'),
batch: 'pub-ads',
},
{
id: 'lazy-load',
expectations: require('./expectations/lazy-load.js'),
config: require('./config.js'),
batch: 'pub-ads',
},
{
id: 'long-tasks',
expectations: require('./expectations/long-tasks.js'),
config: require('./config.js'),
batch: 'pub-ads',
},
{
id: 'render-blocking-tasks',
expectations: require('./expectations/render-blocking-tags.js'),
config: require('./config.js'),
batch: 'pub-ads',
},
{
id: 'script-injected',
expectations: require('./expectations/script-injected.js'),
config: require('./config.js'),
batch: 'pub-ads',
},
{
id: 'top-of-viewport',
expectations: require('./expectations/top-of-viewport.js'),
config: require('./config.js'),
batch: 'pub-ads',
},
{
id: 'not-applicable',
expectations: require('./expectations/not-applicable.js'),
config: require('./config.js'),
batch: 'pub-ads',
},
];

Expand Down
1 change: 1 addition & 0 deletions lighthouse-plugin-publisher-ads/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
"**/*.js",
"**/*.d.ts",
"../node_modules/lighthouse/lighthouse-core/report/html/renderer/util.js",
"../node_modules/lighthouse/lighthouse-core/report/html/renderer/i18n.js",
"../node_modules/lighthouse/lighthouse-core/lib/network-request.js",
"../node_modules/lighthouse/lighthouse-core/lib/url-shim.js",
"../node_modules/lighthouse/lighthouse-core/lib/dependency-graph/*.js"
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "lighthouse-plugin-publisher-ads-wrapper",
"version": "1.0.0",
"version": "1.1.0",
"description": "Wrapper for Publisher Ads Lighthouse Plugin.",
"author": "Google Ads",
"license": "Apache-2.0",
Expand All @@ -20,7 +20,7 @@
"dependencies": {
"@tusbar/cache-control": "^0.3.1",
"@types/esprima": "^4.0.2",
"lighthouse": "^5.6.0",
"lighthouse": "^6.0.0",
"yargs": "^13.3.0"
},
"devDependencies": {
Expand Down
Loading

0 comments on commit 30a9c37

Please sign in to comment.