feat(json-schema-2020-12-samples): support randomness option #9810
Conversation
| @@ -4,7 +4,17 @@ | |||
| import Registry from "./Registry" | |||
|
|
|||
| class OptionRegistry extends Registry { | |||
| #defaults = {} | |||
| #defaults = { | |||
| randomEnabled: false, | |||
There was a problem hiding this comment.
These boolean properties might be confusing as they come with double boolean logic. Let's use string enum of [random, constant].
| randomEnabled: false, | |
| mode: 'constant', |
There was a problem hiding this comment.
Ok makes sense. Actually nice as there could be more modes in the future.
| #defaults = { | ||
| randomEnabled: false, | ||
| random: Math.random, | ||
| minInt: 0, |
There was a problem hiding this comment.
Let's align with json-schema-faker here:
| minInt: 0, | |
| minInteger: -100000000, |
| randomEnabled: false, | ||
| random: Math.random, | ||
| minInt: 0, | ||
| maxInt: 2147483647, |
There was a problem hiding this comment.
| maxInt: 2147483647, | |
| maxInteger: +100000000, |
| random: Math.random, | ||
| minInt: 0, | ||
| maxInt: 2147483647, | ||
| minLen: 4, |
There was a problem hiding this comment.
| minLen: 4, | |
| minLength: 0, |
There was a problem hiding this comment.
@char0n this will generate samples with empty strings by default? Is that what we want as a default behavior?
| minInt: 0, | ||
| maxInt: 2147483647, | ||
| minLen: 4, | ||
| maxLen: 10, |
There was a problem hiding this comment.
No upper boundary
| maxLen: 10, | |
| maxLength: Number.MAX_SAFE_INTEGER, |
There was a problem hiding this comment.
@char0n this will generate samples with extremely long strings by default? Is that what we want as a default behavior?
| maxInt: 2147483647, | ||
| minLen: 4, | ||
| maxLen: 10, | ||
| minDateTime: new Date("1970-01-01T00:00:00.000Z"), |
There was a problem hiding this comment.
| minDateTime: new Date("1970-01-01T00:00:00.000Z"), | |
| minDateTime: new Date("1889-12-31T00:00:00.000Z"), |
| maxLen: 10, | ||
| minDateTime: new Date("1970-01-01T00:00:00.000Z"), | ||
| maxDateTime: new Date("2038-01-19T00:00:00.000Z"), | ||
| maxRandExp: 100, |
There was a problem hiding this comment.
| maxRandExp: 100, | |
| defaultRandExpMax: 10, |
There was a problem hiding this comment.
Yeah 10 is fine which is what I initially thought of, but note that it will change the default behavior of randExp which is 100. But I think it's for the better to keep samples shorter and more readable.
src/core/plugins/json-schema-2020-12-samples/fn/class/Registry.js
Outdated
Show resolved
Hide resolved
|
@char0n let me know if this one is ok before I move to next, I think it should be good to go. Btw I changed the account where I host the fork but shouldn't affect the PR. |
|
@char0n would you or another maintainer be able to look into merging this? I hope to implement the support for propertyNames but there are quite a few more PRs to go, so would like to get started on the next after finished this one. |
Refs #9739
Description
Add support for randomness, enabled via optionAPI and implemented in core/random.js.
Default is no randomness so everything is backward compatible.
For now only used in the minimal number of generates to enable full unit test code coverage of the changes in core/random.js. Other generators can follow in next PRs.
Open question: do we need changes to the documentation? It could make sense but I was expecting a place to already find documentation about APIs like formatAPI where we can add description for optionAPI. We can also do this later when it's actually working with all types/formats and/or exposed to users.
Motivation and Context
This is one step in #9739 to eventually support propertyNames, while also having other uses like configuration option to use randomness across the board.
How Has This Been Tested?
Unit tests have been added to cover the changes, and existing tests still pass.
Checklist
My PR contains...
src/is unmodified: changes to documentation, CI, metadata, etc.)package.json)My changes...
Documentation
Automated tests