feat: add unit test support for bisect (#7553)

This commit is contained in:
Jan Scheffler 2021-09-14 17:21:06 +02:00 committed by GitHub
parent 686030fe0d
commit a0b1f6b401
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 81 additions and 18 deletions

View File

@ -292,12 +292,16 @@ The following steps are needed to update the Chromium version.
### Bisecting upstream changes
Sometimes, performing a Chromium roll causes tests to fail. To figure out the cause, you need to bisect Chromium revisions to figure out the earliest possible revision that changed the behavior. The script in `utils/bisect.js` can be helpful here. Given a Node.js script that calls `process.exit(1)` for bad revisions, run this from the Puppeteer repositorys root directory:
Sometimes, performing a Chromium roll causes tests to fail. To figure out the cause, you need to bisect Chromium revisions to figure out the earliest possible revision that changed the behavior. The script in `utils/bisect.js` can be helpful here. Given a pattern for one or more unit tests, it will automatically bisect the current range:
```sh
node utils/bisect.js --good 686378 --bad 706915 script.js
node utils/bisect.js --unit-test Response.fromCache
```
By default, it will use the Chromium revision in `src/revisions.ts` from the `main` branch and from the working tree to determine the range to bisect.
## Releasing to npm
Releasing to npm consists of the following phases:

View File

@ -21,7 +21,7 @@ const pptr = require('..');
const browserFetcher = pptr.createBrowserFetcher();
const path = require('path');
const fs = require('fs');
const { fork } = require('child_process');
const { fork, spawn, execSync } = require('child_process');
const COLOR_RESET = '\x1b[0m';
const COLOR_RED = '\x1b[31m';
@ -35,12 +35,16 @@ Usage:
node bisect.js --good <revision> --bad <revision> <script>
Parameters:
--good revision that is known to be GOOD
--bad revision that is known to be BAD
<script> path to the script that returns non-zero code for BAD revisions and 0 for good
--good revision that is known to be GOOD, defaults to the chromium revision in src/revision.ts in the main branch
--bad revision that is known to be BAD, defaults to the chromium revision in src/revision.ts
--no-cache do not keep downloaded Chromium revisions
--unit-test pattern that identifies a unit tests that should be checked
--script path to a script that returns non-zero code for BAD revisions and 0 for good
Example:
node utils/bisect.js --good 577361 --bad 599821 simple.js
node utils/bisect.js --unit-test test
node utils/bisect.js --good 577361 --bad 599821 --script simple.js
node utils/bisect.js --good 577361 --bad 599821 --unit-test test
`;
if (argv.h || argv.help) {
@ -48,24 +52,45 @@ if (argv.h || argv.help) {
process.exit(0);
}
if (typeof argv.good !== 'number') {
argv.good = getChromiumRevision('main');
if (typeof argv.good !== 'number') {
console.log(
COLOR_RED + 'ERROR: expected --good argument to be a number' + COLOR_RESET
COLOR_RED +
'ERROR: Could not parse current Chromium revision' +
COLOR_RESET
);
console.log(help);
process.exit(1);
}
}
if (typeof argv.bad !== 'number') {
argv.bad = getChromiumRevision();
if (typeof argv.bad !== 'number') {
console.log(
COLOR_RED + 'ERROR: expected --bad argument to be a number' + COLOR_RESET
COLOR_RED +
'ERROR: Could not parse Chromium revision in the main branch' +
COLOR_RESET
);
console.log(help);
process.exit(1);
}
}
if (!argv.script && !argv['unit-test']) {
console.log(
COLOR_RED +
'ERROR: Expected to be given a script or a unit test to run' +
COLOR_RESET
);
console.log(help);
process.exit(1);
}
const scriptPath = path.resolve(argv._[0]);
if (!fs.existsSync(scriptPath)) {
const scriptPath = argv.script ? path.resolve(argv.script) : null;
if (argv.script && !fs.existsSync(scriptPath)) {
console.log(
COLOR_RED +
'ERROR: Expected to be given a path to a script to run' +
@ -75,7 +100,7 @@ if (!fs.existsSync(scriptPath)) {
process.exit(1);
}
(async (scriptPath, good, bad) => {
(async (scriptPath, good, bad, pattern, noCache) => {
const span = Math.abs(good - bad);
console.log(
`Bisecting ${COLOR_YELLOW}${span}${COLOR_RESET} revisions in ${COLOR_YELLOW}~${
@ -88,9 +113,11 @@ if (!fs.existsSync(scriptPath)) {
const revision = await findDownloadableRevision(middle, good, bad);
if (!revision || revision === good || revision === bad) break;
let info = browserFetcher.revisionInfo(revision);
const shouldRemove = !info.local;
const shouldRemove = noCache && !info.local;
info = await downloadRevision(revision);
const exitCode = await runScript(scriptPath, info);
const exitCode = await (pattern
? runUnitTest(pattern, info)
: runScript(scriptPath, info));
if (shouldRemove) await browserFetcher.remove(revision);
let outcome;
if (exitCode) {
@ -124,7 +151,7 @@ if (!fs.existsSync(scriptPath)) {
console.log(
`RANGE: https://chromium.googlesource.com/chromium/src/+log/${fromSha}..${toSha}`
);
})(scriptPath, argv.good, argv.bad);
})(scriptPath, argv.good, argv.bad, argv['unit-test'], argv['no-cache']);
function runScript(scriptPath, revisionInfo) {
const log = debug('bisect:runscript');
@ -142,6 +169,23 @@ function runScript(scriptPath, revisionInfo) {
});
}
function runUnitTest(pattern, revisionInfo) {
const log = debug('bisect:rununittest');
log('Running unit test');
const child = spawn('npm run unit', ['--', '-g', pattern], {
stdio: ['inherit', 'inherit', 'inherit', 'ipc'],
shell: true,
env: {
...process.env,
PUPPETEER_EXECUTABLE_PATH: revisionInfo.executablePath,
},
});
return new Promise((resolve, reject) => {
child.on('error', (err) => reject(err));
child.on('exit', (code) => resolve(code));
});
}
async function downloadRevision(revision) {
const log = debug('bisect:download');
log(`Downloading ${revision}`);
@ -227,3 +271,18 @@ function fetchJSON(url) {
req.end();
});
}
function getChromiumRevision(gitRevision = null) {
const fileName = 'src/revisions.ts';
const command = gitRevision
? `git show ${gitRevision}:${fileName}`
: `cat ${fileName}`;
const result = execSync(command, {
encoding: 'utf8',
shell: true,
});
const m = result.match(/chromium: '(\d+)'/);
if (!m) return null;
return parseInt(m[1], 10);
}