dubzzz / jest-fast-check

Property based testing for Jest based on fast-check
MIT License
16 stars 0 forks source link

Better `beforeEach` and `afterEach` behavior #165

Open TomerAberbach opened 2 years ago

TomerAberbach commented 2 years ago

It's a bit unfortunate that fast-check has separate lifecycle hooks than jest (e.g. fast-check has asyncBeforeEach and asyncAfterEach via configureGlobal while jest has beforeEach and afterEach).

Seems jest doesn't have a way to hook into the functionality, but I wonder if we could include something like this in the package so that calling beforeEach and afterEach also sets it up for testProp.

import { readConfigureGlobal, configureGlobal } from 'fast-check'

const previousBeforeEach = global.beforeEach
const previousAfterEach = global.afterEach

let during = false

global.beforeEach = (fn, ...args) => {
  previousBeforeEach(async () => {
    if (!during) {
      try {
        await fn()
      } finally {
        during = true
      }
    }
  }, ...args)

  const { asyncBeforeEach: previousAsyncBeforeEach = () => {} } =
    readConfigureGlobal()

  configureGlobal({
    ...readConfigureGlobal(),
    async asyncBeforeEach() {
      if (!during) {
        try {
          await previousAsyncBeforeEach()
          await fn()
        } finally {
          during = true
        }
      }
    },
  })
}

global.afterEach = (fn, ...args) => {
  previousAfterEach(async () => {
    if (during) {
      try {
        await fn()
      } finally {
        during = false
      }
    }
  }, ...args)

  const { asyncAfterEach: previousAsyncAfterEach = () => {} } =
    readConfigureGlobal()

  configureGlobal({
    ...readConfigureGlobal(),
    async asyncAfterEach() {
      if (during) {
        try {
          await previousAsyncAfterEach()
          await fn()
        } finally {
          during = false
        }
      }
    },
  })
}

I've haven't tested this extensively, but I think it ensures that beforeEach and afterEach run before and after each "test" without running twice (which is what unfortunately happens normally if you add the same code to jest's beforeEach and fast-check's asyncBeforeEach)

Let me know what you think! I'd be happy to make a PR

dubzzz commented 2 years ago

The idea would be great, but I am not sure fast-check will work well. If I remember well one cannot set both async and sync versions of before/afterEach at the same time for fast-check. There is probably a need to check for sync/async and register the right version on fast-check.

Otherwise the idea looks legit. I'd probably put such intrusive behavior into a 'jest-fast-check/hoist' or something like that. So that in only applies when this specific file is imported (not sure if it is better or not) 🤔

TomerAberbach commented 2 years ago

For your first point, from looking at the source code jest-fast-check only ever uses asyncProperty (even for possibly only synchronous code). So I think we can get away with only instrumenting asyncBeforeEach and asyncAfterEach for fast-check.

For your second point, yeah I agree we should have the code in its own separately imported module. We could instruct users to include the import in setupFilesForEnv. I've seen other packages suggest something similar

TomerAberbach commented 2 years ago

Oof, I started trying to implement this and realized it might be really complicated to make this work with beforeEach and afterEach scoping in describe blocks...

TomerAberbach commented 2 years ago

It's possible to resolve this for beforeEach by adding tracking to all Jest beforeEach calls because that indicates which ones fast-check should call later. But this doesn't work for afterEach because, well, it runs after fast-check's afterEach runs.

I also looked into whether we could just refactor this module to use test.each, but I don't think that can work either because it only works with arrays as input. If it accepted an async iterable, then I think we could write a lazy async iterable that yields each fast-check test case, and that would inherit the beforeEach/afterEach behavior automatically.

So unless you have any ideas I think we have two options:

  1. Not implement anything here until Jest provides a better way to hook into beforeEach/afterEach
  2. Implement a version that only works with top-level beforeEach/afterEach calls

Up to you!