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

[Proposal] Migrate to fast-check #353

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

Conversation

dubzzz
Copy link

@dubzzz dubzzz commented Jun 15, 2021

What?
This Pull Request is mostly a proposal to migrate from jsverify to fast-check.

Why?
jsverify's owner stopped the maintenance of the library more than a year ago. In addition, the last real commit to jsverify was on the 6th of September 2019.

Benefits of fast-check over jsverify

  1. You don't need to define unmapping fuctions on your calls to map (smap in jsverify).
  2. The arbitraries are able to find more bugs. For instance if you take a look into the updated tests you'll see that I had to calm down the generator of numbers because it was reporting issues if we used it fully.
  3. There is a built-in way to handle commands. But I did not adapt the tests of LRUCache yet (they can definitely use the builder of commands provided by fast-check).
  4. Maintained.

key: jsc.elements(['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h']),
value: jsc.nat,
}) as any
const arbitraryCommand = fc.record({
Copy link
Author

Choose a reason for hiding this comment

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

There is a built-in way to deal with commands in fast-check. I could probably migrate the test to it.

Comment on lines +7 to +15
let numericType = fc
.double({
next: true,
noDefaultInfinity: true,
noNaN: true,
min: -1 / numericTypeMinAccuracy,
max: 1 / numericTypeMinAccuracy,
})
})
.map(d => Math.round(d / numericTypeMinAccuracy) * numericTypeMinAccuracy)
Copy link
Author

Choose a reason for hiding this comment

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

Here the arbitrary fc.double was able to find potential issues when used with full resolution. Contrary to jsc.number it can generate any number from -Infinity to +Infinity including -0, Number.MIN_VALUE, Number.EPSILON and others (from small to large values).

Comment on lines +235 to +236
// If the above expect(...) assertion fails, we won't reach here.
return true
Copy link
Author

Choose a reason for hiding this comment

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

No need for return with fast-check. No return or return true is success. Throw or return false is failure.

@dubzzz dubzzz changed the title Migrate to fast-check [Proposal] Migrate to fast-check Jun 15, 2021
@dubzzz
Copy link
Author

dubzzz commented Nov 24, 2024

Cc @jlfwong, jsverify is clearly dead today. Any blocker to switch to fast-check? More maintained, more powerful, more used...

@jlfwong
Copy link
Owner

jlfwong commented Nov 25, 2024

Hey @dubzzz! If you want to resolve the conflicts on this and get it through CI, I'm happy to merge a PR.

I didn't take a careful look at this in the past because, despite jsverify being dead, that wasn't (and tbh, isn't) causing me any real pain, nor does this resolve any user facing issues.

All that said, I agree it makes more sense to use a library that's maintained in an ongoing manner (especially if the creator is willing to do the migration for you 😄)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants