feat(falsy arbitrary): add arbitrary, tests, and docs #627
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 1dc610d:
|
| import { Arbitrary } from './definition/Arbitrary'; | ||
| import { constantFrom } from './ConstantArbitrary'; | ||
|
|
||
| type FalsyType = boolean | null | number | string | typeof NaN | undefined; |
There was a problem hiding this comment.
I'd maybe type it with something as strict as possible. Unfortunately NaN cannot be strictly typed contrary to all others.
| type FalsyType = boolean | null | number | string | typeof NaN | undefined; | |
| type FalsyType = false | null | undefined | 0 | '' | typeof NaN; |
There was a problem hiding this comment.
ah ok cool, I didn't consider using the primitives themselves in the union, but that makes sense. Thanks for the suggestion. Will get to it this evening.
| * - undefined | ||
| */ | ||
| function falsy(): Arbitrary<FalsyType> { | ||
| return constantFrom<FalsyType>(false, null, undefined, 0, '', NaN); |
There was a problem hiding this comment.
Unrelated
It makes me think that typings of constantFrom should clearly be enhanced (in another review / issue / pr). Ideally one should not have to add <MyType> when using constantFrom but today, there is no other choice :/
| @@ -0,0 +1,19 @@ | |||
| import { Arbitrary } from './definition/Arbitrary'; | |||
There was a problem hiding this comment.
In order to expose fc.falsy, you'll need to import it into and export it from src/fast-check-defaults.ts
There was a problem hiding this comment.
doh! Didn't even think about the export. Will get to it this evening.
| it('Should be able to produce all the falsy values', () => | ||
| fc.assert( | ||
| fc.property( | ||
| fc.anything().filter((v) => !v), | ||
| fc.integer(), | ||
| (falsyValue, seed) => { | ||
| const mrng = stubRng.mutable.fastincrease(seed); | ||
| const arb = falsy(); | ||
| for (let id = 0; id !== 10000; ++id) { | ||
| const g = arb.generate(mrng).value; | ||
| if (g === falsyValue || (isLiterallyNaN(g) && isLiterallyNaN(falsyValue))) return true; | ||
| } | ||
| return false; | ||
| } | ||
| ) | ||
| )); | ||
| }); |
There was a problem hiding this comment.
While this test could be cool, it would take longer than expected to run. Indeed building a single value for fc.anything().filter((v) => !v) can take long.
Under the hood, we first build a random value for fc.anything() then we check if it passes the filter (v) => !v. If not we build another random value for fc.anything()... and so on and so forth until we generated 100 random values that fullfil (v) => !v.
For the moment, I'll go without this test.
There was a problem hiding this comment.
ok cool, will remove this test
Edit: I considered evaluating against a const falsyValues = [false, ...] variable in the test, but then I'm testing that my test has the same values as the arbitrary...
Any suggestions on more efficient approaches would be great!
| * - null | ||
| * - undefined | ||
| */ | ||
| function falsy(): Arbitrary<FalsyType> { |
There was a problem hiding this comment.
To be defined in another review / PR:
There is actually one value missing from it: Bigint(0). But as the library is today compatible with node 0.12 (very very old release of node), we cannot put it by default. It should be under some kind of flag but I don't know which one 🤔
{ lib: "es2020" } or { withBigint: true }, we can certainly do that in another review, it was more to highlight that this value is missing.
There was a problem hiding this comment.
Alternatively a property with an array of enums - I'm not sure if other values should be considered. This also allows the user to determine which values they actually want generated, if we consider future potential values.
e.g.
enum FalsyValue {
bigint0 = 'big-int-0',
//... other potential values?
}
fc.falsy({
enabledValues: [FalsyValue.bigint0, ...]
})* master: ✅ E2E for AsyncScheduler were relying on changing data (dubzzz#630) ⬆️ Bump rollup from 2.15.0 to 2.16.1 (dubzzz#628)
|
Perfect 👌 |
|
I am going to log issues related to the remaining remarks related to bigint and constantFrom not properly typed |
Why is this PR for?
Adding a
falsyarbitrary as per #624In a nutshell
✔️ New feature
Potential impacts
NA