Feature/add event tests #67
Conversation
neodaoist
commented
Oct 17, 2022
neodaoist
commented
- Add unit tests for event logging
- Move forking in-line and improve test run performance via fork caching
- Make couple minor test helper edits
- Clarify some NatSpec
- Add install+build+test instructions to README.md
There was a problem hiding this comment.
@neodaoist we need to adapt the github actions pipeline here to pass through the RPC_URL (which I have added as a secret), and remove --fork-url.
neodaoist
requested review from
0xAlcibiades
and removed request for
Flip-Liquid
| stdstore.target(token).sig(IERC20(token).balanceOf.selector).with_key(who).checked_write(amt); | ||
| } | ||
|
|
||
| // TODO consider better pattern to DRY up event definitions |
There was a problem hiding this comment.
Had the same thought, but I've run into this with forge before. they recommend redefinition in test code: https://book.getfoundry.sh/cheatcodes/expect-emit.
one possibility is to have the test contract implement (via revert()s) the interface which defines the events. its verbose but the bonus is that you'll get build time errors if the events change, rather than failing tests.
There was a problem hiding this comment.
Yeah, I do hear you. Redefinition isn't that bad. Your inheritance suggestion is interesting, it is better than altering production code to DRY up tests — e.g., I've tried putting events in a library or similar low-gas / no-gas approaches to DRY up definition between production and test code, but it doesn't seem a good trade-off bc then the production code itself is harder to read. Will leave as-is for now
…m-core into feature/add-event-tests
