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

tests: use workers, Mocha node api instead of calling the CLI #14215

Merged
merged 13 commits into from
Jul 18, 2022

Conversation

connorjclark
Copy link
Collaborator

This should make the annoyances of Windows shell usage go away.

Also a first step for injecting our own "setup" scripts before certain tests, to make mocking ES modules simpler to write / less error prone. That requires full control on when modules are evaluated, which this PR gives us.

@connorjclark connorjclark requested a review from a team as a code owner July 14, 2022 23:06
@connorjclark connorjclark requested review from adamraine and removed request for a team July 14, 2022 23:06
import {Connection} from '../../gather/connections/connection.js';
import {fnAny, mockCommands} from '../test-utils.js';

const {createMockSendCommandFn} = mockCommands;

// Some imports needs to be done dynamically, so that their dependencies will be mocked.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this was 100% unnecessary, and actually triggers a bug in testdouble I'm in the middle of fixing

lighthouse-core/test/scripts/run-mocha-tests.js Outdated Show resolved Hide resolved
lighthouse-core/test/scripts/run-mocha-tests.js Outdated Show resolved Hide resolved

for (const test of testsToRunIsolated) {
console.log(`Running test in isolation: ${test}`);
const worker = new Worker(new URL(import.meta.url), {
Copy link
Member

Choose a reason for hiding this comment

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

Why does this need a new URL?

Copy link
Collaborator Author

@connorjclark connorjclark Jul 18, 2022

Choose a reason for hiding this comment

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

image

Because urls and esm module paths are maddeningly incompatible... even with itself.

@connorjclark connorjclark changed the title tests: use workers and Mocha node api instead of calling the CLI tests: use workers, Mocha node api instead of calling the CLI Jul 18, 2022
@connorjclark connorjclark merged commit 295df1c into master Jul 18, 2022
@connorjclark connorjclark deleted the mocha-runner branch July 18, 2022 18:23
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.

None yet

3 participants