Use ganache for tests instead of Infura HTTP API #51
Conversation
ca891af to
a7b3072
Compare
- remove integration-test checking on-chain state - pin ganache to 7.3.1 for Node.js v12 compatibility
a7b3072 to
6f67d71
Compare
| const { MethodRegistry } = require('../dist'); | ||
|
|
||
| const provider = new Eth.HttpProvider(`https://mainnet.infura.io/v3/${INFURA_PROJECT_ID}`); | ||
| const { provider } = Ganache.server(); |
There was a problem hiding this comment.
It looks like we're just testing the parse method in these tests; we're not testing the lookup method, which actually makes us of the provider. So, could we just use a fake provider object here? That way we don't even need Ganache.
There was a problem hiding this comment.
Sure, but perhaps in a follow-up? Given that this is currently blocking doing any other changes in this repo.
There was a problem hiding this comment.
@legobeat While I do love the idea of eliminating the need to make HTTP requests in tests, Ganache is going to be at its EOL on December 20th, and is no longer recommended in new projects.
Are we able to make this additional change here?
NEllusion
left a comment
There was a problem hiding this comment.
Spoke with Lego offline & agreed that making this change now is the best path forwards to unblock CI
ganacheinstead of external Infura HTTP API endpoint for teststypescriptto^4.8.4Fixes unit CI tests currently failing on
main.Related
Blocking