-
Notifications
You must be signed in to change notification settings - Fork 202
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
chore: scheduled basic e2e tests for evm testnet endpoint #2106
Conversation
Caution Review failedThe pull request is closed. WalkthroughThe changes in this pull request involve updates to the Changes
Possibly related PRs
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
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (5)
evm-e2e/justfile (2)
19-21
: Consider enhancing the recipe documentation.While the current comment is clear, it would be helpful to add more context about when and how this recipe should be used, especially in relation to the automated workflow.
Consider expanding the comment like this:
-# Runs tx receipt tests. Used for testnet quick check. +# Runs transaction receipt tests only. Used for quick health checks in CI/CD pipeline +# and scheduled testnet verification. For full test suite, use `just test` instead.
21-21
: Consider adding error handling for missing test file.The npm test command might fail silently if the test file is not found. Consider adding a check for file existence.
test-basic: - npm test -- tx_receipt.test.ts + #!/usr/bin/env bash + if [ ! -f "test/tx_receipt.test.ts" ]; then + echo "Error: test/tx_receipt.test.ts not found" + exit 1 + fi + npm test -- tx_receipt.test.ts.github/workflows/e2e-evm-basic-testnet.yml (2)
3-6
: Consider adding manual workflow dispatch trigger.While hourly scheduling is good for automated testing, adding manual trigger capability would be beneficial for debugging purposes.
on: schedule: - cron: "0 * * * *" # every hour at 00 min + workflow_dispatch: # Allows manual trigger from GitHub UI
24-33
: Enhance Slack notification with more context.The current notification could be more helpful by including test summary and failure details.
- name: Send failure to slack channel if: always() uses: ravsamhq/notify-slack-action@v2 with: status: ${{ job.status }} notify_when: "failure" notification_title: "EVM basic tests failed on Testnet" - message_format: "{emoji} *{workflow}* {status_message} Run: {run_url}" + message_format: | + {emoji} *{workflow}* {status_message} + *Commit:* {commit_url} + *Author:* {author} + *Run:* {run_url} + *Changes:* {pr_url} env: SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_TESTNET }}evm-e2e/test/tx_receipt.test.ts (1)
13-13
: LGTM! Good practice to use minimal amounts in automated tests.Reducing the test transaction value from 1 ETH to 0.0001 ETH is a good optimization for scheduled e2e tests. This change:
- Preserves test account funds for longer test runs
- Reduces the cost of running hourly automated tests
- Still effectively tests the transaction receipt functionality
The test coverage and assertions remain unchanged, maintaining the same level of verification.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
.github/workflows/e2e-evm-basic-testnet.yml
(1 hunks)CHANGELOG.md
(1 hunks)evm-e2e/justfile
(1 hunks)evm-e2e/test/tx_receipt.test.ts
(1 hunks)
🔇 Additional comments (4)
evm-e2e/justfile (1)
19-21
: LGTM! Clear and focused implementation.
The new recipe follows the existing pattern and serves its purpose well for the automated testnet checks.
Let's verify the test file exists and is properly referenced:
✅ Verification successful
Test file exists and is correctly referenced ✅
The tx_receipt.test.ts
file exists at the expected location evm-e2e/test/tx_receipt.test.ts
, confirming that the test-basic
recipe in the justfile correctly references an existing test file.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence of tx_receipt.test.ts
# Expected: File should exist in the test directory
fd -t f "tx_receipt.test.ts" evm-e2e/
Length of output: 69
.github/workflows/e2e-evm-basic-testnet.yml (1)
1-33
: Verify workflow uniqueness and potential conflicts.
Let's ensure this workflow doesn't overlap with existing e2e test workflows.
✅ Verification successful
No workflow conflicts found - distinct purposes
The two EVM-related workflows serve different purposes and complement each other:
e2e-evm.yml
: Runs on PR events for code changes, using a local test environmente2e-evm-basic-testnet.yml
: Runs hourly scheduled tests against testnet, with Slack notifications for failures
The workflows use different test commands (test
vs test-basic
) and different environments (local vs testnet), indicating they are designed to work together rather than conflict.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for similar e2e test workflows
echo "Searching for similar e2e test workflows..."
fd -e yml -e yaml . .github/workflows/ --exec grep -l "evm.*test" {} \;
Length of output: 217
Script:
#!/bin/bash
# Let's do a broader search for EVM-related workflows and their content
echo "=== EVM-related workflow files ==="
fd -e yml -e yaml . .github/workflows/ --exec grep -l "evm" {} \;
echo -e "\n=== Content of EVM-related workflows ==="
for file in $(fd -e yml -e yaml . .github/workflows/ --exec grep -l "evm" {} \;); do
echo -e "\n--- $file ---"
cat "$file"
done
echo -e "\n=== Checking for any e2e test patterns ==="
rg -i "e2e|test" .github/workflows/
Length of output: 11233
CHANGELOG.md (2)
103-103
: LGTM! Changelog entry follows guidelines.
The changelog entry for PR #2106 is properly documented under the "Unreleased" section and follows the changelog guiding principles.
Line range hint 1-108
: LGTM! Changelog follows best practices.
The changelog is well-structured and follows best practices:
- Uses Keep a Changelog format
- Follows semantic versioning
- Groups changes by type
- Includes PR links and descriptions
- Latest changes at the top
e2e-evm: | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v4 | ||
|
||
- name: "just install" | ||
run: just install | ||
working-directory: "evm-e2e" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Implement dependency caching to improve workflow execution time.
Adding caching for dependencies can significantly reduce the workflow execution time and load on GitHub's infrastructure.
e2e-evm:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
+
+ - uses: actions/cache@v3
+ with:
+ path: |
+ evm-e2e/.just
+ evm-e2e/node_modules
+ key: ${{ runner.os }}-evm-e2e-${{ hashFiles('evm-e2e/package-lock.json') }}
+ restore-keys: |
+ ${{ runner.os }}-evm-e2e-
- name: "just install"
run: just install
working-directory: "evm-e2e"
Committable suggestion skipped: line range outside the PR's diff.
- name: "Run tests (just test-basic)" | ||
run: just test-basic | ||
working-directory: "evm-e2e" | ||
env: | ||
JSON_RPC_ENDPOINT: https://evm-rpc.testnet-1.nibiru.fi | ||
MNEMONIC: ${{ secrets.WALLET_MNEMONIC_TESTNET }} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enhance test execution reliability and maintainability.
Several improvements could make the test execution more robust:
- The RPC endpoint should be configurable via secrets
- Add timeout to prevent hanging tests
- Implement retry mechanism for transient failures
- name: "Run tests (just test-basic)"
+ timeout-minutes: 10
+ uses: nick-fields/retry@v2
+ with:
+ timeout_minutes: 10
+ max_attempts: 3
+ command: cd evm-e2e && just test-basic
- run: just test-basic
- working-directory: "evm-e2e"
env:
- JSON_RPC_ENDPOINT: https://evm-rpc.testnet-1.nibiru.fi
+ JSON_RPC_ENDPOINT: ${{ secrets.TESTNET_RPC_ENDPOINT }}
MNEMONIC: ${{ secrets.WALLET_MNEMONIC_TESTNET }}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
- name: "Run tests (just test-basic)" | |
run: just test-basic | |
working-directory: "evm-e2e" | |
env: | |
JSON_RPC_ENDPOINT: https://evm-rpc.testnet-1.nibiru.fi | |
MNEMONIC: ${{ secrets.WALLET_MNEMONIC_TESTNET }} | |
- name: "Run tests (just test-basic)" | |
timeout-minutes: 10 | |
uses: nick-fields/retry@v2 | |
with: | |
timeout_minutes: 10 | |
max_attempts: 3 | |
command: cd evm-e2e && just test-basic | |
env: | |
JSON_RPC_ENDPOINT: ${{ secrets.TESTNET_RPC_ENDPOINT }} | |
MNEMONIC: ${{ secrets.WALLET_MNEMONIC_TESTNET }} |
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests
Documentation