Potential Issue with Test Coverage and Functionality in Finance Module

[techvoyagerX]

During my review of the finance module's test suite in the OpenZeppelin Contracts repository, I observed potential concerns regarding the adequacy and reliability of the tests, particularly in achieving full coverage. Specifically:

Test Coverage: While attempting to run the coverage report using brownie coverage, I encountered errors that prevented the successful generation of the full coverage report. This issue may hinder the ability to assess the robustness of the test suite fully.

Test Functionality: I noticed that some tests might not be validating the correct behavior. For instance, certain tests are using the wrong accounts or parameters, which could lead to false positives/negatives during test execution.

Given the importance of maintaining high standards for test coverage and accuracy, especially in a widely-used library like OpenZeppelin Contracts, I recommend reviewing and enhancing the existing test suite in the finance module. This will ensure that all critical paths are covered and that the tests are correctly validating the intended functionality.

[Amxx]

Hello @techvoyagerX

We run test coverage using hardhat and solidity-coverage. Coverage run is done using scripts/checks/coverage.sh. This is integrated into the CI, which validates a high level of coverage.

Last commit on master is documented here

• 99.41% of statements
• 97.32% of branches
• 99.49% of functions
• 99.43% of lines

As for the tests, if you find any of our test to be erroneous, please let us know which one exactly so we can fix that.