tests(runner): drop usages of legacy runner by adamraine · Pull Request #15047 · GoogleChrome/lighthouse

adamraine · 2023-05-05T00:23:48Z

Stop using Runner._gatherArtifactsFromBrowser in gatherFn
Convert LegacyResolvedConfig.fromJson calls to new initializeConfig
Remove some obsolete tests (I will call these out individually)

connorjclark · 2023-05-05T17:05:14Z

Previously the new runner was being given a gatherFn that just ran the legacy runner to do the collection (which we mocked the hell out of to get what we want). This PR stops using legacy runner to fill out the gatherFn and instead uses various bits of the navigation runner implementation. Also, legacy config stuff is replaced with calls to initializeConfig.

Is that everything?

adamraine · 2023-05-05T17:32:24Z

core/test/runner-test.js

-    });
-  });
-
-  it('rejects when given neither passes nor artifacts', async () => {


The general purpose runner is not responsible for checking this, the error is specific to _gatherArtifactsFromBrowser

What checks the equivalent now?

navigation-runner.js (sort of)

It rejects when there are no "config navigations" but there should always be 1 even if the artifacts array on the config doesn't exist or is empty.

lighthouse/core/test/gather/navigation-runner-test.js

Lines 193 to 196 in 3ba11a8

it('should throw if no navigations available', async () => {

resolvedConfig = {...resolvedConfig, navigations: null};

await expect(run()).rejects.toBeTruthy();

});

adamraine · 2023-05-05T17:32:48Z

core/test/runner-test.js

    });
  });

-  it('rejects when given an invalid trace artifact', async () => {


Trace artifact is not given special treatment by the FR runner anymore

adamraine · 2023-05-05T17:34:48Z

core/test/runner-test.js

      expect(lhr.runtimeError.message).toMatch(/did not paint any content.*\(NO_FCP\)/);
    });
-
-    it('includes a pageLoadError runtimeError over any gatherer runtimeErrors', async () => {


The general purpose runner is not responsible for this logic

what is now?

navigation-runner.js

lighthouse/core/test/gather/navigation-runner-test.js

Lines 447 to 462 in 3ba11a8

it('returns navigate errors', async () => {

const {navigation} = createNavigation();

const noFcp = new LighthouseError(LighthouseError.errors.NO_FCP);

mocks.navigationMock.gotoURL.mockImplementation(

/** @param {*} context @param {string} url */

(context, url) => {

if (url.includes('blank')) return {finalDisplayedUrl: 'about:blank', warnings: []};

throw noFcp;

}

);

const {artifacts, pageLoadError} = await run(navigation);

expect(pageLoadError).toBe(noFcp);

expect(artifacts).toEqual({});

});

adamraine · 2023-05-05T17:54:24Z

Is that everything?

Yes + removing some obsolete tests

brendankenny · 2023-05-05T17:42:59Z

core/test/runner-test.js

-    });
-  });
-
-  it('rejects when given neither passes nor artifacts', async () => {


What checks the equivalent now?

brendankenny · 2023-05-05T17:56:14Z

core/test/runner-test.js

        'user-timings',
      ],
+      artifacts: [
+        {id: 'Trace', gatherer: 'trace'},


are artifacts not optional if you're loading via auditMode?

They are optional, but initializeConfig filters out any audits because all required artifacts are missing.

brendankenny · 2023-05-05T17:59:25Z

core/test/runner-test.js

      expect(lhr.runtimeError.message).toMatch(/did not paint any content.*\(NO_FCP\)/);
    });
-
-    it('includes a pageLoadError runtimeError over any gatherer runtimeErrors', async () => {


what is now?

adamraine added 2 commits May 4, 2023 17:19

tests: convert runner test to use FR

7392cb7

no passes

89b9ff6

adamraine requested a review from a team as a code owner May 5, 2023 00:23

adamraine requested review from connorjclark and removed request for a team May 5, 2023 00:23

vercel bot deployed to Preview May 5, 2023 00:24 View deployment

devtools-bot assigned connorjclark May 5, 2023

devtools-bot added the waiting4reviewer label May 5, 2023

connorjclark changed the title ~~tests: convert runner test to use FR~~ tests: convert runner test to use new runner, drop legacy runner May 5, 2023

connorjclark changed the title ~~tests: convert runner test to use new runner, drop legacy runner~~ tests: convert runner test to use new runner instead of legacy runner May 5, 2023

adamraine commented May 5, 2023

View reviewed changes

url

88474bd

vercel bot deployed to Preview May 5, 2023 17:49 View deployment

brendankenny reviewed May 5, 2023

View reviewed changes

adamraine mentioned this pull request May 9, 2023

tests: remove most usages of afterPass #15057

Merged

Merge branch 'main' into gather-test-fr

6dae0a0

vercel bot deployed to Preview June 27, 2023 18:11 View deployment

adamraine added 2 commits June 28, 2023 17:08

test

5f90b01

Merge branch 'main' into gather-test-fr

d2bf48d

vercel bot deployed to Preview June 28, 2023 21:09 View deployment

Merge branch 'main' into gather-test-fr

ed8a543

vercel bot deployed to Preview July 5, 2023 22:57 View deployment

connorjclark approved these changes Jul 6, 2023

View reviewed changes

adamraine changed the title ~~tests: convert runner test to use new runner instead of legacy runner~~ tests(runner): convert to use new runner instead of legacy runner Jul 6, 2023

adamraine changed the title ~~tests(runner): convert to use new runner instead of legacy runner~~ tests(runner): drop usages of legacy runner Jul 6, 2023

adamraine merged commit c0efe60 into main Jul 6, 2023

adamraine deleted the gather-test-fr branch July 6, 2023 19:33

This comment was marked as spam.

Sign in to view

adamraine mentioned this pull request Jul 7, 2023

tests: remove usages of legacy driver #15230

Merged

	it('should throw if no navigations available', async () => {
	resolvedConfig = {...resolvedConfig, navigations: null};
	await expect(run()).rejects.toBeTruthy();
	});

	it('returns navigate errors', async () => {
	const {navigation} = createNavigation();
	const noFcp = new LighthouseError(LighthouseError.errors.NO_FCP);

	mocks.navigationMock.gotoURL.mockImplementation(
	/** @param {} context @param {string} url /
	(context, url) => {
	if (url.includes('blank')) return {finalDisplayedUrl: 'about:blank', warnings: []};
	throw noFcp;
	}
	);

	const {artifacts, pageLoadError} = await run(navigation);
	expect(pageLoadError).toBe(noFcp);
	expect(artifacts).toEqual({});
	});

Conversation

adamraine commented May 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

connorjclark commented May 5, 2023

Uh oh!

adamraine May 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamraine May 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamraine May 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamraine commented May 5, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as spam.

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

adamraine commented May 5, 2023 •

edited

Loading

adamraine May 5, 2023 •

edited

Loading

adamraine May 5, 2023 •

edited

Loading

adamraine May 5, 2023 •

edited

Loading