-
Notifications
You must be signed in to change notification settings - Fork 13
Conversation
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.
lgtm!! very nice @rebelArtists! 🔥
uses: docker/setup-buildx-action@v1 | ||
- name: Clone and build agglayer image locally | ||
run: | | ||
git clone https://github.com/0xPolygon/agglayer.git |
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.
Is this really needed if the current repo is already agglayer?
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.
Is this really needed if the current repo is already agglayer?
good call! and I think we should build the Docker image using Dockerfile.release
.
https://github.com/0xPolygon/kurtosis-cdk/blob/main/.github/workflows/regression-tests.yml#L52-L56
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.
fixed, good call
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.
Really neat 🎩, I find this super lightweight, I think is worth having this here, test e2e in the repo itself.
run: | | ||
git clone https://github.com/0xPolygon/kurtosis-cdk.git | ||
cd kurtosis-cdk | ||
git checkout dan/jit_containers |
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.
Are we going to merge this?
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.
Yup it's not needed anymore. We would just need to install yq
and replace the zkevm_agglayer_image
value by the name of the docker image (e.g. agglayer:local
).
For reference: https://github.com/0xPolygon/kurtosis-cdk/blob/main/.github/workflows/regression-tests.yml#L102-L115
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.
fixed!
- name: Deploy CDK devnet on local github runner | ||
run: | | ||
cd kurtosis-cdk | ||
kurtosis run --enclave cdk-v1 --args-file params.yml . |
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.
Should we use a local params file instead of relying on the version matrix in the kurtosis repo?
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.
This file defines the configuration of the entire CDK stack, not just the agglayer. In the context of e2e tests, I think it's interesting to use the kurtosis-cdk configuration file. It allows you to test the agglayer in contact with the latest versions of the other components (because we maintain the tool and we're iterating fast at the moment) and, what's more, you have one less file to maintain. But up to you :)
Quality Gate passedIssues Measures |
purpose:
tests:
https://github.com/0xPolygon/agglayer/actions/runs/8710918215/job/23893875401
all cdk containers deployed and running via local github action runner (minimize cloud bill as deployment entirely ephemeral)
query verifiedBatchNumber as e2e proof network is functioning as expected:
visualize regression results directly within PR:
we hope this provides your team with a strong foundational check that nothing is fundamentally broken for CDK processing e2e