Skip to content

Chore: Change capital pool unit tests to use ethers instead of web3/truffle #600

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Jan 6, 2023
Merged

Chore: Change capital pool unit tests to use ethers instead of web3/truffle #600

merged 7 commits into from
Jan 6, 2023

Conversation

0xdewy
Copy link
Contributor

@0xdewy 0xdewy commented Jan 5, 2023 •
edited
Loading

Context

Changes proposed in this pull request

This PR replaces web3/truffle with ethers in all the capital pool unit tests

Test plan

Checklist

  • Rebased the base branch
  • Attached corresponding Github issue
  • Prefixed the name with the type of change (i.e. feat, chore, test)
  • Performed a self-review of my own code
  • Followed the style guidelines of this project
  • Made corresponding changes to the documentation
  • Didn't generate new warnings
  • Didn't generate failures on existing tests
  • Added tests that prove my fix is effective or that my feature works

Review

When reviewing a PR, please indicate intention in comments using the following emojis:

  • 🍰 = Nice to have but not essential.
  • 💡 = Suggestion or a comment based on personal opinion
  • 🔨 = I believe this should be changed.
  • 🤔 = I don’t understand something, do you mind giving me more context?
  • 🚀 = Feedback

@0xdewy 0xdewy changed the title Feature: Convert all capital pool unit tests from web3/truffle to ethers Chore: Change capital pool unit tests to use ethers instead of web3/truffle Jan 5, 2023
@0xdewy 0xdewy marked this pull request as ready for review January 5, 2023 23:16
roxdanila
roxdanila reviewed Jan 6, 2023
View reviewed changes
@roxdanila roxdanila force-pushed the feature/update-pool-tests-to-ethers branch from 652f98d to a6b9017 Compare January 6, 2023 11:38
danoctavian
danoctavian reviewed Jan 6, 2023
View reviewed changes
test/unit/Pool/setup.js
const chainlinkNewAsset = await ChainlinkAggregatorMock.new();
await chainlinkNewAsset.setLatestAnswer(new BN((1e18).toString()));
const chainlinkSteth = await ChainlinkAggregatorMock.deploy();
await chainlinkSteth.setLatestAnswer(WeiPerEther);
Copy link
Contributor

Choose a reason for hiding this comment

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

good find this constant, was unaware

danoctavian
danoctavian reviewed Jan 6, 2023
View reviewed changes
danoctavian
danoctavian previously requested changes Jan 6, 2023
View reviewed changes
Copy link
Contributor

@danoctavian danoctavian left a comment

Choose a reason for hiding this comment

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

Looks great overall

Would like a function for that hex(code.padEnd(8, '\0') sequence since it's used so often.

That 8 is the same i don't see the use case for a parameter there (yet).

@roxdanila roxdanila force-pushed the feature/update-pool-tests-to-ethers branch from a6b9017 to 8f45f5f Compare January 6, 2023 14:09
@0xdewy 0xdewy requested a review from danoctavian January 6, 2023 17:06
@roxdanila roxdanila merged commit be0e005 into nexus-v2 Jan 6, 2023
@roxdanila roxdanila deleted the feature/update-pool-tests-to-ethers branch January 6, 2023 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shark0der shark0der shark0der approved these changes

@roxdanila roxdanila roxdanila approved these changes

@danoctavian danoctavian Awaiting requested review from danoctavian

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@0xdewy @shark0der @danoctavian @roxdanila