Skip to content

Commit

Permalink
fix(test): pass jest args correctly for v28/29 (#5068)
Browse files Browse the repository at this point in the history
the original goal of this effort was to remove the `any` type on
`jest-apt.ts`'s `JestTestRunner` type. doing so revealed a handful of
subtle errors in the implementation of the jest runner for stencil.

1. `jest-facade.ts#getCreateJestTestRunner` was incorrectly typed.
   _technically_ this was correct in that it returned `any`, which can
   be a function. however, we were not explicit/clear that this was a
   function being returned in the JSDoc.
2. `jest-runner.ts#createTestRunner` was incorrectly typed.
   Again, this was _technically_ correct in that it used `any`, but it
   was unclear that an anonymous class was returned by this function.
3. The anonymous class that is returned by `createTestRunner` for Jest
   28/29 was incorrectly typed. the dynamic import of `jest-runner`
   from Jest was typed as `any`, which caused the overrides of
   `runTests` to be improperly typed. in fact, it revealed we were not
   passing arguments correctly to the parent class.

by removing the dynamic import within the `jest-runner.ts`
implementations for each respective version of jest, stencil's bundler
(for the compiler itself) began to pull in additional files to the
bundle that it wasn't able to process. Jest's `jest-runner` module was
already being dynamically imported, and was deemed to externalize in
stencil's testing bundle.

STENCIL-960: Narrow Jest Runner Type
  • Loading branch information
rwaskiewicz committed Nov 16, 2023
1 parent f73aa14 commit 5c4ac32
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 26 deletions.
1 change: 1 addition & 0 deletions scripts/bundles/testing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export async function testing(opts: BuildOptions) {
'expect',
'@jest/reporters',
'jest-message-id',
'jest-runner',
'net',
'os',
'path',
Expand Down
17 changes: 11 additions & 6 deletions src/testing/jest/jest-27-and-under/jest-runner.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import type { AggregatedResult } from '@jest/test-result';
import type * as d from '@stencil/core/internal';
import { default as TestRunner } from 'jest-runner';

import type { ConfigFlags } from '../../../cli/config-flags';
import { setScreenshotEmulateData } from '../../puppeteer/puppeteer-emulate';
import type { JestTestRunner } from '../jest-apis';
import { JestTestRunnerConstructor } from '../jest-apis';
import { buildJestArgv, getProjectListFromCLIArgs } from './jest-config';

export async function runJest(config: d.ValidatedConfig, env: d.E2EProcessEnv) {
Expand Down Expand Up @@ -50,12 +51,16 @@ export async function runJest(config: d.ValidatedConfig, env: d.E2EProcessEnv) {
* Creates a Stencil test runner
* @returns the test runner
*/
export function createTestRunner(): JestTestRunner {
// The left hand side of the '??' is needed for Jest v27, the right hand side for Jest 26 and below
const TestRunner = require('jest-runner').default ?? require('jest-runner');

export function createTestRunner(): JestTestRunnerConstructor {
class StencilTestRunner extends TestRunner {
async runTests(tests: { path: string }[], watcher: any, onStart: any, onResult: any, onFailure: any, options: any) {
override async runTests(
tests: { context: any; path: string }[],
watcher: any,
onStart: any,
onResult: any,
onFailure: any,
options: any,
) {
const env = process.env as d.E2EProcessEnv;

// filter out only the tests the flags said we should run
Expand Down
13 changes: 6 additions & 7 deletions src/testing/jest/jest-28/jest-runner.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import type { AggregatedResult } from '@jest/test-result';
import type * as d from '@stencil/core/internal';
import { default as TestRunner } from 'jest-runner';

import type { ConfigFlags } from '../../../cli/config-flags';
import { setScreenshotEmulateData } from '../../puppeteer/puppeteer-emulate';
import type { JestTestRunner } from '../jest-apis';
import { JestTestRunnerConstructor } from '../jest-apis';
import { buildJestArgv, getProjectListFromCLIArgs } from './jest-config';

export async function runJest(config: d.ValidatedConfig, env: d.E2EProcessEnv) {
Expand Down Expand Up @@ -50,11 +51,9 @@ export async function runJest(config: d.ValidatedConfig, env: d.E2EProcessEnv) {
* Creates a Stencil test runner
* @returns the test runner
*/
export function createTestRunner(): JestTestRunner {
const TestRunner = require('jest-runner').default;

export function createTestRunner(): JestTestRunnerConstructor {
class StencilTestRunner extends TestRunner {
async runTests(tests: { path: string }[], watcher: any, onStart: any, onResult: any, onFailure: any, options: any) {
override async runTests(tests: { context: any; path: string }[], watcher: any, options: any) {
const env = process.env as d.E2EProcessEnv;

// filter out only the tests the flags said we should run
Expand All @@ -76,12 +75,12 @@ export function createTestRunner(): JestTestRunner {
setScreenshotEmulateData(emulateConfig, env);

// run the test for each emulate config
await super.runTests(tests, watcher, onStart, onResult, onFailure, options);
await super.runTests(tests, watcher, options);
}
} else {
// not doing e2e screenshot tests
// so just run each test once
await super.runTests(tests, watcher, onStart, onResult, onFailure, options);
await super.runTests(tests, watcher, options);
}
}
}
Expand Down
13 changes: 6 additions & 7 deletions src/testing/jest/jest-29/jest-runner.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import type { AggregatedResult } from '@jest/test-result';
import type * as d from '@stencil/core/internal';
import { default as TestRunner } from 'jest-runner';

import type { ConfigFlags } from '../../../cli/config-flags';
import { setScreenshotEmulateData } from '../../puppeteer/puppeteer-emulate';
import type { JestTestRunner } from '../jest-apis';
import { JestTestRunnerConstructor } from '../jest-apis';
import { buildJestArgv, getProjectListFromCLIArgs } from './jest-config';

export async function runJest(config: d.ValidatedConfig, env: d.E2EProcessEnv) {
Expand Down Expand Up @@ -50,11 +51,9 @@ export async function runJest(config: d.ValidatedConfig, env: d.E2EProcessEnv) {
* Creates a Stencil test runner
* @returns the test runner
*/
export function createTestRunner(): JestTestRunner {
const TestRunner = require('jest-runner').default;

export function createTestRunner(): JestTestRunnerConstructor {
class StencilTestRunner extends TestRunner {
async runTests(tests: { path: string }[], watcher: any, onStart: any, onResult: any, onFailure: any, options: any) {
override async runTests(tests: { context: any; path: string }[], watcher: any, options: any) {
const env = process.env as d.E2EProcessEnv;

// filter out only the tests the flags said we should run
Expand All @@ -76,12 +75,12 @@ export function createTestRunner(): JestTestRunner {
setScreenshotEmulateData(emulateConfig, env);

// run the test for each emulate config
await super.runTests(tests, watcher, onStart, onResult, onFailure, options);
await super.runTests(tests, watcher, options);
}
} else {
// not doing e2e screenshot tests
// so just run each test once
await super.runTests(tests, watcher, onStart, onResult, onFailure, options);
await super.runTests(tests, watcher, options);
}
}
}
Expand Down
15 changes: 13 additions & 2 deletions src/testing/jest/jest-apis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,19 @@ export type JestPreprocessor = {
getCacheKey(sourceText: string, sourcePath: string, ...args: any[]): string;
};

// TODO(STENCIL-960): Improve this typing by narrowing it
export type JestTestRunner = any;
/**
* For Stencil's purposes, an instance of a Jest `TestRunner` only needs to have an async `runTests` function.
* This does not mean that Jest does not require additional functions. However, those requirements may change from
* version-to-version of Jest. Stencil overrides the `runTests` function, and with our current design of integrating
* with Jest, require it to be overridden (for test filtering and supporting screenshot testing.
*/
export type JestTestRunner = {
runTests(...args: any[]): Promise<any>;
};
/**
* Helper type for describing a function that returns a {@link JestTestRunner}.
*/
export type JestTestRunnerConstructor = new (...args: any[]) => JestTestRunner;

/**
* This type serves as an alias for the type representing the initial configuration for Jest.
Expand Down
8 changes: 4 additions & 4 deletions src/testing/jest/jest-facade.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { JestPreprocessor, JestPresetConfig, JestPuppeteerEnvironment, JestTestRunner } from './jest-apis';
import { JestPreprocessor, JestPresetConfig, JestPuppeteerEnvironment, JestTestRunnerConstructor } from './jest-apis';

/**
* Interface for Jest-version specific code implementations that interact with Stencil.
Expand Down Expand Up @@ -56,11 +56,11 @@ export interface JestFacade {
getJestPreprocessor(): JestPreprocessor;

/**
* Retrieve a custom Stencil-Jest test runner
* Retrieve a function that returns the custom Stencil-Jest test runner
*
* @returns the test runner
* @returns a function that retrieves the test runner
*/
getCreateJestTestRunner(): JestTestRunner;
getCreateJestTestRunner(): () => JestTestRunnerConstructor;

/**
* Retrieve a function that returns the setup configuration code to run between tests.
Expand Down

0 comments on commit 5c4ac32

Please sign in to comment.