Skip to content
This repository was archived by the owner on Jul 14, 2025. It is now read-only.

Conversation

kangarang
Copy link

@kangarang kangarang commented May 22, 2018

This PR address #125, adding tests to eip20Factory.js

  • Clean up the current test
  • Add test for created mapping
  • Add tests for verifyEIP20
  • Add comments

@kangarang kangarang changed the title cleanup, add comments & test Factory tests May 23, 2018
@kangarang
Copy link
Author

^ Ready for review! Thanks @maurelian for the guidance 👍

const result = await factory.verifyEIP20.call(newTokenAddr, { from: accounts[0] });
assert.strictEqual(result, true, 'the bytecode at newTokenAddr '
+ 'was not the same as the bytecode of an EIP20 token');

Copy link
Contributor

Choose a reason for hiding this comment

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

for consistency, would add // verify: new token's address is listed in the isEIP20 mapping


const { contractAddress } = await web3.eth.getTransactionReceipt(txHash);
const result = await factory.verifyEIP20.call(contractAddress);
assert.strictEqual(result, true, 'should have returned true because the bytecode was exact');
Copy link
Contributor

Choose a reason for hiding this comment

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

This is failing for me. Here's some output from inserting a debugger statement on the line above, and inspecting the run time:

code == EIP20Bytecode
false
code.length
6262
EIP20Bytecode.length
7076

It looks like the hardcoded bytecode above was generated by a different truffle/solc version than the one listed in package.json here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I fixed this test by changing line 2 to
const EIP20Bytecode = artifacts.require('EIP20').bytecode;

It feels a bit circular, but it translates to: "does the bytecode from compiling EIP20, match the bytecode deployed by the EIP20Factory".

Copy link
Contributor

Choose a reason for hiding this comment

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

This test is also broken when rebased onto staging because the factory is using SafeEIP20 there, so this is dependent on that.

@kangarang
Copy link
Author

made those 2 changes. will wait til #135 gets merged into master then make changes to support SafeEIP20

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants