-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
deps: update yargs to latest #11794
deps: update yargs to latest #11794
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -69,31 +69,54 @@ function getDefinitionsToRun(allTestDefns, requestedIds, {invertMatch}) { | |
* CLI entry point. | ||
*/ | ||
async function begin() { | ||
const argv = yargs | ||
const rawArgv = yargs | ||
.help('help') | ||
.usage('node $0 [<options>] <test-ids>') | ||
.example('node $0 -j=1 pwa seo', 'run pwa and seo tests serially') | ||
.example('node $0 --invert-match byte', 'run all smoke tests but `byte`') | ||
.describe({ | ||
'debug': 'Save test artifacts and output verbose logs', | ||
'jobs': 'Manually set the number of jobs to run at once. `1` runs all tests serially', | ||
'retries': 'The number of times to retry failing tests before accepting. Defaults to 0', | ||
'runner': 'The method of running Lighthouse', | ||
'tests-path': 'The path to a set of test definitions to run. Defaults to core smoke tests.', | ||
'invert-match': 'Run all available tests except the ones provided', | ||
.option('_', { | ||
array: true, | ||
type: 'string', | ||
}) | ||
.boolean(['debug', 'invert-match']) | ||
.alias({ | ||
'jobs': 'j', | ||
.options({ | ||
'debug': { | ||
type: 'boolean', | ||
default: false, | ||
describe: 'Save test artifacts and output verbose logs', | ||
}, | ||
'jobs': { | ||
type: 'number', | ||
alias: 'j', | ||
describe: 'Manually set the number of jobs to run at once. `1` runs all tests serially', | ||
}, | ||
'retries': { | ||
type: 'number', | ||
describe: 'The number of times to retry failing tests before accepting. Defaults to 0', | ||
}, | ||
'runner': { | ||
default: 'cli', | ||
choices: ['cli', 'bundle'], | ||
describe: 'The method of running Lighthouse', | ||
}, | ||
'tests-path': { | ||
type: 'string', | ||
describe: 'The path to a set of test definitions to run. Defaults to core smoke tests.', | ||
}, | ||
'invert-match': { | ||
type: 'boolean', | ||
default: false, | ||
describe: 'Run all available tests except the ones provided', | ||
}, | ||
}) | ||
.choices('runner', ['cli', 'bundle']) | ||
.default('runner', 'cli') | ||
.wrap(yargs.terminalWidth()) | ||
.argv; | ||
|
||
// TODO: use .number() when yargs is updated | ||
const jobs = argv.jobs !== undefined ? Number(argv.jobs) : undefined; | ||
const retries = argv.retries !== undefined ? Number(argv.retries) : undefined; | ||
// Augmenting yargs type with auto-camelCasing breaks in [email protected] and @types/[email protected], | ||
// so for now cast to add yarg's camelCase properties to type. | ||
const argv = /** @type {typeof rawArgv & CamelCasify<typeof rawArgv>} */ (rawArgv); | ||
|
||
const jobs = Number.isFinite(argv.jobs) ? argv.jobs : undefined; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not a big deal, but |
||
const retries = Number.isFinite(argv.retries) ? argv.retries : undefined; | ||
|
||
const runnerPath = runnerPaths[/** @type {keyof typeof runnerPaths} */ (argv.runner)]; | ||
if (argv.runner === 'bundle') { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,7 +27,7 @@ const {ProgressLogger} = require('./lantern/collect/common.js'); | |
const LH_ROOT = `${__dirname}/../..`; | ||
const ROOT_OUTPUT_DIR = `${LH_ROOT}/timings-data`; | ||
|
||
const argv = yargs | ||
const rawArgv = yargs | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @connorjclark I tried to minimize the changes needed in here. I don't think I changed any semantics |
||
.help('help') | ||
.describe({ | ||
// common flags | ||
|
@@ -51,12 +51,14 @@ const argv = yargs | |
'delta-property-sort': 'Property to sort by its delta', | ||
'desc': 'Set to override default ascending sort', | ||
}) | ||
.option('n', {type: 'number', default: 1}) | ||
.string('filter') | ||
.string('url-filter') | ||
.alias({'gather': 'G', 'audit': 'A'}) | ||
.default('report-exclude', 'key|min|max|stdev|^n$') | ||
.default('delta-property-sort', 'mean') | ||
.default('output', 'table') | ||
.array('urls') | ||
.option('urls', {array: true, type: 'string'}) | ||
.string('lh-flags') | ||
.default('desc', false) | ||
.default('sort-by-absolute-value', false) | ||
|
@@ -65,6 +67,10 @@ const argv = yargs | |
.wrap(yargs.terminalWidth()) | ||
.argv; | ||
|
||
// Augmenting yargs type with auto-camelCasing breaks in [email protected] and @types/[email protected], | ||
// so for now cast to add yarg's camelCase properties to type. | ||
const argv = /** @type {typeof rawArgv & CamelCasify<typeof rawArgv>} */ (rawArgv); | ||
|
||
const reportExcludeRegex = | ||
argv.reportExclude !== 'none' ? new RegExp(argv.reportExclude, 'i') : null; | ||
|
||
|
@@ -120,12 +126,15 @@ function round(value) { | |
* @param {number} total | ||
* @return {string} | ||
*/ | ||
function getProgressBar(i, total = argv.n * argv.urls.length) { | ||
function getProgressBar(i, total = argv.n * (argv.urls || []).length) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no way to do required arrays? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
But I think we only want a required array of I was trying to leave everything mostly untouched so it would all be recognizable when you were next back in here, but happy to commit any suggestions |
||
const bars = new Array(Math.round(i * 40 / total)).fill('▄').join('').padEnd(40); | ||
return `${i + 1} / ${total} [${bars}]`; | ||
} | ||
|
||
async function gather() { | ||
if (typeof argv.name !== 'string') { | ||
throw new Error('expected entry for name option'); | ||
} | ||
const outputDir = dir(argv.name); | ||
if (fs.existsSync(outputDir)) { | ||
console.log('Collection already started - resuming.'); | ||
|
@@ -136,7 +145,7 @@ async function gather() { | |
progress.log('Gathering…'); | ||
|
||
let progressCount = 0; | ||
for (const url of argv.urls) { | ||
for (const url of argv.urls || []) { | ||
const urlFolder = `${outputDir}/${urlToFolder(url)}`; | ||
await mkdir(urlFolder, {recursive: true}); | ||
|
||
|
@@ -161,12 +170,15 @@ async function gather() { | |
} | ||
|
||
async function audit() { | ||
if (typeof argv.name !== 'string') { | ||
throw new Error('expected entry for name option'); | ||
} | ||
const outputDir = dir(argv.name); | ||
const progress = new ProgressLogger(); | ||
progress.log('Auditing…'); | ||
|
||
let progressCount = 0; | ||
for (const url of argv.urls) { | ||
for (const url of argv.urls || []) { | ||
const urlDir = `${outputDir}/${urlToFolder(url)}`; | ||
for (let i = 0; i < argv.n; i++) { | ||
const gatherDir = `${urlDir}/${i}`; | ||
|
@@ -309,6 +321,9 @@ function isNumber(value) { | |
} | ||
|
||
function summarize() { | ||
if (typeof argv.name !== 'string') { | ||
throw new Error('expected entry for name option'); | ||
} | ||
const results = aggregateResults(argv.name); | ||
print(filter(results)); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the
coerce()
functions from yargs gave a perfect place to do this loading/parsing and avoid the awkward type dance. Just moved intocli-flags