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.

Sign up for GitHub

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

Jump to bottom

fix: jest-circus shares events among imports #11483 #11529

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

fix: jest-circus shares events among imports #11483 #11529

wants to merge 3 commits into from
+52 −10

Conversation

satanTime
Copy link

@satanTime satanTime commented Jun 5, 2021 •
edited
Loading

closes #11483

Summary

The issue is described here: #11483

A public interface to subscribe to events from jest-circus.

Test plan

No changes in UI.

@facebook-github-bot
Copy link
Contributor

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!

@facebook-github-bot
Copy link
Contributor

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

@codecov-commenter
Copy link

codecov-commenter commented Jun 5, 2021 •
edited
Loading

Codecov Report

Merging #11529 (fcde463) into main (faef0b4) will decrease coverage by 0.06%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            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%) ⬆️
packages/jest-circus/src/types.ts 100.00% <100.00%> (ø)
packages/jest-circus/src/eventHandler.ts 0.75% <0.00%> (-9.10%) ⬇️
packages/jest-circus/src/formatNodeAssertErrors.ts 9.21% <0.00%> (-2.64%) ⬇️
packages/jest-circus/src/utils.ts 11.32% <0.00%> (-0.48%) ⬇️

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 faef0b4...fcde463. Read the comment docs.

@satanTime satanTime force-pushed the feature/11483 branch 3 times, most recently from 8315535 to 7c70335 Compare June 5, 2021 09:01
@satanTime satanTime mentioned this pull request Jul 1, 2021
Open
fredericojesus
fredericojesus approved these changes Jul 29, 2021
View reviewed changes
Copy link

@fredericojesus fredericojesus left a comment

Choose a reason for hiding this comment

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

Tks for this PR. Looking forward for seeing it released. I don't see any issue with the code, like the approach.

szakharchenko
szakharchenko approved these changes Aug 2, 2021
View reviewed changes
Copy link

@szakharchenko szakharchenko left a comment

Choose a reason for hiding this comment

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

Exposing testing events in the API makes so much sense that I basically assumed it was there when I started using Jest, only to find out that it wasn't. The proposed changes seem in line with what's already in there. One may argue that this is a feature, not a fix, and adjust the CHANGELOG.md entry, but other than that, things look fine.

@satanTime
Copy link
Author

HI @SimenB,

the PR has been rebased.

@satanTime satanTime force-pushed the feature/11483 branch 2 times, most recently from a37105c to ee436cb Compare September 11, 2021 16:18
@satanTime
Copy link
Author

HI @SimenB,

the PR has been rebased.

@satanTime
Copy link
Author

Hi @SimenB,

the PR has been rebased.

@satanTime satanTime force-pushed the feature/11483 branch 2 times, most recently from fad07ab to 3f03ebf Compare September 20, 2021 15:52
@satanTime
Copy link
Author

Hi @SimenB,

the PR has been rebased.

@satanTime satanTime force-pushed the feature/11483 branch 2 times, most recently from 902c7c2 to 1206b5b Compare October 31, 2021 12:09
@dbjorge dbjorge mentioned this pull request Apr 27, 2022
Closed
@vjeux
Copy link
Contributor

vjeux commented May 11, 2022

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/

@szakharchenko szakharchenko mentioned this pull request Sep 20, 2022
Open
@satanTime
Copy link
Author

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 #12678, jest becomes a framework, which is very hard to support in 3rd-party libraries.

@satanTime satanTime force-pushed the feature/11483 branch 2 times, most recently from 5508f05 to 77fd95e Compare April 16, 2023 15:43
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Apr 30, 2023 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

@netlify
Copy link

netlify bot commented Apr 30, 2023 •
edited
Loading

✅ Deploy Preview for jestjs ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 68b9307
🔍 Latest deploy log https://app.netlify.com/sites/jestjs/deploys/66ccd61cfed8d20008a52636
😎 Deploy Preview https://deploy-preview-11529--jestjs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@satanTime satanTime force-pushed the feature/11483 branch 10 times, most recently from 4a02fd9 to 810144d Compare November 7, 2023 15:15
@ChristophP ChristophP mentioned this pull request Nov 8, 2023
Open
@satanTime
Copy link
Author

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.

@flevi29 flevi29 mentioned this pull request Jan 16, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fredericojesus fredericojesus fredericojesus approved these changes

@szakharchenko szakharchenko szakharchenko approved these changes

@krusche krusche krusche approved these changes

Assignees
No one assigned
Labels
cla signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

the same context for addEventHandler from jest-circus/src/state.ts