Skip to content

tests: timing smoke test#13614

Merged
adamraine merged 11 commits into
fr-defer-audit-runnerfrom
timing-smoke-test
Feb 2, 2022
Merged

tests: timing smoke test#13614
adamraine merged 11 commits into
fr-defer-audit-runnerfrom
timing-smoke-test

Conversation

@adamraine

@adamraine adamraine commented Feb 1, 2022

Copy link
Copy Markdown
Contributor

Should be the last addition to #13569

Creates a new _includes smoke test helper to check for a subset of an array.

@adamraine adamraine requested a review from a team as a code owner February 1, 2022 19:58
@adamraine adamraine requested review from connorjclark and removed request for a team February 1, 2022 19:58
@connorjclark

Copy link
Copy Markdown
Collaborator

Is this test adding something of use beyond what sample_v2.json or the CDT layout tests provide? I only ask, because those methods are updatable automatically, while this smoke test would need to be manually changed

@adamraine

adamraine commented Feb 1, 2022

Copy link
Copy Markdown
Contributor Author

Is this test adding something of use beyond what sample_v2.json or the CDT layout tests provide? I only ask, because those methods are updatable automatically, while this smoke test would need to be manually changed

It verifies that lh:runner:gather timing entry is generated in all of our smoke environments. The sample_v2.json has this timing entry manually checked in. IMO the timing list is short enough where manually adding new timing entries shouldn't be that hard.

Comment thread lighthouse-cli/test/smokehouse/test-definitions/timing.js Outdated

@connorjclark connorjclark left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one change requested, +1 otherwise

Comment thread lighthouse-cli/test/smokehouse/test-definitions/timing.js Outdated
Comment thread lighthouse-core/runner.js
Comment thread lighthouse-cli/test/smokehouse/report-assert.js Outdated
Comment thread lighthouse-cli/test/smokehouse/report-assert.js Outdated
<meta name="viewport" content="width=device-width, initial-scale=1.0">
</head>
<body>
<h1>Just a simple page</h1>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread lighthouse-core/runner.js
});

// Calling log.timeEnd here will retroactively update artifacts.Timing.
// Must end time here so this timing entry can be stored on saved artifacts.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The smoke found its first problem 🎉 . This retroactive behavior doesn't happen in DevTools.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants