Skip to content
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

Increase code coverage #212

Merged
merged 4 commits into from
Sep 14, 2018
Merged

Increase code coverage #212

merged 4 commits into from
Sep 14, 2018

Conversation

mirathewhite
Copy link
Contributor

Wrote more unit tests to increase code coverage to 100%

test/minting/MintControllerTests.js Outdated Show resolved Hide resolved
test/minting/MintControllerTests.js Show resolved Hide resolved
'token': token.address,
'controllers': {'controller1Account': A.minterAccount }
}
await checkMintControllerState([mintController], [customState]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is going to be confusing to write tests and have separate functions for checkVariables for different contracts (such as checkMintControllerState) as when others write tests they wont know which functions to call and may neglect to check the state of a particular contract. What do you think of making checkVariables work for all our contracts that will force checking of all contracts (checkMinterControllerState could become a function within checkVariables)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll think about how to rewrite this. I want to avoid rewriting existing unit test to use a new framework, but there might be a way to get checkVariables to handle both. This will be a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -70,14 +53,14 @@ async function run_tests(newToken, accounts) {
it('abi004 FiatToken pause() is public', async function () {
let badData = functionSignature('pause()');
var tx = new Tx({
nonce: web3.toHex(web3.eth.getTransactionCount(pauserAccount)),
nonce: web3.toHex(web3.eth.getTransactionCount(Accounts.pauserAccount)),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mira, what do you think of exporting these variables in an object directly instead of using Accounts. It looks a little more verbose to use Accounts.[accountName] in every test?

@o-a-hudson o-a-hudson merged commit 5c13426 into circlefin:multi-issuer Sep 14, 2018
@mirathewhite mirathewhite deleted the code-coverage branch September 17, 2018 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants