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

feat(Programmatic API): Add programmatic API #14062

Open
wants to merge 43 commits into
base: main
Choose a base branch
from

Conversation

nicojs
Copy link
Contributor

@nicojs nicojs commented Apr 9, 2023

Summary

This pull request adds a documented programmatic API to run Jest. It can be used to run using the new experimental API.

import {createJest} from 'jest';

const jest = await createJest();
jest.globalConfig = {
  collectCoverage: false,
  ...jest.globalConfig,
};
const {results} = await jest.run();
console.log(`run success, ${result.numPassedTests} passed tests.`);

The new Jest instance has the following properties:

  • globalConfig: The global configuration to use for this run. You can use it to override arbitrary options (albeit by overriding the entire object reference).
  • projectConfigs: The project configurations to run. You can use it to override arbitrary project options (albeit by overriding the entire array reference).
  • run: Function that kicks of a test run.

Motivation

This change adds a documented programmatic API to run Jest, which makes it easier to build tools that use Jest or for users to run Jest from their own scripts.

Test plan

I have run it using yarn test. I've also added a unit test.

TODO

  • Document the programmatic API in /docs.
  • Add an e2e test that uses the programmatic API.
  • Test the API from my code.

@SimenB can you confirm that I'm on the right track here?

Closes #14034
Closes #5048

Add a documented programmatic API to run jest. It allows this use case:

```ts
import { run, readConfigs } from 'jest';
const { globalConfig, configs } = await readConfigs(process.argv, [process.cwd()]);
// change globalConfig or configs as you see fit
const results = await run(globalConfig, configs);
console.log(results);
```
@nicojs
Copy link
Contributor Author

nicojs commented Apr 9, 2023

The build is failing because packages/jest-core/build/cli/index.d.ts now has a reference to pnpapi. I have no clue why it would have that:

 /**
  * Copyright (c) Meta Platforms, Inc. and affiliates.
  *
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  */
 /// <reference types="node" />
 /// <reference types="node" />
 /// <reference types="node" />
 /// <reference types="node" />
 /// <reference types="node" />
+/// <reference types="pnpapi" />
 import type { AggregatedResult } from '@jest/test-result';

packages/jest-core/src/cli/index.ts Outdated Show resolved Hide resolved
packages/jest-core/src/cli/index.ts Outdated Show resolved Hide resolved
packages/jest-core/src/cli/index.ts Outdated Show resolved Hide resolved
@mrazauskas
Copy link
Contributor

For my use case I would need something like this:

import {run} from 'jest';

run(['path/to/some.test.ts'], {coverage:true});

Reading configs is useful, of course. But how to make it possible to override some option or selecting files almost like from CLI. runCLI() is not a solution, because I would like to be able to pass InitialOptions and it does not allow selecting files.

What if programmatic API would be shaped like this:

import {Jest} from 'jest';

const jest = new Jest();

jest.readConfig(); // same as `readConfigs()`

jest.setConfig({coverage: true});  // or as a shorthand new Jest({coverage: true})

jest.selectFiles(['path/to/some.test.ts']);
jest.selectProjects(['backend']);

const result = await jest.run();
// or await jest.watch();

@nicojs
Copy link
Contributor Author

nicojs commented Apr 10, 2023

Thanks for your review @mrazauskas! I've implemented your suggestions and created a separate runCore API. I've moved the entire run stuff to a new file, as it makes more sense now than a cli/index.ts file.

I've also decided to make runCore return a new object:

/**
 * The result of running runCore.
 */
export type RunCoreResult = {
  result: AggregatedResult;
};

That way, we can later introduce more return results without breaking the API.

Reading configs is useful, of course. But how to make it possible to override some option or selecting files almost like from CLI. runCLI() is not a solution, because I would like to be able to pass InitialOptions and it does not allow selecting files.

This is already possible. In fact, this is how Stryker currently works:

import {runCLI} from 'jest';
import {readInitialOptions} from 'jest-config';
const initialOptions = readInitialOptions();
initialOptions.testRegex = ['path/to/some.test.ts'];
const result = await runCLI(
      {
        $0: 'unused',
        _: [],
        config: JSON.stringify(initialOptions)
      },
      [process.cwd()]
    );

What if programmatic API would be shaped like this:

import {Jest} from 'jest';

const jest = new Jest();

jest.readConfig(); // same as `readConfigs()`

jest.setConfig({coverage: true});  // or as a shorthand new Jest({coverage: true})

jest.selectFiles(['path/to/some.test.ts']);
jest.selectProjects(['backend']);

const result = await jest.run();
// or await jest.watch();

This could be done, but this would be a deviation from the functional API that has been the norm so far. Note that your example can also be done with the new runCore functionality:

import {runCore, readConfigs} from 'jest';
const {globalConfig, configs} = await readConfigs(process.argv, [process.cwd()]);
globalConfig.collectCoverage = true;
globalConfig.testPathPattern = ['path/to/some.test.ts'];
const results = await runCore(globalConfig, configs);
const projectConfigs = configs.filter(({displayName}) => displayName = 'fooProject');
const {result} = await runCore(globalConfig, projectConfigs);

I've allowed for more return properties in the future, so we might add a jest instance or something in the result of runCore, which could allow for faster subsequent runs (this is actually what we would want in the end for Stryker, as we need to run thousands of times with slightly different global variables for switching between active mutants).

@mrazauskas
Copy link
Contributor

mrazauskas commented Apr 10, 2023

Sure, many things can be done with current API. That is not a question. Usability is missing. What I am looking for an API which is as easy as jest some.test.ts or jest --config={"coverage":true} --selectProjects second.

import {Jest} from 'jest';

const jest = new Jest();

const result = await jest.run(['some.test.ts']);
import {Jest} from 'jest';

const jest = new Jest({coverage: true});
jest.selectProjects(['second']);

const result = await jest.run();
import {Jest, readConfig} from 'jest';

const config = readConfig('.');
config.coverage = undefined;

const jest = new Jest(config);
const result = await jest.run();

All I ask is feedback, happy to implement it myself (; Only that this has to wait for major release.

@nicojs
Copy link
Contributor Author

nicojs commented Apr 11, 2023

All I ask is feedback, happy to implement it myself (; Only that this has to wait for major release.

I'm not sure. I personally don't mind either API. If you want to go the class syntax route, does this PR still make sense? Also: maybe the class should be called Jester? 😜

I need some help with the failing build. Does anyone know why pnpapi is suddenly referenced?

Error: /home/runner/work/jest/jest/packages/jest-core/build/runCore.d.ts has the following non-node type references:

/// <reference types="pnpapi" />

    at file:///home/runner/work/jest/jest/scripts/buildTs.mjs:225:17

@mrazauskas
Copy link
Contributor

mrazauskas commented Apr 12, 2023

The pnpapi issue looks familiar. See #12783


Class was just a rough idea. Actually a function like createJest() instead of the class could be better idea, because it can be async. That way it could take anything exported from Jest config file (just another way to use it):

import jestConfig from '../jest.config.js';

const jest = await createJest(jestConfig);

const result = await jest.run();

@nicojs nicojs marked this pull request as ready for review April 21, 2023 19:36
@nicojs
Copy link
Contributor Author

nicojs commented Apr 21, 2023

I've been able to fix the <reference types="pnpapi" /> error (great linting error, btw!). I've added some e2e tests and unit tests. I'm still busy with the docs, but you should take a look.

@mrazauskas I'm unopinionated about your API proposal. Maybe @SimenB can also pitch in here since he previously expressed an opinion about the programmatic API. I wouldn't want to do more work just to be shot down later...

@SimenB
Copy link
Member

SimenB commented Apr 22, 2023

/easycla

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Apr 22, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@SimenB
Copy link
Member

SimenB commented Apr 22, 2023

I like @mrazauskas's suggestion. It should be with a function and not a class so it can be async like mentioned. I'd expect the initial function to either take a path to a config file, inline config, or nothing (which would be Jest try to find the config manually).

@SimenB
Copy link
Member

SimenB commented Oct 4, 2023

@segrey @DmitryMakhnev Is this something that would be usable from IntelliJ (or Fleet)? Anything obvious missing (or wrong)?

@connectdotz same question - does this help the vscode extension?

Would love to hear your feedback 🙂

@segrey
Copy link

segrey commented Oct 4, 2023

@SimenB IntelliJ/Fleet both run Jest tests using Jest CLI. It appears that we're not currently interested in it.

@SimenB
Copy link
Member

SimenB commented Oct 4, 2023

Fair enough 🙂

If you wanna use the API instead of CLI and reporter at some point in the future, I'd love to figure out what's missing then 👍

@connectdotz
Copy link
Contributor

@SimenB I took a quick read on the api document; it seems these APIs are mainly to manage config, invoke the CLI/core logic, then get back the aggregated test results from the run. It could probably save us a few steps like managing child processes (for CLI), parsing the json result file, etc. But given these are already implemented and working, changing the CLI-based eco-system is not trivial, and I didn't see extra features or additional use cases enabled by the API for our extension... so not sure we will be using the API, at least for now...

I do have some general questions though:

  • The CLI output is pretty useful. Does the API still provide a way to output these in real-time (not at the end of the run)?
  • are the reporters still supported? what will a reporter looks like with the API approach? Reporter mainly provides a more real-time, incremental information, which the API looks like only supports batch mode?
  • I assume the API also supports coverage reporting?

@nicojs
Copy link
Contributor Author

nicojs commented Oct 14, 2023

@SimenB I think I'm done now 😀

@nicojs
Copy link
Contributor Author

nicojs commented Oct 16, 2023

I've updated the PR text to reflect the recent changes. I don't know why the build is failing.

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

sorry, forgot about this 😅 made some quick comments, will look through more throughly later

Comment on lines +20 to +24
jest.globalConfig = {
collectCoverage: false,
watch: false,
...jest.globalConfig,
};
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should expose something that can be mutated in this way. I'd rather have a mergeConfig or some such that is invoked with the current config and expects to get a new config back it assigns internally

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean something like this?

import {createJest, mergeConfig} from 'jest';
const jest = await createJest();

// Override global options
jest.globalConfig = mergeConfig(jest.globalConfig, {
  collectCoverage: false,
});

Or more like this?

import {createJest} from 'jest';
const jest = await createJest();

// Override global options
jest.mergeConfig(jest.globalConfig, {
  collectCoverage: false,
});

Copy link
Member

Choose a reason for hiding this comment

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

import {createJest} from 'jest';
const jest = await createJest();

// Override global options
jest.mergeGlobalConfig({
  collectCoverage: false,
});

or

jest.globalConfig.merge({
  collectCoverage: false,
});

The latter would mean globalConfig was some sort of class or something instead of just the object

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How would you override a project-specific config?

I think allowing you to mutate config in-place makes more sense, actually. So removing the read-only feature. I don't think freezing the config serves a real purpose. 🤷‍♀️

Copy link
Member

Choose a reason for hiding this comment

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

I generally prefer encapsulation - makes it easier to debug when all mutations come though known methods and you can set a breakpoint in it.


How would you override a project-specific config?

jest.mergeProjectConfig(index, {
  testEnvironment: 'node',
});

This actually brings an important point to mind - any config modifications must go through its part in normalize within jest-config, to e.g. resolve testEnvironment to an absolute path.

I guess we can punt on that for now though. But if we do kick it down the line for later, I want an

jest.unstable_modifyConfig() or some such so we can add proper config normalization later without a breaking change.

@@ -5,18 +5,20 @@
* LICENSE file in the root directory of this source tree.
*/

const {runCLI} = require('@jest/core');
const {createJest} = require('jest');
Copy link
Member

Choose a reason for hiding this comment

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

@nicojs missed?

Comment on lines 6 to 7
"testEnvironment": "node",
"maxWorkers": 1
Copy link
Member

Choose a reason for hiding this comment

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

why is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not. Will remove

return {contexts, hasteMapInstances};
};

export const _run = async (
Copy link
Member

Choose a reason for hiding this comment

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

why do we export with _ prefix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Fixed by removing the _

@nicojs
Copy link
Contributor Author

nicojs commented Oct 31, 2023

Thanks @SimenB, welcome back 🎃

I think fixed all your remarks except for one where I have a question.

@nicojs
Copy link
Contributor Author

nicojs commented Nov 7, 2023

@SimenB, what would you think about just making the config objects mutable? I personally don't see a reason to make it more complicated than it needs to be. What are we actually afraid of? It was always 'shallow immutable' anyway. 🤷‍♂️

@SimenB
Copy link
Member

SimenB commented Dec 25, 2023

Main issue isn't mutability, but the fact you can provide invalid configuration values. E.g. jsdom is not valid configuration at this point - it has to be an absolute path to a module. Our normalize function needs to do its work on any input from the user, thus it needs to go via a setter of some kind.

@mbaumanndev
Copy link

@SimenB @nicojs I work on a codebase using Stryker for my tests where this PR would be a huge help to simplify the codebase. Is there anything I can do to help on this PR ? I've not contributed to many projects but I'm willing to invest some time to help here as it would help me a lot in my daily work

@SimenB
Copy link
Member

SimenB commented Mar 16, 2024

Hey @mbaumanndev! I think we're only missing normalization of configuration when setting it. If you could contribute that, I think we're ready to land this 🙂

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