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

Drop itProp for it.prop in @fast-check/jest #3303

Closed
dubzzz opened this issue Oct 12, 2022 · 3 comments
Closed

Drop itProp for it.prop in @fast-check/jest #3303

dubzzz opened this issue Oct 12, 2022 · 3 comments

Comments

@dubzzz
Copy link
Owner

dubzzz commented Oct 12, 2022

💡 Idea

The source API of Jest is only based on it. You can call it.each, it.failing.each, it.concurrent.failing.each and many others.

In order to be even more integrated with the existing API provided by Jest an idea could be to add prop as any other customizer already provided by Jest. More precisely to have prop every time Jest offer each.

Then we need to find a new way to instantiate fast-check that will be closer to the it.each one. it.each is basically a it.each(inputs)(name, fn, timeout) what if we proposed something like:

it.prop(fc.string(), fc.string(), fc.string())('should always contain its substring', (a, b, c) => {
  expect(a + b + c).toContain(b);
}, {/* assert options */});

And possibly a variant:

it.prop({ a: fc.string(), b: fc.string(), c: fc.string() })('should always contain its substring', ({ a, b, c }) => {
  expect(a + b + c).toContain(b);
}, {/* assert options */});
@dubzzz
Copy link
Owner Author

dubzzz commented Oct 12, 2022

@TomerAberbach I'm pretty sure you'll have a useful opinion on this one.

@TomerAberbach
Copy link
Contributor

I like the idea of being consistent with Jest! I do have a couple nitpicks with the currently proposed API though.

First, since Jest's it.each function takes either:

  1. An array of arrays of values: results in the function receiving its parameters as spread from the inner arrays
  2. An array of non-array values: mapped to an array of arrays containing single values. Results in the function receiving a single parameter

The "an array of" part doesn't make sense for fast-check because we don't have an array of test cases. The arbitraries themselves generate the values. So if we're trying to match Jest's API while adhering to fast-check's semantics we would remove the top-level "an array of" part and replace "value" with "arbitrary":

  1. An array of arbitraries: results in the function receiving its parameters as spread from the array of generated values
  2. An arbitrary: mapped to an array containing a single arbitrary. Results in the function receiving a single generated value

Based on that, your first example would be written as:

it.prop([fc.string(), fc.string(), fc.string()])('should always contain its substring', (a, b, c) => {
  expect(a + b + c).toContain(b);
}, {/* assert options */});

And your second example would be written as:

it.prop(fc.record({ a: fc.string(), b: fc.string(), c: fc.string() }))('should always contain its substring', ({ a, b, c }) => {
  expect(a + b + c).toContain(b);
}, {/* assert options */});

I think this would be most consistent with Jest's API. Not sure there's a compelling reason to special case the object literal case if Jest doesn't (it's just case number 2 in my list above).

Second, Jest supports a timeout for the third parameter of the function returned by it.each so I'm not sure we should use that for the fast-check options. Perhaps the fast-check options should be in the second parameter of it.prop?

@dubzzz
Copy link
Owner Author

dubzzz commented Oct 21, 2022

Thanks a lot for the suggestions.

I started to POC a very first version of it. So far I only implemented the tuple-based version but I'll add an object one soon. For the later, I believe the best would be not to ask for the fc.record if we do provide such signature.

dubzzz added a commit that referenced this issue Oct 23, 2022
dubzzz added a commit that referenced this issue Oct 28, 2022
dubzzz added a commit that referenced this issue Oct 28, 2022
thewilkybarkid added a commit to PREreview/prereview.org that referenced this issue Nov 21, 2022
The fast-check integration with Jest has a new syntax based on Jest's standard API. This change helps to reduce the indentation in a few places.

Refs dubzzz/fast-check#3303
@dubzzz dubzzz closed this as completed Dec 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants