add contracts for token and crowdsale, and solidity tests #5
Conversation
| @@ -31,7 +33,9 @@ | |||
| "scripts": { | |||
| "start": "react-scripts start", | |||
| "build": "react-scripts build", | |||
| "test": "mocha --require babel-core/register --require test/setup.js", | |||
| "test": "npm run test:react && npm run test:contracts", | |||
| "test:contracts": "truffle test test/contracts/* --network unexisting", | |||
There was a problem hiding this comment.
I found this weird. If I didn't set network, or set it to some existing network, it would say that the contract couldn't be deployed because of gas.
There was a problem hiding this comment.
I think that by default the test network will be the development network which has a gasLimit. Did you try a higher one?
There was a problem hiding this comment.
Yes. But then I had plenty of problems with deploying the contracts to geth, and I read something about a wrong constructor. I made many changes back and forth, until it started working, so I'm not sure what was wrong.
I reported #7, to investigate this.
| public | ||
| returns (bool) | ||
| { | ||
| require(jurisdiction.hasAttribute(_to, 'VALID')); |
There was a problem hiding this comment.
This needs to be checked in transfer and transferFrom too.
There was a problem hiding this comment.
Thanks for catching this!
I added the overwrites, and tests specific to the token.
| @@ -31,7 +33,9 @@ | |||
| "scripts": { | |||
| "start": "react-scripts start", | |||
| "build": "react-scripts build", | |||
| "test": "mocha --require babel-core/register --require test/setup.js", | |||
| "test": "npm run test:react && npm run test:contracts", | |||
| "test:contracts": "truffle test test/contracts/* --network unexisting", | |||
There was a problem hiding this comment.
I think that by default the test network will be the development network which has a gasLimit. Did you try a higher one?
|
|
||
| beforeEach(async function () { | ||
| this.jurisdiction = await Jurisdiction.new(); | ||
| this.jurisdiction.addValidator(owner); |
|
|
||
| it('should create crowdsale with correct parameters', async function () { | ||
| this.crowdsale.should.exist; | ||
| this.token.should.exist; |
There was a problem hiding this comment.
I think these two lines are redundant... We can assume the contract constructors to return an instance.
|
Thanks @frangio! This is ready for another review. |
|
I forgot to say that any suggestions about the style of the contracts and tests would be appreciated. I'm currently just copying stuff I saw somewhere else. |
| { from: owner } | ||
| ).should.be.fulfilled; | ||
| (await this.token.balanceOf(investor1)) | ||
| .should.be.bignumber.equal(investmentAmount); |
There was a problem hiding this comment.
Since you're using chai-as-promised you should be able to write
await this.token.balanceOf(investor1).should.eventually.be.bignumber.equal(investmentAmount)There was a problem hiding this comment.
Do you think that reads better? It's a lot longer than adding two parentheses.
There was a problem hiding this comment.
It probably doesn't read better. No strong feelings about this.
|
Alright, merging. Thanks @frangio! |
No description provided.