ci(levm): silence messages #1837
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,9 +2,9 @@ name: LEVM | |
|
|
||
| on: | ||
| push: | ||
| branches: ["main"] | ||
| pull_request: | ||
| branches: ["**"] | ||
| paths: | ||
| - "crates/vm/levm/**" | ||
|
|
||
|
|
@@ -58,6 +58,49 @@ jobs: | |
| echo "Percentage is not 100%." | ||
| exit 1 | ||
| fi | ||
| hive-report-creation: | ||
| uses: ./.github/workflows/common_hive_reports.yaml | ||
|
There was a problem hiding this comment. since these are ran daily, can't we take the results from the last (levm) daily run and compare it to the current run? There was a problem hiding this comment. I think it's done in commit e91040b. We would have to merge it to fully test it |
||
| # Check we don't have a regression | ||
| hive-test-check: | ||
| # "Integration Test" is a required check, don't change the name | ||
| name: Integration Test | ||
|
There was a problem hiding this comment. We have two integration test, can we do something like this instead? https://github.com/lambdaclass/ethrex/blob/main/.github/workflows/ci_l1.yaml#L207-L226 There was a problem hiding this comment. Working on it There was a problem hiding this comment. Done in 5f3a652 |
||
| needs: hive-report-creation | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout sources | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Download results (levm) | ||
| uses: actions/download-artifact@v4 | ||
| with: | ||
| name: results_levm.md | ||
|
|
||
| - name: Rename result (1) | ||
| run: cp results.md results_levm.md | ||
|
|
||
| - name: Download results (revm) | ||
| uses: actions/download-artifact@v4 | ||
| with: | ||
| name: results_revm.md | ||
|
|
||
| - name: Rename result (2) | ||
| run: cp results.md results_revm.md | ||
|
|
||
| - name: Create diff message | ||
| run: | | ||
| bash .github/scripts/levm_revm_diff.sh results_revm.md results_levm.md >> diff.md | ||
| cat diff.md >> $GITHUB_STEP_SUMMARY | ||
| - name: Check Regression | ||
| run: | | ||
| if grep -q "No differences found" diff.md; then | ||
|
There was a problem hiding this comment. This doesn't seem to check that levm has a regresion (levm vs levm), but that levm is equal to revm. So this check will fail until there is feature parity between revm and levm. Is that what we want? There was a problem hiding this comment. We would have to check against main, i will start with it, maybe it's too cumbersome for now. When LEVM is used as default we would definitely want the (levm vs levm) regression check. There was a problem hiding this comment. I thinks it's done in commit e91040b. I save the daily artifact and use it for the comparison/check. |
||
| echo "No regression found." | ||
| else | ||
| echo "Regression found." | ||
| exit 1; | ||
| fi | ||
| test: | ||
| # "Test" is a required check, don't change the name | ||
| name: Test | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| name: Run Hive Tests | ||
|
There was a problem hiding this comment. nit: maybe I would move the reusable jobs to There was a problem hiding this comment. I wasn't able to do it, the job doesn't run. Seems that the feature is not present on github's CI. There are some issues related to subfolders: https://github.com/orgs/community/discussions/18055. I don't know if it's a bug or what, because the actions' linter throws an error if i point to a |
||
|
|
||
| on: workflow_call | ||
|
There was a problem hiding this comment. would it make sense to have an There was a problem hiding this comment. Done in commit e91040b. The |
||
|
|
||
| env: | ||
| CARGO_TERM_COLOR: always | ||
| RUST_VERSION: 1.81.0 | ||
|
|
||
| jobs: | ||
| run-hive: | ||
| name: Hive (${{ matrix.vm }}) - ${{ matrix.test.name }} | ||
| runs-on: ubuntu-latest | ||
| strategy: | ||
| matrix: | ||
| vm: [levm, revm] | ||
| test: | ||
| - { | ||
| name: "Rpc Compat tests", | ||
| file_name: rpc-compat, | ||
| simulation: ethereum/rpc-compat, | ||
| } | ||
| - { name: "Devp2p eth tests", file_name: devp2p, simulation: devp2p } | ||
| - { | ||
| name: "Cancun Engine tests", | ||
| file_name: engine, | ||
| simulation: ethereum/engine, | ||
| } | ||
| - { name: "Sync tests", file_name: sync, simulation: ethereum/sync } | ||
|
|
||
| steps: | ||
| - name: Pull image | ||
| if: ${{ matrix.vm == 'revm' }} | ||
| run: | | ||
| docker pull ghcr.io/lambdaclass/ethrex:latest | ||
| docker tag ghcr.io/lambdaclass/ethrex:latest ethrex:latest | ||
|
|
||
| - name: Checkout sources | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Build Image with LEVM | ||
| if: ${{ matrix.vm == 'levm' }} | ||
| run: cd crates/vm/levm && make build-image-levm | ||
|
|
||
| - name: Setup Go | ||
| uses: actions/setup-go@v5 | ||
|
|
||
| - name: Setup Hive | ||
| run: make setup-hive | ||
|
|
||
| - name: Run Hive Simulation | ||
| run: cd hive && ./hive --client ethrex --sim ${{ matrix.test.simulation }} --sim.parallelism 16 | ||
| continue-on-error: true | ||
|
|
||
| - name: Upload results | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: ${{ matrix.test.file_name }}_${{ matrix.vm }}_logs | ||
| path: hive/workspace/logs/*-*.json | ||
| if-no-files-found: error | ||
|
|
||
| hive-report: | ||
| name: Generate and Save report (${{ matrix.vm }}) | ||
| needs: run-hive | ||
| runs-on: ubuntu-latest | ||
| strategy: | ||
| matrix: | ||
| vm: [levm, revm] | ||
| steps: | ||
| - name: Checkout sources | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Rustup toolchain install | ||
| uses: dtolnay/rust-toolchain@stable | ||
| with: | ||
| toolchain: ${{ env.RUST_VERSION }} | ||
|
|
||
| - name: Download all results | ||
| uses: actions/download-artifact@v4 | ||
| with: | ||
| path: hive/workspace/logs | ||
| pattern: "*_${{ matrix.vm }}_logs" | ||
| merge-multiple: true | ||
|
|
||
| - name: Caching | ||
| uses: Swatinem/rust-cache@v2 | ||
|
|
||
| - name: Generate the hive report | ||
| run: cargo run -p hive_report > results.md | ||
|
|
||
| - name: Upload ${{matrix.vm}} result | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: results_${{matrix.vm}}.md | ||
| path: results.md | ||
| if-no-files-found: error | ||
|
|
||
| - name: Post results in summary | ||
| run: | | ||
| echo "# Hive coverage report (${{ matrix.vm }})" >> $GITHUB_STEP_SUMMARY | ||
| cat results.md >> $GITHUB_STEP_SUMMARY | ||
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.
why is this file not in ci_levm? That would be more consistent
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.
Because one file has the
Integration Testjob, making it arequired check.And the
ci_bench_levm_in_pr.yamlonly modifies the github comments.The logic was separated in case of an external contribution, if we send the PR comment inside the
Integration Testjob in order to avoid repetition, the job will fail because the external contributor doesn't have access to the TOKENs