test: add e2e consensus test #3126
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces enhancements to the GitHub Actions workflow for end-to-end testing, specifically in the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
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
|
gartnera
force-pushed
the
e2e-consensus-test
branch
from
fb9a3e0 to
bf6c2f0
Compare
gartnera
requested review from
a team,
brewmaster012,
kingpinXD,
lumtis,
ws4charlie,
skosito,
swift1337 and
fbac
as code owners
![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 (3)
contrib/localnet/docker-compose.yml (1)
71-71: Consider documenting platform configurationThe platform configuration enables cross-architecture testing, which is valuable for the reported arm64 macOS testing scenario. However, consider documenting the supported platforms in the file's header comments to assist other developers.
Add documentation like:
# - ZetaChain with 2 nodes (zetacore0, zetacore1). When profile set to stress, 4 nodes will be created. +# Note: zetacore1 supports custom platform configuration via ZETACORE1_PLATFORM for cross-architecture testing.github/workflows/e2e.yml (1)
132-141: Fix indentation in getPrLabels functionThe function implementation is solid, but the indentation is inconsistent with the rest of the file.
Apply this diff to fix the indentation:
const getPrLabels = async (pull_number) => { const { data: pr } = await github.rest.pulls.get({ - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: pull_number, - }); - const labels = pr.labels.map(label => label.name); - console.log(`labels for ${pull_number}:`, labels); - return labels; + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: pull_number, +}); +const labels = pr.labels.map(label => label.name); +console.log(`labels for ${pull_number}:`, labels); +return labels; }Makefile (1)
289-294: Consider adding documentation for the consensus test target.The implementation looks correct, but it would be beneficial to add a comment explaining:
- The purpose of using the development version of zetacore
- The significance of the platform specification
- Any prerequisites or requirements for running the test
Apply this diff to add documentation:
+# start-e2e-consensus-test initiates a local network using the development version of zetacored. +# This target is designed for testing consensus-breaking changes and requires specific PR labels: +# - CONSENSUS_TESTS: Required to opt-in for consensus tests +# - CONSENSUS_BREAKING_ACK: Required if there are breaking changes start-e2e-consensus-test: e2e-images
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
.github/workflows/e2e.yml(4 hunks)Makefile(2 hunks)contrib/localnet/docker-compose.yml(1 hunks)
🔇 Additional comments (6)
contrib/localnet/docker-compose.yml (2)
70-70: LGTM: Dynamic image configuration enables consensus testing
The environment variable ${ZETACORE1_IMAGE-zetanode:latest} with fallback aligns with the PR's objective to support consensus testing with development versions of zetacored.
70-71: Verify configuration consistency with orchestrator
The changes enable dynamic configuration of zetacore1, but we should verify that the orchestrator service properly handles these configurations during consensus tests.
.github/workflows/e2e.yml (2)
114-114: LGTM: Output variable properly declared
The CONSENSUS_TESTS output variable follows the established pattern and is correctly integrated with other test flags.
228-230: LGTM: Matrix configuration properly structured
The consensus test matrix entry is well-configured, following the established pattern and using appropriate runner specifications.
Makefile (2)
271-271: LGTM!
The formatting change improves consistency with other similar commands in the Makefile.
291-292: Verify the image and platform compatibility.
The target uses a hardcoded platform and image. Let's verify that these values are consistently used across the codebase.
✅ Verification successful
Platform and image specifications are consistent with the development environment
The verification reveals that:
- The
ZETACORE1_IMAGEandZETACORE1_PLATFORMvariables are used correctly in the docker-compose setup with appropriate fallback values - The platform specification
linux/amd64is consistently used across various services that require explicit platform definition - The image versioning follows a consistent pattern using either
latest,develop, oroldtags depending on the context
The hardcoded values in the Makefile align with the broader development environment configuration and are appropriate for their intended use.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for other occurrences of the development image and platform specifications
# Test 1: Search for other references to the development image
echo "Checking for development image references:"
rg "ghcr.io/zeta-chain/zetanode:develop"
# Test 2: Search for platform specifications
echo "Checking for platform specifications:"
rg "ZETACORE.*_PLATFORM"
Length of output: 447
Script:
#!/bin/bash
# Let's check the docker-compose file to understand the context better
echo "Checking docker-compose.yml content:"
cat contrib/localnet/docker-compose.yml
# Let's also check for any other docker platform specifications
echo -e "\nChecking for other platform specifications:"
rg "platform.*linux"
# Check for other image references in docker compose
echo -e "\nChecking for other image references in docker compose:"
rg "image:.*zetanode"
Length of output: 13349
gartnera
force-pushed
the
e2e-consensus-test
branch
from
8d416cb to
be4ad2a
Compare
This check only works on develop PRs atm. I'd need to push release docker images and turn on mergequeue for release PRs for it to work there. |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Description
Add
start-e2e-consensus-testwhich starts up localnet with the develop version of zetacored pushed in #3121.Opt in on your PR with the
CONSENSUS_TESTSlabel. This will generally require your PR to be up to date with develop.To acknowledge a breaking change, you must add the
CONSENSUS_BREAKING_ACKlabel to your PR. If your PR is consensus breaking, you should also have an explanation point in your PR title (feat!,fix!, etc).If
CONSENSUS_BREAKING_ACKis not applied to your PR, the consensus tests will be run on the mergequeue automatically. The mergequeue is the best place to run this test as the head branch is guaranteed to be one commit above develop (at least with our current config).e2e will still point at zetacore0 so any rpc only changes should not result in failure.
Also add block production monitor to
zetae2e localwhich will halt the e2e tests quickly if blocks are not being produced.Demo in #3129
Closes #3120
How Has This Been Tested?
make start-e2e-consensus-teston arm64 macosSummary by CodeRabbit
New Features
Improvements
zetacore1service in the Docker Compose configuration for improved configurability with dynamic image and platform settings.Bug Fixes