-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
clients(lr): run benchmark repeatedly given special query parameter #14075
Conversation
const bidxRunCount = options.settings.__internalMegaBenchmarkIndex; | ||
// Keep the first one in there | ||
const bidxes = [baseArtifacts.BenchmarkIndex]; | ||
for (let i = 0; i < bidxRunCount; i++) { |
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.
It'd be simpler if the internal setting was just minus one'd here (And we asserted it either doesn't exist or is >= 1).
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.
as we're the only users i think this isn't a huge deal. and it'd complicate the impl
if (options.settings.__internalMegaBenchmarkIndex) { | ||
const bidxRunCount = options.settings.__internalMegaBenchmarkIndex; | ||
// Keep the first one in there | ||
const bidxes = [baseArtifacts.BenchmarkIndex]; |
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.
nit: bidxes -> indexes?
@@ -61,6 +61,11 @@ export async function runLighthouseInLR(connection, url, flags, lrOpts) { | |||
config.settings.onlyCategories = categoryIDs; | |||
} | |||
} | |||
// Set to 0 to disable |
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.
Set what to 0? Who is this comment for?
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.
Oh, I think I see. Could you instead make megaBidx
be optional, or union with | false
?
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.
moved it to channel='fr' && queryParam('bidx')
instead.
I assume I can review this fine without understanding the motivation :) |
This weird mode is for some lightrider variance work underway.
Example output:
cc @adrianaixba