jest
jest copied to clipboard
fix: jest-circus shares events among imports #11483
closes #11483
Summary
The issue is described here: https://github.com/facebook/jest/issues/11483
A public interface to subscribe to events from jest-circus.
Test plan
No changes in UI.
Hi @satanTime!
Thank you for your pull request and welcome to our community.
Action Required
In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.
Process
In order for us to review and merge your suggested changes, please sign 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 to sign the corporate CLA.
Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.
If you have received this in error or have any questions, please contact us at [email protected]. Thanks!
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!
Codecov Report
Merging #11529 (fcde463) into main (faef0b4) will decrease coverage by
0.06%. The diff coverage is100.00%.
@@ Coverage Diff @@
## main #11529 +/- ##
==========================================
- Coverage 68.47% 68.41% -0.07%
==========================================
Files 324 324
Lines 16967 16972 +5
Branches 5060 5062 +2
==========================================
- Hits 11618 11611 -7
- Misses 5317 5329 +12
Partials 32 32
| Impacted Files | Coverage Δ | |
|---|---|---|
| packages/jest-circus/src/index.ts | 70.66% <ø> (ø) |
|
| packages/jest-circus/src/state.ts | 95.65% <100.00%> (+16.70%) |
:arrow_up: |
| packages/jest-circus/src/types.ts | 100.00% <100.00%> (ø) |
|
| packages/jest-circus/src/eventHandler.ts | 0.75% <0.00%> (-9.10%) |
:arrow_down: |
| packages/jest-circus/src/formatNodeAssertErrors.ts | 9.21% <0.00%> (-2.64%) |
:arrow_down: |
| packages/jest-circus/src/utils.ts | 11.32% <0.00%> (-0.48%) |
:arrow_down: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update faef0b4...fcde463. Read the comment docs.
HI @SimenB,
the PR has been rebased.
HI @SimenB,
the PR has been rebased.
Hi @SimenB,
the PR has been rebased.
Hi @SimenB,
the PR has been rebased.
Hi @SimenB, might you explain what the problem with this PR is? Its merge would bring huge relief in life-cycle management.
Hi @satanTime! I'd just like to say thank you for sticking by this PR without any feedback from me. Unfortunately, I've had little to no combination of time, effort, energy or plain want to work on OSS for months, so this (and other stuff) has just been hanging. The entire 💩storm that is ESM in Node (and specifically its vm API which upstream V8 seems to not care about enabling) completely drained any and all joy I got out of working on OSS in general and Jest specifically.
Hopefully I'll come back to it sooner rather than later, but PRs such as this (where the code itself isn't the hard part, but rather if the underlying issue is something we should support and secondly if the solution here is correct) is not something I'm currently in the correct place to consider
@SimenB I'm curious, are you the only maintainer of jest now? Is facebook not supporting this project internally anymore?
Nobody at FB (Meta?) has worked on Jest for years at this point, beyond a few PRs here and there like any other open source contributor
Hi @SimenB, glad to see you alive :) And happy new year!
Open source is huge pain in the hole :) I hope nobody will give up, only the brave!
I've rebased the PR. Please let me know if there is anything I need to change / explain etc :)
Regarding the issue, I think it's context hell.
Different dependencies require jest-circus in different places and each require creates a different context, that's why require in global scope has its own context and doesn't allow to properly manage EVENT_HANDLERS.
Nobody at FB (Meta?) has worked on Jest for years at this point, beyond a few PRs here and there like any other open source contributor
I honestly though you were working for Facebook in development of Jest, what a shame :-(
I commented it on Twitter: https://twitter.com/mafalda_sfu/status/1488600128680341517
We need to make noise, @meta is still publicing Jest as from them but they are not maintaining anymore, while in fact they should have contracted @SimenB to maintaining it, that's the fair thing to do on this topic.
(To avoid potential drama I definetely do not have the energy for, I'll make this preemptive post).
Just to make sure everybody is on the same page - while FB has never paid me (or, as far as I know, any other non-employee) to work on Jest, that doesn't mean there hasn't been opportunities to be paid for the work I and others have put into the project. Specifically the Open Collective has been available, and I've been told multiple times to file expenses against it. However, personally, I've preferred to keep the time I've spent on this project without compensation to avoid feeling compelled to work on it. It might be idealistic, but by not getting paid for the work I've purposefully kept the project from something I've felt obligated to work on. I think by taking the money I'd feel obligated (rightly!) to dedicate time to the project. Which I've never felt comfortable doing. This is a personal choice to keep my independence (and freedom to not interact with the project as much as I probably should the last months), but does not reflect on FB's/Meta's motivations or priorities. My personal choices doesn't mean FB has expected me to work for free, just that I've chosen not to take salary/compensation.
That said I want to be clear there's no "bad guy" here - nobody is forcing me to (or trying to) work on things, and my off hands approach is solely down do lack of time and energy, and not any deeper motivation. Jest has been an open source project for years and continues to be so. Anybody missing any features should continue to feel free to contribute to make those features happen.
(For transparency, FB paid for my and other OSS maintainers trip to London to work physically together on the project back in 2017 and 2018. I can provide more details if people want)
Hi all, could we keep this thread related to the PR with no drama and a pure focus on solving the issue?
@SimenB Is Meta using a different testing framework internally, or Jest is just good enough as-is for them?
Hi @SimenB, the PR has been rebased. What stops you from including in it the next major release?
The issue is context:
currently it is like that:
const a = () => {
const context = {};
return context;
};
a() === a(); // false
when it should be
global.context = global.context || {};
const a = () => {
return global.context;
};
a() === a(); // true and everybody is happy
Hi @satanTime,
Seeing the time needed for this to be merged, maybe you could publish a gist to use along with https://www.npmjs.com/package/patch-package, installed via a postinstall script ? Just wondering if this is doable.... Of course, hotpatching jest isn't a viable long term solution, but it might at least be a POC users will use, and eventually give confidence to maintainers to (finally) merge this PR.
I'm having the feeling that this package, used by a lot of people, would gain by redefining contributors permissions & include more people, so that you/we could have more people to talk to to bring evolutions in.
Cheers
Hi @GerkinDev, it's a good idea, thanks for the hint. I just need to find time to implement it.
I wanted to give an update, in order to address the maintenance concerns mentioned in this thread, Jest has been transferred to a foundation. Hopefully this will help! https://engineering.fb.com/2022/05/11/open-source/jest-openjs-foundation/
Hi @vjeux,
not sure if that's changed anything, in the article it says "Jest joining the OpenJS foundation does not change how Jest is developed or released".
@SimenB, is there a way to get someone from your team to take a look at this PR?
Without this change, and knowing about the issue in hooks https://github.com/facebook/jest/issues/12678, jest becomes a framework, which is very hard to support in 3rd-party libraries.
The committers listed above are authorized under a signed CLA.
- :white_check_mark: login: satanTime (3b5066071a3d02277ebe8c3db529e6770f40da87, d9cda507c07db32a0a38dc112013ac1042058e32)
Deploy Preview for jestjs ready!
Built without sensitive environment variables
| Name | Link |
|---|---|
| Latest commit | 3b5066071a3d02277ebe8c3db529e6770f40da87 |
| Latest deploy log | https://app.netlify.com/sites/jestjs/deploys/665b3a94d77171000867f2e3 |
| Deploy Preview | https://deploy-preview-11529--jestjs.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
Hi @SimenB, happy Wednesday!
Is there an opportunity to get your attention on this issue again? The fix is here for more than 2 years.