🪝 Support for beforeEach/afterEach in @fast-check/jest - #6586
Conversation
🦋 Changeset detectedLatest commit: e92d09a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| } | ||
|
|
||
| // Between runs: close previous iteration, then open next | ||
| for (const hook of afterEachHooks) { |
There was a problem hiding this comment.
Why not hooking yourself in afterEach?
There was a problem hiding this comment.
By doing so these after don't run with others... but actually I get why you did it
There was a problem hiding this comment.
What do you mean? Call propertyInstance.afterEach after jest afterEach (and vitest afterEach)?
There was a problem hiding this comment.
but actually I get why you did it
So, what should we do in this place? What do you think? 🙂
| propertyInstance.beforeEach(async (previousHook: () => Promise<void>) => { | ||
| await previousHook(); | ||
|
|
||
| if (isFirstRun) { |
There was a problem hiding this comment.
The same logic goes for last run? Unfortunately we won't be able to know if it's the last or not
There was a problem hiding this comment.
Yes, here's the lifecycle of fc and jest hook calls:
Current pass for N runs:
jeft.beforeEach ← covers the 1st beforeEach
fc.beforeEach: skip ← 1st run, do nothing
run(v1)
fc.afterEach: (noop)
fc.beforeEach: afterEach + beforeEach ← closed the 1st run, opened the 2nd
run(v2)
fc.afterEach: (noop)
fc.beforeEach: afterEach + beforeEach ← closed the 2nd run, opened the 3rd
run(v3)
fc.afterEach: (noop)
jest.afterEach ← covers the last afterEach
Total: N beforeEach + N afterEach — the order is correct.
If a different approach were used, with a naive call to jest.beforeEach and jest.afterEach in fc.beforeEach and fc.afterEach, respectively, the final run would call afterEach twice:
Pass for N=3:
jest.beforeEach ← bE #1 ✅
fc.beforeEach: skip
run(v1)
fc.afterEach: jest.aE ← aE #1 ✅
fc.beforeEach: jest.bE ← bE #2 ✅
run(v2)
fc.afterEach: jest.aE ← aE #2 ✅
fc.beforeEach: jest.bE ← bE #3 ✅
run(v3)
fc.afterEach: jest.aE ← aE #3 ✅
jest.afterEach ← aE #4 ❌
Result: N beforeEach (ok), but N+1 afterEach (error) — jest.afterEach can't be suppressed, and it adds an extra call.
Maybe this also partially answers your question you asked here: #6586 (comment)
| // - jest calls its own afterEach after the test (covers last run) | ||
| // | ||
| // Result: exactly N beforeEach + N afterEach for N property runs. | ||
| const jestState = getJestCircusState(); |
There was a problem hiding this comment.
Only working with Jest circus in CJS mode, ESM execution of Jest might not work
There was a problem hiding this comment.
Why do you think it won't work in ESM? jest-circus is included by default in Jest 27+ and runs independently of the CJS/ESM environment. globalThis and Symbol('JEST_STATE_SYMBOL') will also be available
| // Result: exactly N beforeEach + N afterEach for N property runs. | ||
| const test = getCurrentTest(); | ||
| const suite = test?.suite || test?.file; | ||
| if (suite) { |
There was a problem hiding this comment.
What would it mean to have no suite?
We could probably drop one level of if nesting by doing:
const beforeEachHooks = suite!==undefined?collectBeforeEachHooks(suite):[];
// same for after
There was a problem hiding this comment.
test is undefined if called outside a test callback (for example, in a worker thread where the _test vitest state is not set).
suite is undefined if the test has neither a parent test suite nor a file reference (a top-level test or a detached test).
I'll add a comment to the code that describes this behavior.
There was a problem hiding this comment.
Also, we can't do suite !== undefined ? collectBeforeEachHooks(suite) : [], because both test and suite are required to call the hooks. We must ensure that test and suite are defined before running the hooks to ensure type safety.
…erty run" This reverts commit e814193.
beforeEach/afterEach in @fast-check/jest and @fast-check/vitestbeforeEach/afterEach in @fast-check/jest
|
@rushelex now that we merged vitest one, we can come back to this one. I believe we should stick with the same philosophy as the pr we merged for vitest. So probably worth checking and updating code and test in the same spirit |
|
@dubzzz Hi. Unfortunately, I don't have much time to work on this PR right now. I'd appreciate it if you could complete it, or we can wait until I have more time. |
Description
This PR closes issue #3942.
Currently, when using
it.prop/test.propin@fast-check/jest,beforeEachandafterEachhooks are only called once per test — not once per property run. This means that if a property executes 100 runs inside a singleit.prop, hooks likebeforeEach(commonly used for setup/teardown, mocking, DB transactions, etc.) only fire once instead of 100 times.This PR makes
beforeEach/afterEachfire for each property run, not just once per test. For example, with 1 regularit()+ 1 regularit()+ 1it.prop()withnumRuns: 10,beforeEach/afterEachwill now fire 12 times total instead of 3.Strategy:
beforeEachbefore the test (covers 1st run)afterEach→beforeEachviapropertyInstance.beforeEach()hookafterEachafter the test (covers last run)beforeEach+ NafterEachfor N property runs@fast-check/jest: Accesses jest-circus internal state viaObject.getOwnPropertySymbols(globalThis)matchingSymbol('JEST_STATE_SYMBOL')string representation. Only works with jest-circus runner (default since Jest 27); graceful degradation for jest-jasmine2 (hooks fire once as before). Does not work in worker mode (property runs in a separate process without access to jest-circus state).Checklist — Don't delete this checklist and make sure you do the following before opening the PR
pnpm run bumpand flag the impacts properlyAdvanced
import typefromjest-circusused internally only.