test(contracts-rfq): remove test contracts from coverage report #3291
Conversation
WalkthroughThis pull request introduces new empty test functions across various contracts to prevent them from appearing in coverage reports. The changes affect the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (17)
packages/contracts-rfq/test/mocks/NonPayableRecipient.sol (1)
7-8: LGTM: Appropriate addition of test functionThe new
testNonPayableRecipientfunction serves its purpose well by preventing the contract from being excluded from coverage reports. This aligns with the PR objective.A minor suggestion for improvement:
Consider adding the
puremodifier to the function since it doesn't read from or modify the contract's state. This would make the function's behavior more explicit:- function testNonPayableRecipient() external {} + function testNonPayableRecipient() external pure {}packages/contracts-rfq/test/MockERC20.sol (1)
14-15: LGTM: Well-documented empty test functionThe addition of the empty
testMockERC20()function, along with its explanatory comment, effectively achieves the goal of excluding this contract from the coverage report. The function is correctly marked asexternal.A minor suggestion to improve the comment:
Consider updating the comment to be more specific:
-/// @notice We include an empty "test" function so that this contract does not appear in the coverage report. +/// @notice This empty test function prevents the contract from appearing in the coverage report.This slight rewording makes the comment more concise and direct.
packages/contracts-rfq/test/mocks/RecipientMock.sol (2)
12-13: LGTM: Empty function added to exclude from coverageThe addition of the
testRecipientMock()function effectively achieves the goal of excluding this test contract from coverage reports. The naming and visibility are appropriate.Consider slightly modifying the comment to be more explicit:
- /// @notice We include an empty "test" function so that this contract does not appear in the coverage report. + /// @notice This empty function ensures that this test contract is excluded from the coverage report.
Line range hint
1-20: Overall: Changes effectively exclude test contract from coverageThe additions successfully achieve the PR objective of removing this test contract from coverage reports without altering its functionality. This approach is clean and doesn't introduce any risks.
To further emphasize the test-only nature of this contract, consider adding a
@devtag to the contract-level documentation:// solhint-disable no-empty-blocks /// @notice Recipient mock for testing purposes. DO NOT USE IN PRODUCTION. +/// @dev This contract is for testing only and should never be used in a production environment. contract RecipientMock is IFastBridgeRecipient {packages/contracts-rfq/test/mocks/ExcessiveReturnValueRecipient.sol (1)
12-13: LGTM: Appropriate test coverage exclusion techniqueThe addition of the empty
testExcessiveReturnValueRecipientfunction is a good approach to exclude this mock contract from coverage reports. The comment clearly explains its purpose.Consider making the function
publicinstead ofexternalfor consistency with Solidity's style guide for test functions:- function testExcessiveReturnValueRecipient() external {} + function testExcessiveReturnValueRecipient() public {}packages/contracts-rfq/test/FastBridgeV2.GasBench.Dst.Excl.t.sol (1)
6-6: LGTM, but consider more specific linter disabling.The addition of
no-empty-blocksto the linter disable comment is consistent with the new empty test function. However, consider using inline linter disabling for specific functions instead of disabling it for the entire contract to maintain better code quality where possible.packages/contracts-rfq/test/mocks/IncorrectReturnValueRecipient.sol (1)
12-13: LGTM: Appropriate test function added to exclude from coverage.The added
testIncorrectReturnValueRecipientfunction serves its purpose well by excluding this contract from coverage reports. The comment clearly explains its intention.Consider adding a
puremodifier to the function to explicitly indicate that it doesn't read from or modify the contract's state:- function testIncorrectReturnValueRecipient() external {} + function testIncorrectReturnValueRecipient() external pure {}This change would make the function's behavior more explicit and align with Solidity best practices.
packages/contracts-rfq/test/FastBridgeV2.GasBench.Src.ArbitraryCall.t.sol (2)
8-9: LGTM: Empty test function added for coverage exclusion.The addition of this empty test function effectively achieves the PR objective of excluding this test contract from the coverage report. The function name is consistent with the contract name, which is good practice.
Consider adding a TODO comment to remind future developers about the purpose of this empty function, as its intention might not be immediately clear without context.
You could add a TODO comment like this:
// TODO: This empty function is intentional. It exists to exclude this contract from coverage reports. function testFastBridgeV2GasBenchmarkSrcArbitraryCallTest() external {}
Line range hint
11-20: LGTM with suggestions: Overridden createFixturesV2 function.The overridden
createFixturesV2function correctly extends the parent implementation and sets up mock call parameters for various transaction types. This is consistent with the purpose of the test contract.However, consider the following suggestions for improvement:
- Using different mock parameters for each transaction type could provide more comprehensive test coverage.
- Consider extracting the mock parameter generation into a separate function for better readability and reusability.
Here's a potential refactoring to address these suggestions:
function createFixturesV2() public virtual override { super.createFixturesV2(); bridgedTokenTx.callParams = generateMockParams("BridgedToken"); bridgedEthTx.callParams = generateMockParams("BridgedEth"); provenTokenTx.callParams = generateMockParams("ProvenToken"); provenEthTx.callParams = generateMockParams("ProvenEth"); setTokenTestCallParams(bridgedTokenTx.callParams); setEthTestCallParams(bridgedEthTx.callParams); } function generateMockParams(string memory identifier) internal view returns (bytes memory) { return abi.encode(userA, keccak256(abi.encodePacked(identifier))); }This refactoring improves readability and allows for unique parameters for each transaction type.
packages/contracts-rfq/test/FastBridgeV2.Src.ProtocolFees.t.sol (1)
8-9: LGTM: Empty test function serves its intended purpose.The addition of the empty
testFastBridgeV2SrcProtocolFeesTestfunction is an appropriate way to exclude this contract from the coverage report. This change aligns with the PR objective and doesn't modify the contract's behavior or utility.This approach is a common and effective technique for managing test coverage reporting. It allows you to maintain the contract structure while controlling which contracts appear in coverage reports.
packages/contracts-rfq/test/UniversalTokenLibHarness.sol (1)
8-9: Empty test function added to include contract in coverage reportAn empty function
testUniversalTokenLibHarnesshas been added with a comment explaining its purpose. This change aligns with the PR objective of managing test contracts in the coverage report.However, it's worth noting that this approach might not be the most maintainable solution in the long term. Consider exploring alternative methods for managing test coverage, such as configuration files or coverage ignore patterns, which could provide a more scalable approach.
Would you like suggestions for alternative approaches to manage test coverage without adding empty functions?
packages/contracts-rfq/script/FastBridge.s.sol (1)
10-11: Approved: Empty test function added to exclude from coverage.The addition of the empty
testDeployFastBridgefunction aligns with the PR objective to remove test contracts from the coverage report. This approach is acceptable for script contracts.Consider slightly modifying the comment for clarity:
- /// @notice We include an empty "test" function so that this contract does not appear in the coverage report. + /// @notice This empty "test" function prevents this script contract from appearing in the coverage report.packages/contracts-rfq/test/FastBridgeV2.GasBench.Dst.ArbitraryCall.t.sol (2)
15-16: LGTM: Empty test function serves its purpose.The addition of the empty test function
testFastBridgeV2GasBenchmarkDstArbitraryCallTesteffectively achieves the goal of excluding this contract from the coverage report. This aligns with the PR's objective.Consider slightly modifying the comment for clarity:
- /// @notice We include an empty "test" function so that this contract does not appear in the coverage report. + /// @dev This empty test function ensures that this contract is not included in the coverage report.This change emphasizes that it's a development-specific comment and slightly improves the wording.
Line range hint
18-31: LGTM: Setup and fixture creation look good.The modifications to
setUpandcreateFixturesV2functions appropriately set up the test environment for gas benchmarking. The use of a mock recipient aligns well with the goal of measuring arbitrary call overhead.For consistency with Solidity style guidelines, consider adding the
overridekeyword to thecreateFixturesV2function:- function createFixturesV2() public virtual override { + function createFixturesV2() public virtual override {This change maintains consistency with the
setUpfunction declaration.packages/contracts-rfq/test/FastBridgeV2.GasBench.Src.PFees.t.sol (2)
Line range hint
11-15: LGTM:configureFastBridgefunction looks good.The modifications to the
configureFastBridgefunction are appropriate for testing protocol fees. It correctly calls the parent implementation, grants the necessary role, and sets a reasonable fee rate for testing.Consider extracting the fee rate (1e4) to a constant for better readability and easier modification in future tests. For example:
uint256 private constant PROTOCOL_FEE_RATE = 1e4; // 1% function configureFastBridge() public virtual override { // ... existing code ... fastBridge.setProtocolFeeRate(PROTOCOL_FEE_RATE); }
Line range hint
17-38: LGTM:createFixturesfunction is well-structured.The
createFixturesfunction is appropriately modified to set up test scenarios with protocol fees. It correctly initializes transaction amounts, fee amounts, and nonces for both token and ETH transactions.To improve clarity, consider adding a brief comment explaining the relationship between the different transaction amounts. For example:
function createFixtures() public virtual override { super.createFixtures(); // Set up token transaction with 1% fee tokenTx.originFeeAmount = 0.01e6; // 1% of 1e6 tokenTx.originAmount = 0.99e6; // Original amount minus fee tokenTx.destAmount = 0.98e6; // Destination amount (may include additional fees) tokenParams.destAmount = 0.98e6; // Set up ETH transaction with 1% fee ethTx.originFeeAmount = 0.01 ether; // 1% of 1 ether ethTx.originAmount = 0.99 ether; // Original amount minus fee ethTx.destAmount = 0.98 ether; // Destination amount (may include additional fees) ethParams.destAmount = 0.98 ether; // ... rest of the function ... }This addition would make it easier for other developers to understand the relationship between the various amounts and the applied fees.
packages/contracts-rfq/test/FastBridgeV2.t.sol (1)
47-49: Consider alternative methods to exclude from coverage reports.The added empty function serves the purpose of preventing this contract from appearing in the coverage report. While this achieves the desired outcome, it introduces code that doesn't contribute to the contract's functionality or tests.
Have you considered alternative methods to exclude this contract from coverage reports? Some testing frameworks offer configuration options to exclude specific files or contracts without modifying the code. This approach would align better with the previous feedback about not unnecessarily modifying test utility contracts.
If no better alternatives exist, please add a more detailed comment explaining why this approach was chosen over other potential solutions.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (17)
- packages/contracts-rfq/script/FastBridge.s.sol (1 hunks)
- packages/contracts-rfq/test/FastBridgeMock.sol (1 hunks)
- packages/contracts-rfq/test/FastBridgeV2.Dst.Base.t.sol (1 hunks)
- packages/contracts-rfq/test/FastBridgeV2.GasBench.Dst.ArbitraryCall.t.sol (1 hunks)
- packages/contracts-rfq/test/FastBridgeV2.GasBench.Dst.Excl.t.sol (1 hunks)
- packages/contracts-rfq/test/FastBridgeV2.GasBench.Src.ArbitraryCall.t.sol (1 hunks)
- packages/contracts-rfq/test/FastBridgeV2.GasBench.Src.PFees.t.sol (1 hunks)
- packages/contracts-rfq/test/FastBridgeV2.Src.ProtocolFees.t.sol (1 hunks)
- packages/contracts-rfq/test/FastBridgeV2.t.sol (1 hunks)
- packages/contracts-rfq/test/MockERC20.sol (1 hunks)
- packages/contracts-rfq/test/UniversalTokenLibHarness.sol (1 hunks)
- packages/contracts-rfq/test/harnesses/MulticallTargetHarness.sol (2 hunks)
- packages/contracts-rfq/test/mocks/ExcessiveReturnValueRecipient.sol (1 hunks)
- packages/contracts-rfq/test/mocks/IncorrectReturnValueRecipient.sol (1 hunks)
- packages/contracts-rfq/test/mocks/NoReturnValueRecipient.sol (1 hunks)
- packages/contracts-rfq/test/mocks/NonPayableRecipient.sol (1 hunks)
- packages/contracts-rfq/test/mocks/RecipientMock.sol (1 hunks)
🧰 Additional context used
📓 Learnings (9)
packages/contracts-rfq/script/FastBridge.s.sol (1)
Learnt from: ChiTimesChi PR: synapsecns/sanguine#3284 File: packages/contracts-rfq/test/FastBridgeV2.t.sol:53-56 Timestamp: 2024-10-14T13:24:45.293Z Learning: Modifying test utility contracts in the `test` directory is unnecessary and would only pollute the code.packages/contracts-rfq/test/FastBridgeMock.sol (1)
Learnt from: ChiTimesChi PR: synapsecns/sanguine#3284 File: packages/contracts-rfq/test/FastBridgeV2.t.sol:53-56 Timestamp: 2024-10-14T13:24:45.293Z Learning: Modifying test utility contracts in the `test` directory is unnecessary and would only pollute the code.packages/contracts-rfq/test/FastBridgeV2.Dst.Base.t.sol (1)
Learnt from: ChiTimesChi PR: synapsecns/sanguine#3284 File: packages/contracts-rfq/test/FastBridgeV2.t.sol:53-56 Timestamp: 2024-10-14T13:24:45.293Z Learning: Modifying test utility contracts in the `test` directory is unnecessary and would only pollute the code.packages/contracts-rfq/test/FastBridgeV2.GasBench.Dst.ArbitraryCall.t.sol (1)
Learnt from: ChiTimesChi PR: synapsecns/sanguine#3284 File: packages/contracts-rfq/test/FastBridgeV2.t.sol:53-56 Timestamp: 2024-10-14T13:24:45.293Z Learning: Modifying test utility contracts in the `test` directory is unnecessary and would only pollute the code.packages/contracts-rfq/test/FastBridgeV2.GasBench.Dst.Excl.t.sol (1)
Learnt from: ChiTimesChi PR: synapsecns/sanguine#3284 File: packages/contracts-rfq/test/FastBridgeV2.t.sol:53-56 Timestamp: 2024-10-14T13:24:45.293Z Learning: Modifying test utility contracts in the `test` directory is unnecessary and would only pollute the code.packages/contracts-rfq/test/FastBridgeV2.GasBench.Src.ArbitraryCall.t.sol (1)
Learnt from: ChiTimesChi PR: synapsecns/sanguine#3284 File: packages/contracts-rfq/test/FastBridgeV2.t.sol:53-56 Timestamp: 2024-10-14T13:24:45.293Z Learning: Modifying test utility contracts in the `test` directory is unnecessary and would only pollute the code.packages/contracts-rfq/test/FastBridgeV2.GasBench.Src.PFees.t.sol (2)
Learnt from: ChiTimesChi PR: synapsecns/sanguine#3284 File: packages/contracts-rfq/test/FastBridgeV2.t.sol:53-56 Timestamp: 2024-10-14T13:24:45.293Z Learning: Modifying test utility contracts in the `test` directory is unnecessary and would only pollute the code.Learnt from: ChiTimesChi PR: synapsecns/sanguine#3284 File: packages/contracts-rfq/test/FastBridgeV2.t.sol:53-56 Timestamp: 2024-10-14T13:24:45.293Z Learning: In Solidity test contracts under the `test` directory, such as `FastBridgeV2.t.sol`, contracts are not deployed, so gas consumption is not a concern.packages/contracts-rfq/test/FastBridgeV2.Src.ProtocolFees.t.sol (1)
Learnt from: ChiTimesChi PR: synapsecns/sanguine#3284 File: packages/contracts-rfq/test/FastBridgeV2.t.sol:53-56 Timestamp: 2024-10-14T13:24:45.293Z Learning: Modifying test utility contracts in the `test` directory is unnecessary and would only pollute the code.packages/contracts-rfq/test/FastBridgeV2.t.sol (3)
Learnt from: ChiTimesChi PR: synapsecns/sanguine#3284 File: packages/contracts-rfq/test/FastBridgeV2.t.sol:53-56 Timestamp: 2024-10-14T13:24:45.293Z Learning: Modifying test utility contracts in the `test` directory is unnecessary and would only pollute the code.Learnt from: ChiTimesChi PR: synapsecns/sanguine#3284 File: packages/contracts-rfq/test/FastBridgeV2.t.sol:53-56 Timestamp: 2024-10-14T13:24:45.293Z Learning: In Solidity test contracts under the `test` directory, such as `FastBridgeV2.t.sol`, contracts are not deployed, so gas consumption is not a concern.Learnt from: ChiTimesChi PR: synapsecns/sanguine#3284 File: packages/contracts-rfq/test/FastBridgeV2.Src.t.sol:919-993 Timestamp: 2024-10-14T14:48:01.520Z Learning: In Solidity test files for FastBridgeV2 (e.g., `packages/contracts-rfq/test/FastBridgeV2.Src.t.sol`), code duplication in test functions is acceptable to maintain readability and maintainability, even if it contradicts DRY principles.
🔇 Additional comments (23)
packages/contracts-rfq/test/mocks/NonPayableRecipient.sol (2)
4-4: LGTM: Appropriate use of Solhint directiveThe added Solhint directive to disable empty block warnings is appropriate, considering the new empty function being introduced. This helps maintain clean code without unnecessary warnings.
Line range hint
1-15: Summary: Changes align with PR objectives without altering contract behaviorThe modifications to this contract successfully achieve the goal of including it in coverage reports without changing its core functionality. The added empty function and Solhint directive are appropriate for a test mock contract.
Some key points:
- The existing
fastBridgeTransferReceivedfunction remains unchanged, preserving the contract's original behavior.- The comment clearly states that this is a mock contract for testing purposes and should not be used in production.
- These changes contribute to more accurate test coverage reporting without introducing any risks.
packages/contracts-rfq/test/mocks/NoReturnValueRecipient.sol (2)
4-4: LGTM: Solhint directive update is appropriate.The update to the solhint directive to disable the rule against empty blocks is consistent with the addition of an empty test function. This is a good practice when intentionally including empty blocks in the code.
10-11: LGTM: New function addition aligns with PR objectives.The addition of the
testNoReturnValueRecipientfunction is in line with the PR objective of removing test contracts from the coverage report. The function is correctly implemented as an empty external function, and its name is descriptive and follows the contract name convention.packages/contracts-rfq/test/MockERC20.sol (1)
6-6: LGTM: Appropriate use of solhint disable commentThe solhint disable comment is correctly placed and specifically targets the
no-empty-blocksrule. This is a good practice to suppress warnings for the intentional empty function that follows.packages/contracts-rfq/test/mocks/RecipientMock.sol (1)
6-6: LGTM: Solhint directive added to allow empty blocksThe addition of the
// solhint-disable no-empty-blocksdirective is appropriate here. It prevents linting errors for the intentionally empty function that follows, which is used to exclude this test contract from coverage reports.packages/contracts-rfq/test/mocks/ExcessiveReturnValueRecipient.sol (2)
6-6: LGTM: Appropriate use of linter directiveThe added solhint directive to disable the no-empty-blocks rule is appropriate here, as we're intentionally adding an empty function for test coverage purposes.
6-13: Summary: Appropriate changes for test coverage exclusionThe changes made to this file effectively exclude the
ExcessiveReturnValueRecipientmock contract from coverage reports without altering its functionality. This aligns well with the PR objectives.Key points:
- The added solhint directive is appropriate for the intentional use of an empty block.
- The new empty test function serves its purpose of excluding the contract from coverage.
These changes improve the accuracy of test coverage metrics by excluding mock contracts designed for testing purposes.
packages/contracts-rfq/test/FastBridgeV2.GasBench.Dst.Excl.t.sol (3)
8-11: LGTM. Changes align with the PR objective.The addition of the
EXCLUSIVITY_PERIODconstant and the empty test functiontestFastBridgeV2GasBenchmarkDstExclusivityTestserve clear purposes:
- The constant is used in other functions to set up the test environment.
- The empty test function prevents this contract from appearing in the coverage report, which aligns with the PR objective of removing test contracts from the coverage report.
These changes are appropriate and don't unnecessarily modify the test utility contract.
Line range hint
13-16: LGTM. setUp function appropriately modified for test scenario.The
setUpfunction has been correctly overridden to call the parent implementation and then skip time by half of theEXCLUSIVITY_PERIOD. This modification sets up a specific test scenario where half of the exclusivity period has passed, which is consistent with the purpose of this test contract.
Line range hint
18-22: LGTM. createFixturesV2 function properly sets up exclusivity test environment.The
createFixturesV2function has been appropriately overridden to call the parent implementation and then set token and ETH test exclusivity parameters using theEXCLUSIVITY_PERIOD. These modifications are consistent with setting up the test environment for exclusivity testing, which aligns with the purpose of this test contract.packages/contracts-rfq/test/mocks/IncorrectReturnValueRecipient.sol (2)
6-6: LGTM: Appropriate solhint directive added.The added solhint directive to disable the no-empty-blocks rule is appropriate, given the new empty function being introduced. This prevents unnecessary linting warnings while maintaining the intended testing functionality.
6-13: LGTM: Changes successfully exclude the contract from coverage reports.The modifications to this file effectively achieve the PR's objective of removing test contracts from coverage reports. The added empty function and accompanying comment serve their purpose without affecting the core functionality or security of the contract. These changes will lead to more accurate test coverage reports by excluding mock contracts designed for testing purposes.
packages/contracts-rfq/test/FastBridgeV2.GasBench.Src.ArbitraryCall.t.sol (1)
6-6: LGTM: Appropriate solhint directive update.The solhint directive has been correctly updated to include 'no-empty-blocks', which is consistent with the addition of an intentionally empty test function. This is a good practice to avoid unnecessary linter warnings.
packages/contracts-rfq/test/FastBridgeV2.Src.ProtocolFees.t.sol (1)
6-6: LGTM: Updated comment directive is appropriate.The updated comment directive to disable checks for mixed-case function names and empty blocks is appropriate, considering the intentional addition of an empty test function. This change helps prevent unnecessary linting warnings in the test contract.
packages/contracts-rfq/test/UniversalTokenLibHarness.sol (1)
6-6: Linter directive updated to accommodate new empty functionThe Solidity linter directive has been expanded to disable the
no-empty-blockscheck in addition to the existingorderingcheck. This change is appropriate given the introduction of the emptytestUniversalTokenLibHarnessfunction.packages/contracts-rfq/script/FastBridge.s.sol (1)
Line range hint
1-35: LGTM: No impact on existing functionality.The addition of the
testDeployFastBridgefunction doesn't affect the existingrunfunction or any other part of the contract. The core functionality of deploying theFastBridgecontract, managing roles, and handling ownership remains intact.packages/contracts-rfq/test/harnesses/MulticallTargetHarness.sol (2)
6-6: LGTM: Appropriate use of linter directiveThe addition of the
// solhint-disable no-empty-blocksdirective is a good practice. It prevents unnecessary linter warnings for the intentionally empty function that follows.
15-16: LGTM: Effective implementation for coverage exclusionThe addition of the empty
testMulticallTargetHarnessfunction, along with its explanatory comment, is an effective way to exclude this contract from the coverage report. This aligns well with the PR objective of removing test contracts from coverage reporting.packages/contracts-rfq/test/FastBridgeV2.GasBench.Dst.ArbitraryCall.t.sol (1)
7-7: LGTM: Solhint directive update is appropriate.The modification to disable the
no-empty-blocksrule is necessary to accommodate the intentionally empty test function. This change aligns with the PR's objective of removing test contracts from the coverage report.packages/contracts-rfq/test/FastBridgeV2.GasBench.Src.PFees.t.sol (1)
6-9: LGTM: New test contract and empty test function.The new contract
FastBridgeV2GasBenchmarkSrcProtocolFeesTestand the empty test function are well-structured and follow good practices for Solidity test contracts. The empty test function effectively prevents this contract from appearing in the coverage report, which is the intended behavior for specialized test cases.packages/contracts-rfq/test/FastBridgeV2.Dst.Base.t.sol (2)
6-6: LGTM: Updated linter directive is appropriate.The addition of
no-empty-blocksto the linter directive is consistent with the introduction of the empty test function. This change is acceptable in the context of test contracts.
10-11: LGTM: Empty test function serves its purpose.The addition of the empty
testFastBridgeV2DstBaseTestfunction is an appropriate way to exclude this test contract from the coverage report. This change aligns with the PR objective and is consistent with previous feedback about not unnecessarily modifying test utility contracts.
| /// @notice We include an empty "test" function so that this contract does not appear in the coverage report. | ||
| function testFastBridgeMock() external {} | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider alternative approaches to exclude test contracts from coverage.
The added testFastBridgeMock() function serves its purpose of preventing this contract from appearing in the coverage report. However, modifying test utility contracts in the test directory might be unnecessary and could potentially pollute the code.
Instead of adding empty functions to each test contract, consider exploring alternative methods to exclude test contracts from coverage reports:
- Configure your coverage tool to ignore specific directories or files.
- Use naming conventions or file extensions that your coverage tool can recognize and exclude.
- If using solidity-coverage, you can add a
.solcover.jsconfiguration file to exclude specific contracts or directories.
These approaches would achieve the same goal without modifying the test contracts themselves.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3291 +/- ##
============================================
Coverage ? 91.48936%
============================================
Files ? 59
Lines ? 1269
Branches ? 158
============================================
Hits ? 1161
Misses ? 104
Partials ? 4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Changes to gas cost
🧾 Summary (50% most significant diffs)
Full diff report 👇
|
Deploying sanguine-fe with
|
| Latest commit: |
0799a1d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://49b8bb1c.sanguine-fe.pages.dev |
| Branch Preview URL: | https://test-fast-bridge-coverage.sanguine-fe.pages.dev |
Description
A clear and concise description of the features you're adding in this pull request.
Additional context
Add any other context about the problem you're solving.
Metadata
Summary by CodeRabbit
New Features
Bug Fixes
Documentation