Internal improvement: Remove faker from abi-utils and generate arbitraries manually #5484
Conversation
eggplantzzz
force-pushed
the
ridda-faker
branch
2 times, most recently
from
8913b32
to
3d0a01a
Compare
There was a problem hiding this comment.
Seems basically good, but one part seems overly complicated, you might want to simplify it.
Btw, some history you may be interested in: When I added errors, I was originally going to do them as adjective-noun, but I ultimately went with noun-noun because one of faker's adjectives was "1080p" and that obviously caused problems if it went first! (Although I guess I could have filtered out things that started with digits.)
packages/abi-utils/lib/arbitrary.ts
Outdated
| transform = camelCase | ||
| ): fc.Arbitrary<string> => { | ||
| const results: fc.Arbitrary<string>[] = []; | ||
| for (const wordType of wordTypes) { |
There was a problem hiding this comment.
This seems overly complicated to me in two ways:
- Rather than having these separate noun-vs-verb branches, you could just use
wordLists[wordType]everywhere with no branch. - Instead of doing
fc.integer(...).noBias().noShrink().map(...), you could just usefc.constantFrom(...), if I'm understanding correctly?
It looks to me like doing those would simplify this function a lot.
There was a problem hiding this comment.
2. Instead of doing
fc.integer(...).noBias().noShrink().map(...), you could just usefc.constantFrom(...), if I'm understanding correctly?
OMG I was looking for this very thing!
@eggplantzzz I'll address this concern and push up a new commit, since this is my mess that Harry's showing issue with.
There was a problem hiding this comment.
Updated. Wow that's so much cleaner. Thanks @haltman-at!
| const EventName = () => Name(["verb", "noun"], pascalCase); | ||
| const ErrorName = () => Name(["noun", "noun"], pascalCase); | ||
| const FunctionName = () => Name(["verb", "noun"]); |
There was a problem hiding this comment.
I really like how this makes it seem like fast-check just wants to play Mad Libs
|
(Note this needs to be rebased against develop to solve the yarncheck problem.) |
... to match style of other arbitrary-generating functions
One of the MetaMask team pointed out that faker is a big dependency of abi-utils and asked Truffle to remove it as they are trying to cut down the size of their extension. Truffle does not really use faker for much besides generating random names for when it creates arbitrary abis. This PR implements a simple utility that generates these names manually.