[CENT-331]: Unify checkVariables and checkState #221
Conversation
…kMINTp0 function for unit tests
mirathewhite
changed the title
mirathewhite
force-pushed
the
checkState
branch
from
61541e3 to
fd3ded7
Compare
There was a problem hiding this comment.
Awesome to see these tests working!
It would be nice if we could run all the tests with just checkVariables, rather than a new method checkMINTp0 that requires different syntax for writing tests. This might be better suited for a separate PR, but here's what I'm thinking:
When we create the MintController token object, add a property to the truffle contract object, mintController.tokenType = "MintController". Then in checkVariables, the buildExpectedState and getActualState functions check the tokenType property before determining what state the token should have. If tokenType == MintController, delegate to getActualStateOfMintController and buildMintControllerExpectedState.
I left some other minor questions/comments, we don't necessarily have to address the checkVariables thing in this PR, but figured I'd bring it up.
test/minting/ControllerTestUtils.js
Outdated
| @@ -1,5 +1,6 @@ | |||
| var BigNumber = require('bignumber.js'); | |||
| var Q = require('q'); | |||
| var clone = require('clone'); | |||
| } | ||
|
|
||
|
|
||
| // Gets the actual state of the mintController contract. | ||
| // Evaluates all mappings on the provided accounts. | ||
| async function getActualMintControllerState(mintController, accounts) { | ||
| return { | ||
| 'minterManager': await mintController.minterManager.call() | ||
| var minterManager = await mintController.minterManager.call(); |
There was a problem hiding this comment.
Should this check that the value of each account in accounts in the controllers mapping? Looks like that was the original plan based on the accounts param and the comment. Also, maybe owner should be added, since that is also part of the state?
There was a problem hiding this comment.
The function checkMintControllerState calls checkControllerState in ControllerTestUtils.js. This lets us take advantage of inheritance: MintController is Controller. So the controllersmapping andowner` variable states are checked there.
There was a problem hiding this comment.
In that case can we remove the accounts param here?
| @@ -23,9 +23,9 @@ | |||
| version "0.7.0" | |||
There was a problem hiding this comment.
looks like we need to run "yarn install" again from build error
|
We discussed creating a special function |
-Created checkMINTp0 that is specific for testing MINT p0 contracts.
-Created ControllerState and MintControllerState objects
-Replaced cloneState with 3rd party clone() function.