There are a bunch of Node construction helpers which take anywhere from one to 5 params. Two of these params are for offline mode and mock mining, aka alwaysWinningTicket. Offline mode is a Node ConfigOpt, and if any ConfigOpts are passed to some of these helpers and specifically to genNode, the offline flag will override that config.
Proposal is 2 things:
Try to get rid of any flags that override node InitOpts or ConfigOpts; there should be only one way of setting up either one and extra flags may do things surprising to the test writer and result in false positives or negatives in test cases.
Reduce the number of params being passed around to set up a node to two: a text context and A node options struct which takes all the necessary settings to set up a node the way you want. This struct is at minimum node.InitOpts and node.ConfigOpts, but may include other things.
Acceptance criteria
Anything that uses the node test helpers uses the refactored helpers.
Setting up a customized node in test cases is easier to figure out and write
There is only one way to do it and it's clear
Allows test writer to set up nodes closer to real life situation, or to fake as much as needed for better unit testing
Test code in general is easier to read
Easier and obvious how to write more test helpers
No unused functions
All test pass of course
Risks + pitfalls
when investigating this, because we supply a function that sets things like offline mode or addresses, there's currently no way to query what is being set when we pass options to genNode (and hence also to the node instantiator). It means for example it's difficult to know, without the "offlineMode" flag, that the swarm address needs to change from 0.0.0.0 based to 127.0.0.1. It will need some thought.
shannonwells
commented
5 years ago
Description
There are a bunch of Node construction helpers which take anywhere from one to 5 params. Two of these params are for offline mode and mock mining, aka alwaysWinningTicket. Offline mode is a Node ConfigOpt, and if any ConfigOpts are passed to some of these helpers and specifically to genNode, the offline flag will override that config.
Proposal is 2 things:
Try to get rid of any flags that override node InitOpts or ConfigOpts; there should be only one way of setting up either one and extra flags may do things surprising to the test writer and result in false positives or negatives in test cases.
Reduce the number of params being passed around to set up a node to two: a text context and A node options struct which takes all the necessary settings to set up a node the way you want. This struct is at minimum
node.InitOpts andnode.ConfigOpts, but may include other things.Acceptance criteria
Risks + pitfalls
when investigating this, because we supply a function that sets things like offline mode or addresses, there's currently no way to query what is being set when we pass options to genNode (and hence also to the node instantiator). It means for example it's difficult to know, without the "offlineMode" flag, that the swarm address needs to change from 0.0.0.0 based to 127.0.0.1. It will need some thought.