Skip to content
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

core(fr): use frame url in gather context #13699

Merged
merged 3 commits into from
Feb 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions lighthouse-cli/test/fixtures/redirects-self.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
</head>
<body>
<h1>Redirect to myself</h1>
<script>
const done = new URLSearchParams(window.location.search).has('done');
if (!done) {
const url = new URL(window.location.href);
url.searchParams.set('done', '');
window.location.replace(url);
} else {
const url = new URL(window.location.href);
url.searchParams.delete('done');
window.history.pushState({}, '', url);
}
</script>
</body>
</html>
2 changes: 2 additions & 0 deletions lighthouse-cli/test/smokehouse/core-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import redirectsHistoryPushState from './test-definitions/redirects-history-push
import redirectsMultipleServer from './test-definitions/redirects-multiple-server.js';
import redirectsSingleClient from './test-definitions/redirects-single-client.js';
import redirectsSingleServer from './test-definitions/redirects-single-server.js';
import redirectsSelf from './test-definitions/redirects-self.js';
import screenshot from './test-definitions/screenshot.js';
import seoFailing from './test-definitions/seo-failing.js';
import seoPassing from './test-definitions/seo-passing.js';
Expand Down Expand Up @@ -111,6 +112,7 @@ const smokeTests = [
redirectsMultipleServer,
redirectsSingleClient,
redirectsSingleServer,
redirectsSelf,
screenshot,
seoFailing,
seoPassing,
Expand Down
30 changes: 30 additions & 0 deletions lighthouse-cli/test/smokehouse/test-definitions/redirects-self.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/**
* @license Copyright 2022 The Lighthouse Authors. All Rights Reserved.
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
*/
'use strict';

/**
* @type {Smokehouse.ExpectedRunnerResult}
*/
const expectations = {
artifacts: {
MainDocumentContent: /Redirect to myself/,
},
lhr: {
requestedUrl: 'http://localhost:10200/redirects-self.html',
finalUrl: 'http://localhost:10200/redirects-self.html?done=',
audits: {
},
runWarnings: [
'The page may not be loading as expected because your test URL (http://localhost:10200/redirects-self.html) was redirected to http://localhost:10200/redirects-self.html?done=. Try testing the second URL directly.',
],
},
};

export default {
id: 'redirects-self',
expectations,
};

2 changes: 2 additions & 0 deletions lighthouse-core/fraggle-rock/gather/navigation-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ async function _computeNavigationResult(
async function _navigation(navigationContext) {
const artifactState = getEmptyArtifactState();
const phaseState = {
url: await navigationContext.driver.url(),
gatherMode: /** @type {const} */ ('navigation'),
driver: navigationContext.driver,
computedCache: navigationContext.computedCache,
Expand All @@ -223,6 +224,7 @@ async function _navigation(navigationContext) {
await collectPhaseArtifacts({phase: 'startInstrumentation', ...phaseState});
await collectPhaseArtifacts({phase: 'startSensitiveInstrumentation', ...phaseState});
const navigateResult = await _navigate(navigationContext);
phaseState.url = navigateResult.finalUrl;
await collectPhaseArtifacts({phase: 'stopSensitiveInstrumentation', ...phaseState});
await collectPhaseArtifacts({phase: 'stopInstrumentation', ...phaseState});
await _cleanupNavigation(navigationContext);
Expand Down
4 changes: 3 additions & 1 deletion lighthouse-core/fraggle-rock/gather/runner-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
* @property {LH.Gatherer.GatherMode} gatherMode
* @property {Map<string, LH.ArbitraryEqualityMap>} computedCache
* @property {LH.Config.Settings} settings
* @property {string} url
*/

/** @typedef {Record<string, Promise<any>>} IntermediateArtifacts */
Expand Down Expand Up @@ -74,6 +75,7 @@ async function collectPhaseArtifacts(options) {
gatherMode,
computedCache,
settings,
url,
} = options;
const priorPhase = phaseToPriorPhase[phase];
const priorPhaseArtifacts = (priorPhase && artifactState[priorPhase]) || {};
Expand All @@ -90,7 +92,7 @@ async function collectPhaseArtifacts(options) {
: /** @type {Dependencies} */ ({});

return gatherer[phase]({
url: await driver.url(),
url,
gatherMode,
driver,
baseArtifacts,
Expand Down
3 changes: 2 additions & 1 deletion lighthouse-core/fraggle-rock/gather/snapshot-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ async function snapshot(options) {

/** @type {Map<string, LH.ArbitraryEqualityMap>} */
const computedCache = new Map();
const url = await options.page.url();
const url = await driver.url();

const runnerOptions = {config, computedCache};
const artifacts = await Runner.gather(
Expand All @@ -39,6 +39,7 @@ async function snapshot(options) {
const artifactDefinitions = config.artifacts || [];
const artifactState = getEmptyArtifactState();
await collectPhaseArtifacts({
url,
phase: 'getArtifact',
gatherMode: 'snapshot',
driver,
Expand Down
7 changes: 5 additions & 2 deletions lighthouse-core/fraggle-rock/gather/timespan-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,12 @@ async function startTimespan(options) {
/** @type {Map<string, LH.ArbitraryEqualityMap>} */
const computedCache = new Map();
const artifactDefinitions = config.artifacts || [];
const requestedUrl = await options.page.url();
const requestedUrl = await driver.url();
const baseArtifacts = await getBaseArtifacts(config, driver, {gatherMode: 'timespan'});
const artifactState = getEmptyArtifactState();
/** @type {Omit<import('./runner-helpers.js').CollectPhaseArtifactOptions, 'phase'>} */
const phaseOptions = {
url: requestedUrl,
driver,
artifactDefinitions,
artifactState,
Expand All @@ -52,7 +53,9 @@ async function startTimespan(options) {

return {
async endTimespan() {
const finalUrl = await options.page.url();
const finalUrl = await driver.url();
phaseOptions.url = finalUrl;

const runnerOptions = {config, computedCache};
const artifacts = await Runner.gather(
async () => {
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/test/fraggle-rock/gather/mock-driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ function createMockDriver() {
_page: page,
_executionContext: context,
_session: session,
url: () => page.url(),
url: jest.fn(() => page.url()),
defaultSession: session,
connect: jest.fn(),
disconnect: jest.fn(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ describe('collectPhaseArtifacts', () => {
it(`should run the ${phase} phase of gatherers in ${gatherMode} mode`, async () => {
const {artifactDefinitions, gatherers} = createGathererSet();
await helpers.collectPhaseArtifacts({
url: 'https://example.com',
driver,
artifactDefinitions,
artifactState,
Expand All @@ -156,6 +157,7 @@ describe('collectPhaseArtifacts', () => {
it('should gather the artifacts', async () => {
const {artifactDefinitions} = createGathererSet();
await helpers.collectPhaseArtifacts({
url: 'https://example.com',
driver,
artifactDefinitions,
artifactState,
Expand Down Expand Up @@ -188,6 +190,7 @@ describe('collectPhaseArtifacts', () => {
];

await helpers.collectPhaseArtifacts({
url: 'https://example.com',
driver,
artifactDefinitions,
artifactState,
Expand Down Expand Up @@ -221,6 +224,7 @@ describe('collectPhaseArtifacts', () => {

const {artifactDefinitions, gatherers} = createGathererSet();
await helpers.collectPhaseArtifacts({
url: 'https://example.com',
driver,
artifactDefinitions,
artifactState,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ describe('Snapshot Runner', () => {
});

it('should collect base artifacts', async () => {
mockPage.url.mockResolvedValue('https://lighthouse.example.com/');
mockDriver.url.mockResolvedValue('https://lighthouse.example.com/');

await snapshot({page, config});
const artifacts = await mockRunner.gather.mock.calls[0][0]();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,11 @@ describe('Timespan Runner', () => {
});

it('should collect base artifacts', async () => {
mockPage.url.mockResolvedValue('https://start.example.com/');
mockDriver.url.mockResolvedValue('https://start.example.com/');

const timespan = await startTimespan({page, config});

mockPage.url.mockResolvedValue('https://end.example.com/');
mockDriver.url.mockResolvedValue('https://end.example.com/');

await timespan.endTimespan();
const artifacts = await mockRunner.gather.mock.calls[0][0]();
Expand Down