[WIP] Adds state and run exports to index

[kvendrik]

Summary

Currently when you want to write a minimal jest-circus implementation you have to reach into the build/ folder to be able to get all the tools you need. @SimenB and I discussed having these essential tools available through from the top level.

Test plan

I don't see any tests for the exports and figure that if this messes up other exports existing tests will fail. If you'd like me to add tests to this somehow let me know, I'd be happy to do so.

[kvendrik]

cc @SimenB

[facebook-github-bot]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

[SimenB]

@scotthovestadt @aaronabramov does this match the abstraction we're going for?

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[scotthovestadt]

@kvendrik Can you provide a little more information about the use-case?

I don't think we want to expose those to the average consumer of the package (the API might change), but I agree internal exports could be improved within Jest.

[kvendrik]

@scotthovestadt the use-case is one where jest-circus is used to create a minimal Jest test runner from scratch. When creating a test runner from scratch these methods are essential but it all depends on what the team's goal is for the package. If the package is strictly supposed be compatible with the Jest's testRunner option I can see why you might not want these to be top level exports as they promote a use-case you might not want to actively support.

[SimenB]

It was, at least initially, meant to be a relatively generic test runner. It was designed to eventually work in a browser setting

[kvendrik]

Gotcha if that's still the case I feel like having addEventListener and run as top level exports probably makes sense(?) considering you need them to be able to properly set up an in-browser test runner.

[codecov-io]

Codecov Report

Merging #8748 into master will increase coverage by 0.04%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #8748      +/-   ##
==========================================
+ Coverage   64.12%   64.16%   +0.04%     
==========================================
  Files         275      275              
  Lines       11629    11629              
  Branches     2846     2846              
==========================================
+ Hits         7457     7462       +5     
+ Misses       3545     3540       -5     
  Partials      627      627

Impacted Files	Coverage Δ
packages/jest-circus/src/index.ts	68.05% <ø> (ø)	⬆️
packages/jest-circus/src/run.ts	7.69% <0%> (+7.69%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e39b8dc...a5eb00d. Read the comment docs.

[SimenB]

Right now, the exports here are the globals jest injects, and nothing about the runner itself. This is by design, as the thought is that (eventually) you can do import {beforeEach, test} from 'jest'. Adding state and run to that doesn't seem right.

We have a runner file, which is meant to be given to testRunner config option: https://github.com/facebook/jest/blob/4482e71bd5c6c70b7f17450299dbee1394bfaa09/packages/jest-circus/runner.js. I wonder if these new exports should be added to that instead? We should also add a runner.d.ts

[kvendrik]

@SimenB that makes a lot of sense. So import would be:

import {run, addEventHandler} from 'jest-circus/runner';

Is that correct? I feel like that might make sense if we don't want it to be top level but still easily accessible. So the definition would be:

Top-level Jest exports are all current Jest globals. Utilities like run and addEventListener are essential for a bare-bones test runner setup but as they aren't globals they're not available from the top-level but instead made available through jest-circus/runner.

[SimenB]

Yeah, exactly

[kvendrik]

@SimenB considering runner.js is a JS file and run and state are TypeScript files what do you think would be the best way to tackle this? I'm thinking runner should maybe be moved to src/runner.ts and export all three (./build/legacy-code-todo-rewrite/jestAdapter as the default, * from state and run).

[SimenB]

Works for me. We need to create a runner.d.ts in the root anyways so TS types are still correct. Or can we fake it with jsdoc comments?

[kvendrik]

@SimenB hows something like this https://github.com/facebook/jest/pull/8748/files

Only thing now is that those files aren't always available which breaks the type check. Did you have any thoughts on how you wanted to work around that?

[SimenB]

Did you have any thoughts on how you wanted to work around that?

Ignore the file via the tsconfig in jest-circus

[kvendrik]

Updated it to somewhat where we're hoping to get to. This will need some further looking into and testing so I'll add a [WIP] prefix to the title for now. P.s. if anyone needs this sooner rather than later, feel free to pick it up.

[SimenB]

This is probably unblocked by us adding import {expect, test} from '@jest/globals' (#9801). Wanna rebase? 😀

[SimenB]

@kvendrik would you be able to refresh this? I think it's something we still want to do

[github-actions]

This PR is stale because it has been open 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.