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

Ship Jest Circus #6295

Closed
6 tasks done
captbaritone opened this issue May 26, 2018 · 39 comments · Fixed by #10686
Closed
6 tasks done

Ship Jest Circus #6295

captbaritone opened this issue May 26, 2018 · 39 comments · Fixed by #10686
Milestone

Comments

@captbaritone
Copy link
Contributor

captbaritone commented May 26, 2018

Here is my memory of the tasks that lay ahead of us in order to roll out Jest Circus. Feel free to add anything that I'm missing.

  • Get all of Jest's own tests passing with JEST_CIRCUS=1 jest-circus feature parity with jest-jasmine #4362
  • Run Jest with JEST_CURCUS=1 on a few large code bases. Resolve all issues and add regression tests.
  • Add some sort of strict/warn mode where users can opt into warnings/errors if they are using Jasmine APIs not supported buy Circus. (@captbaritone) Add a "errorOnDeprecated" mode that errors when calling deprecated APIs #6339
  • Expose jest-jasmine in such a way that it can be used as a custom runner for those that can't yet move to jest-circus. (that's free - testRunner: 'jest-jasmine2)
  • Make jest-circus the default runner, with guidance in a blog post/changelog that people can use the jest-jasmine custom runner if they need to.
  • Remove the JEST_CIRCUS environment variable and the SkipOnJestCircus helper class.
@aaronabramov
Copy link
Contributor

i volunteer to update jest in www :)

@SimenB
Copy link
Member

SimenB commented May 28, 2018

If anybody wanna test on React, I migrated to jest 23 here: facebook/react#12894. It still uses jasmine spies for spyOnDev spyOnProd stuff, though. Maybe codemod it?

@aaronabramov
Copy link
Contributor

i'll play with codemods in www today. it won't be perfect, but i hope ti'll get us at least 70% there

@nickpresta
Copy link
Contributor

We have a large (just over 9000 tests), and a few moderately sized (a few hundred tests) codebases internally at Wave and we would love to help test some of the Jest Circus functionality to provide feedback, etc.

Are there docs for testing Jest Circus, or is it as simple as installing jest@next and setting the environment variable?

@SimenB
Copy link
Member

SimenB commented Jul 3, 2018

Awesome!

yarn add -D jest-circus (or npm i -D jest-circus) and set testRunner in config to jest-circus should be enough. (If not, please tell us 😅)

Jest@23 has it all, no need for a beta 🙂

@nickpresta
Copy link
Contributor

@SimenB awesome, thanks. Just to help out any future people who want to give this a shot, I had to:

  1. Install jest-circus with yarn add -D jest-circus
  2. Do one of the following:
    • Set testRunner in my Jest configuration (specified in package.json in my case) to jest-circus/runner
    • Don't change anything in my Jest configuration but set JEST_CIRCUS=1 on the command line when I run my tests (e.g. JEST_CIRCUS=1 yarn test).

It seems that specifying jest-circus in testRunner isn't enough, as jest-circus exports an object and the main entry points to index.js, instead of runner.js. I'm not sure if this is a bug, but the default runner exports a TestRunner instance, not an object.

No other files import jest-circus alone (relying on index.js being the entrypoint) so it seems safe to update the main field to runner.js, but maybe this is only a temporary problem until circus goes live. If you think it's a bug, I wouldn't mind submitting a patch to update the main entrypoint of jest-circus. Let me know.

@SimenB
Copy link
Member

SimenB commented Jul 3, 2018

@aaronabramov ^ seems like a bug, no?

@aaronabramov
Copy link
Contributor

that was intentional! so we could import {test} from 'jest' in the future
the runner in the config should point to https://github.com/facebook/jest/blob/87114d3d8ce87c2113e2073e7fc6148af87618be/packages/jest-circus/runner.js
which just proxies it to ./build/legacy_code_todo_rewrite/jest_adapter

@nickpresta let me know if it works for you! i also wrote a bunch of codemods in jest-codemods repo to help migrate old jasmine tests to jest-circus (if you still have jasmine references)

@dylang
Copy link
Contributor

dylang commented Jul 16, 2018

Running 4715 tests with JEST_CIRCUS=1 and [email protected]:

💯 After two minor changes, all test pass! 💯

Important notes:

  • Manipulating Date.now breaks the duration value sent to custom reporters.
    • In one test it went from 10ms to 48y 208d 2h 39m 44.2s 😨
    • In the file: beforeEach(() => { Date.now = jest.fn(() => now); });
  • When I had one expect(object).toEqual(...) fail, 9 other tests timed out.
    • This happened in CI, with no Jest cache from previous runs.
    • The same tests timed out each time.
    • Each timed out test is in a separate file.
    • When the above expect was fixed, the other tests stopped timing out.
    • We have seen similar behavior before using JEST_CIRCUS=1, but at a smaller scale (one timed out test might lead to one other unrelated test timing out). I haven't been able to trace this well enough to file a useful ticket.
  • I'm still getting occasional seemingly random timeouts in tests that normally take 10ms.
    • I've increased the time to 5 seconds and it didn't help.
    • It's not happening often enough that I can debug it or create a repo case.

I can't share the code I'm testing, but I'm happy to keep trying things.

@aaronabramov
Copy link
Contributor

@dylang this is awesome!!
the Date thing totally makes sense, we probably forgot to reuse the original Date instead of the current one (which can be mocked)

the timeouts are strange. I've seen it before as well, my guess that it's coming from a global unhandled error somewhere that break other tests in the same test worker (unfortunately we don't have perfect isolation)

@evocateur
Copy link

I just enabled jest-circus in Lerna. Only had to replace a couple references to jasmine.DEFAULT_TIMEOUT_INTERVAL with jest.setTimeout(). 💯

@SimenB
Copy link
Member

SimenB commented Jul 18, 2018

That's so awesome!

@rattrayalex-stripe
Copy link

I saw "jest circus" referenced in #2713 (comment) - should it have a readme? have to do some digging to discover that it's a "jest-jasmine" thing.

@rickhanlonii
Copy link
Member

A readme for jest-circus is a great idea

@rickhanlonii
Copy link
Member

@rattrayalex-stripe README added here: #7198

@rickhanlonii
Copy link
Member

@captbaritone have we done this?

Expose jest-jasmine in such a way that it can be used as a custom runner for those that can't yet move to jest-circus

I installed jest-jasmine2 and set the testRunner=jest-jasmine2 and tests run

@SimenB
Copy link
Member

SimenB commented Feb 24, 2019

@glasser
Copy link

glasser commented Nov 4, 2019

Is this something that's still in progress? It's been a while since the last update, but it's the only recommended solution for #2713

@SimenB
Copy link
Member

SimenB commented Nov 4, 2019

It's pretty much shipped - FB uses it for everything. We just need to flip the default

@glasser
Copy link

glasser commented Nov 4, 2019

That's great to hear! What's the currently recommended way to use it? I've seen references to a config file and to an env var.

@ChrisCrewdson
Copy link

The current way we use it is to set the testRunner in jest.config.js like this:

testRunner: 'jest-circus/runner',

@segrey
Copy link

segrey commented Jan 25, 2020

@G-Rath Thanks for the heads up! Yep, IntelliJ Jest integration uses a Jasmine reporter to get access to more fine-grained events about tests execution. For example, this allows to have "Click to see difference" link that shows a diff dialog for expected/actual values for failed assertions.
Since the Jasmine reporter consumes Jasmine Reporter API, it can be attached to jest-jasmine2 only.

I hope that Jest reporters API (--reporters) will be improved one day (#8840) to eliminate the need in IntelliJ Jasmine reporter. If this is not going to happen before jest-circus becomes the default runner, then probably IntelliJ should support jest-circus in the same way as it supports jest-jasmine2 unless there are other options?

My understanding is that jest-circus is completely backwards compatible, so in this case all that should be required is changing that string - while jest-circus might have new API endpoints that IntelliJ/WebStorm could use, treating jest-circus as jest-jasmine2 should work painlessly for the common cases - Please let me know if that's not the case.

Unfortunately, that doesn't seem to be the case according to https://github.com/facebook/jest/blob/master/packages/jest-circus and https://github.com/facebook/jest/blob/master/packages/jest-types/src/Circus.ts - jest-circus defines its own API, not compatible with Jasmine.
Also, I couldn't find a way to obtain expected/actual values for failed assertions in jest-circus API. Am I missing something? Any help would be much appreciated. @G-Rath @SimenB

@G-Rath
Copy link
Contributor

G-Rath commented Jan 25, 2020

@segrey thanks for the quick & detailed response - I'm keen to get the gap between jest-circus & IntelliJ closed as fast as possible, so more than happy to help out wherever I can.

I'll post a kick-off comment on #8840 to try and scope out how much work it requires, and hopefully with a bit of guidance from @SimenB (or similar peoples that know the reporters side of jest in decent detail) I should be able to tackle at least some of it.

My understanding is that jest-circus is completely backwards compatible,

I can't find where I read that, and it was at least half a year ago, so I'm thinking I probably misunderstood the original posting.

Just to make sure I'm understanding:

Also, I couldn't find a way to obtain expected/actual values for failed assertions in jest-circus API. Am I missing something?

Is that the only primary thing that's missing for you right now, or is there more? (don't expect a full list, but just trying to scope out how big the gap actually is right now).

It'll probably be worth tracking this via a separate issue, depending on what @SimenB thinks. In the meantime I'll have a dig around jest-circus internals to see if I can load some more context into my head :)

@segrey
Copy link

segrey commented Jan 25, 2020

Is that the only primary thing that's missing for you right now, or is there more?

Also, I stumbled upon adding a custom Circus.EventHandler via addEventHandler: as I can see there is addEventHandler(environment.handleTestEvent.bind(environment)) in jest-circus/src/legacy-code-todo-rewrite/jestAdapterInit.ts, but IntelliJ cannot overwrite existing Jest environment.
Is there a way to add a custom Circus.EventHandler from a script registered via setupFilesAfterEnv?

@jeysal
Copy link
Contributor

jeysal commented Jan 25, 2020

No official way (without private imports from jest-circus or the likes) afaik. I've been critical of the inflexible test environment abstraction before :/

@G-Rath
Copy link
Contributor

G-Rath commented Jan 25, 2020

@jeysal cheers for weighing in - I've made #9468 to track this, to avoid overwhelming this thread since it's not technically been classed as blocking for the shipping of jest circus.

I'll ask my follow up question over there :)

@ron23
Copy link

ron23 commented Mar 27, 2020

Wanted to share that running with jest-circus has made my memory leak twice as bad.

@SimenB
Copy link
Member

SimenB commented May 1, 2020

Memory leak in circus was fixed yesterday and released in 25.5.3. I'm thinking we're finally ready to switch defaults on next major

@jeysal
Copy link
Contributor

jeysal commented May 1, 2020

@SimenB There's #8096, which I would consider a pretty important polish. But we either need to strip out the hooks part, making it only half polished, or get the FB codebase to not do weird hook scheduling things

@jeysal
Copy link
Contributor

jeysal commented May 1, 2020

(And in general, for majors to come, we should always go through the milestone and explicitly reschedule things to a later major before commiting to the release (which we've kinda neglected for 25) to make sure there's nothing essential from the milestone missing

@cpojer cpojer modified the milestones: Jest 26, High priority future May 2, 2020
@jeysal jeysal modified the milestones: High priority future, Jest 27 May 2, 2020
@Mouvedia
Copy link

Mouvedia commented May 3, 2020

I would love to have #8604 before the announcement.

@SimenB
Copy link
Member

SimenB commented May 3, 2020

We'll ship it as default in 27 instead of 26, but say we're going to do so in the blog post so people start moving.

Not sure if it warrants migration guide or not - as long as you use only the publicly documented API (aka, no Jasmine stuff) then it should be a drop-in replacement (modulo #8360 which we should fix)

@Cosmin-Gramada
Copy link

@SimenB
Roughly, when is Jest 27 expected to be launched?

@SimenB
Copy link
Member

SimenB commented May 11, 2020

No timeline currently, but I'd guess a few months out. You can use it today though, you don't have to wait for this issue. Instructions for how to set it up are in its readme: https://github.com/facebook/jest/blob/master/packages/jest-circus/README.md

Any feedback would be appreciated so we can make it as solid as possible before releasing it to everybody 🙂

@jeysal
Copy link
Contributor

jeysal commented May 11, 2020

Jest 20 was released almost exactly 3 years ago (2017-05-06), Jest 26 very recently (2020-05-04). So in the last three years there has been an average of one major release every 6 months. Based on that (not on any knowledge I may have as a contributor) I would expect the Jest 27 release to be around November 2020.

@SimenB
Copy link
Member

SimenB commented May 11, 2020

I'd like to release more often than every 6 months if we have breaking changes that we think will improve user experience or make maintenance of Jest itself easier, such as the changes we made in Jest 26 and have scheduled for Jest 27. IMO it went too long between Jest 24 and 25 (4 days off a full year). But upgrading majors is not free for people, so we shouldn't drop a new major every other month either.

@SimenB
Copy link
Member

SimenB commented Nov 26, 2020

🎉

2.5 years to the day 😛

@github-actions
Copy link

This issue 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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.