fix: failing CI tests #341
Conversation
🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughThe updates involve modifying GitHub Actions workflow files to use a newer version of the Changes
Sequence Diagram(s)sequenceDiagram
participant TestContract
participant TargetContract
participant VM
TestContract->>VM: expectRevert("Error message")
TestContract->>TargetContract: Call function expected to revert
TargetContract-->>VM: Revert with "Error message"
VM-->>TestContract: Test passes if revert matches
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/coveralls.yaml(1 hunks).github/workflows/test.yaml(2 hunks)script/integration/InspectIntegration.s.sol(1 hunks)test/messaging/AuthVerifier.t.sol(1 hunks)test/messaging/GasFeePricing.t.sol(2 hunks)test/messaging/MessageBusSender.t.sol(1 hunks)test/router/libs/UniversalTokenLib.t.sol(0 hunks)
💤 Files with no reviewable changes (1)
- test/router/libs/UniversalTokenLib.t.sol
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: hardhat-test
- GitHub Check: foundry-integration
- GitHub Check: foundry-base
- GitHub Check: Build
- GitHub Check: test
- GitHub Check: foundry-base
- GitHub Check: hardhat-test
- GitHub Check: Build
- GitHub Check: foundry-integration
- GitHub Check: test
🔇 Additional comments (8)
script/integration/InspectIntegration.s.sol (1)
6-6: LGTM! Modernized logging import and usage.The update from
consoletoconsole2aligns with forge-std best practices and likely addresses compatibility issues with newer versions of the testing framework.Also applies to: 12-12
test/messaging/AuthVerifier.t.sol (1)
23-27: Excellent improvement to test explicitness and readability.The change from the
testFailprefix to explicitvm.expectRevert()with the specific error message makes the test more precise and self-documenting. The revert message correctly matches OpenZeppelin's Ownable contract behavior.test/messaging/GasFeePricing.t.sol (2)
28-32: Great improvement to test clarity and precision.The explicit
vm.expectRevert()with the specific Ownable error message makes this test more reliable and self-documenting compared to the previoustestFailapproach.
89-95: Improved test explicitness with correct error message.The explicit revert expectation for the empty destination native address scenario is much clearer than the previous implicit approach. The error message "dstNativeAddress empty" correctly matches the expected validation failure.
test/messaging/MessageBusSender.t.sol (4)
53-56: Improved test precision for unset fee scenario.The explicit revert expectation with "Fee not set" message is much clearer than the previous
testFailapproach and correctly validates the fee estimation behavior.
58-63: Better test clarity for wrong chain ID scenario.The explicit revert expectation correctly captures the expected behavior when attempting to send a message to an unsupported chain ID.
66-72: Enhanced test explicitness for insufficient fee validation.The explicit revert expectation with "Insuffient gas fee" message clearly documents the expected behavior when insufficient fees are provided. The test logic correctly uses
estimatedFee - 1to trigger the insufficient fee condition.
75-82: Comprehensive test improvement with dual revert expectations.Excellent enhancement that explicitly tests both the
estimateFeeandsendMessagecalls should revert with "Fee not set" when fees are unset for the destination chain. The dualvm.expectRevert()calls correctly validate both failure points.
Pull Request Test Coverage Report for Build 15540353210Details
💛 - Coveralls |
Description
Isolated test fixes from #334
Summary by CodeRabbit