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

Rb/truffle to foundry #595

Merged
merged 5 commits into from
Jul 22, 2022
Merged

Rb/truffle to foundry #595

merged 5 commits into from
Jul 22, 2022

Conversation

RiccardoBiosas
Copy link
Contributor

@RiccardoBiosas RiccardoBiosas commented Jul 13, 2022

What does this pull request do? Explain your changes. (required)
Since the protocol repo has been gradually adopting Foundry to run Solidity-based tests - which provides a better alternative to Truffle-, the PR refactors the current truffle-based Solidity tests so that they can be run as part of the Foundry codebase.

Specific updates (required)

How did you test each of these updates (required)
Successfully run yarn test and forge test -v --match-contract Test to test all the new foundry-based unit tests

Does this pull request close any open issues?
#576

Checklist:

  • README and other documentation updated
  • All tests using yarn test pass

@RiccardoBiosas RiccardoBiosas changed the base branch from confluence to cf/next July 13, 2022 12:00
@coveralls
Copy link

Pull Request Test Coverage Report for Build 2663238582

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 10 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-2.0%) to 97.399%

Files with Coverage Reduction New Missed Lines %
contracts/libraries/MathUtils.sol 2 50.0%
contracts/libraries/PreciseMathUtils.sol 2 50.0%
contracts/libraries/SortedDoublyLL.sol 6 87.14%
Totals Coverage Status
Change from base Build 2556706908: -2.0%
Covered Lines: 768
Relevant Lines: 781

💛 - Coveralls

Copy link
Contributor

@red-0ne red-0ne left a comment

Choose a reason for hiding this comment

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

Left one non-blocking comment. Other than that, looks all good to me.

Copy link
Contributor

@kautukkundan kautukkundan left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@kautukkundan kautukkundan left a comment

Choose a reason for hiding this comment

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

Ah actually it would be great to add additional tests for the reduced coverage - https://coveralls.io/builds/50828602
Should be small only

@RiccardoBiosas
Copy link
Contributor Author

RiccardoBiosas commented Jul 22, 2022

Ah actually it would be great to add additional tests for the reduced coverage - https://coveralls.io/builds/50828602 Should be small only

The only reason why the coverage dropped is because the package solidity-coverage is not currently taking into account the foundry tests - unlike the previous truffle-based tests run via runSolidityTest.js. Foundry has recently added a coverage feature in one of its latest 2.x releases but we cannot upgrade to it right away due to this issue in the way foundry 2.x handles L2 networks: once the issue will be addressed I think we can fix the 'fake' coverage drop.

Copy link
Contributor

@kautukkundan kautukkundan left a comment

Choose a reason for hiding this comment

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

Gotcha! Thanks for the note.

@RiccardoBiosas RiccardoBiosas merged commit ac937f0 into cf/next Jul 22, 2022
@hjpotter92 hjpotter92 deleted the rb/truffle-to-foundry branch February 2, 2024 03:55
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.

4 participants