-
Notifications
You must be signed in to change notification settings - Fork 368
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Basic tests #224
Basic tests #224
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm, left a few minor comments
test/minting/MintP0BasicTests.js
Outdated
expectedMintControllerState.controllers['controller1Account'] = Accounts.minterAccount; | ||
await checkMINTp0([token, mintController], [expectedTokenState, expectedMintControllerState]); | ||
|
||
// now set controllers[controller1Account]=arbitraryAccount |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comment needs to be updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
test/minting/MintP0_ABITests.js
Outdated
var sendRawTransaction = abiUtils.sendRawTransaction; | ||
var functionSignature = abiUtils.functionSignature; | ||
var encodeAddress = abiUtils.encodeAddress; | ||
var encodeUint = abiUtils.encodeUint; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
functionSignature, encodeAddress, encodeUint are unused
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
test/minting/MintP0_ABITests.js
Outdated
var msgData = abiUtils.msgData; | ||
var msgData1 = abiUtils.msgData1; | ||
var msgData2 = abiUtils.msgData2; | ||
var msgData3 = abiUtils.msgData3; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
msgData2, msgData3 are unused
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
Controller.sol,configureController,controllers[C]=0,bt019,bt019 configureController works when controller[C]=0 | ||
Controller.sol,configureController,controllers[C] != 0,bt020,bt020 configureController works when controller[C] != 0 | ||
Controller.sol,configureController,controllers[C]=C,bt021,"bt021 configureController(C,C) works" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably want to remove quotes around descriptions in this csv for consistency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed all quotes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Returned quotes because they are needed. If the description has a comma inside it, then the entire column contents needs to be in quotation marks.
@@ -205,7 +193,7 @@ function verification_reporter (runner) { | |||
|
|||
// Do not report missing unit tests for files that did not run | |||
for(var testSuite in missingUnitTests){ | |||
if(! _.has(executedTestSuites, testSuite) && ! testSuite.match(/Completeness/)) { | |||
if(! _.has(executedTestSuites, testSuite) /* && ! testSuite.match(/Completeness/)*/) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you leave a comment on the PR why this change is necessary just to clarify?
I modified verification/verification_reporter.js to exclude showing missing Completeness unit tests. It is difficult to write/test single unit test files because in this case ALL tests in the Completeness CSV files will be printed as missing. I created a work-item to fine-tune this script to show missing Completeness unit test units only when testing the whole project. |
Adding unit tests: BasicTests and ABITests