feat(coverage): Enable coverage on any test change (rebased with uv) (ethereum#790)

winsvega · danceratopz · web-flow · commit f89d09ba99a3 · 2024-10-25T16:19:26.000-06:00
* coverage on every pr

* do not try to detect tests in files
do not skip the coverage script for safety

* build the changed files array once

* simplify regex for gitdiff changed files detection

* use uv run in coverage script

* refactor(ci): use changed-files action to detect changes in tests/

* fix the coverage script, remove retesteth, simplify

* exclude prague

* address issues

* fix evmone long build
fix uv run fill on empty file set

---------

Co-authored-by: danceratopz &lt;danceratopz@gmail.com&gt;
diff --git a/.github/actions/build-evm-client/evmone/action.yaml b/.github/actions/build-evm-client/evmone/action.yaml
@@ -9,6 +9,10 @@ inputs:
     description: 'Reference to branch, commit, or tag to use to build the EVM binary'
     required: true
     default: 'master'
+  targets:
+    description: 'Which targets to build from evmone repo'
+    required: false
+    default: 'all'
 runs:
   using: "composite"
   steps:
@@ -27,5 +31,5 @@ runs:
         mkdir -p $GITHUB_WORKSPACE/bin
         cd $GITHUB_WORKSPACE/evmone
         cmake -S . -B build -DEVMONE_TESTING=ON
-        cmake --build build --parallel
-        echo $GITHUB_WORKSPACE/evmone/build/bin/ >> $GITHUB_PATH
+        cmake --build build --parallel --target ${{ inputs.targets }}
+        echo $GITHUB_WORKSPACE/evmone/build/bin/ >> $GITHUB_PATH
diff --git a/.github/workflows/coverage.yaml b/.github/workflows/coverage.yaml
@@ -3,60 +3,99 @@ name: Evmone Coverage Report
 on:
   pull_request:
     paths:
-      - "converted-ethereum-tests.txt" # This triggers the workflow only for changes in file.txt
+      - 'tests/**' # This triggers the workflow for any changes in the tests folder
+      - '!tests/prague/**'  # exclude changes in 'tests/prague'
+      - '!tests/osaka/**'   # exclude changes in 'tests/osaka'
 
 jobs:
   evmone-coverage-diff:
     runs-on: ubuntu-latest
-    strategy:
-      matrix:
-        driver: [retesteth, native]
 
     steps:
       - name: Checkout code
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
+
+      - name: Debug GitHub context
+        run: |
+          echo "Git reference: ${{ github.ref }}"
+          echo "Git head ref: ${{ github.head_ref }}"
+          echo "Git base ref: ${{ github.base_ref }}"
+          echo "Node Version: $(node -v)"
+          echo "NPM Version: $(npm -v)"
+
+
+      - name: Get all changed python files in tests/ and changes to coverted-ethereum-tests.txt
+        id: changed-tests
+        uses: tj-actions/changed-files@v45
+        with:
+          # TODO: non-test modules such as __init__.py or spec.py could effect coverage - in this case we should
+          # fill all applicable tests (i.e., all the test_*.py files in or under the changed module's directory)
+          include_all_old_new_renamed_files: true
+          output_renamed_files_as_deleted_and_added: true
+          files_yaml: |
+            tests:
+              - tests/**/test_*.py
+              - '!tests/prague/**'
+              - '!tests/osaka/**'
+            converted_tests:
+              - converted-ethereum-tests.txt
+
+      - name: Exit workflow if there are no changed python files
+        if: steps.changed-tests.outputs.tests_any_changed != 'true'
+        run: |
+          echo "No python files were changed in ./tests/ - no action necessary"
+          exit 0
 
-      - name: Fetch target branch
-        run: git fetch origin ${{ github.base_ref }}:refs/remotes/origin/${{ github.base_ref }}
+      - name: Report changed python test moudules
+        if: steps.changed-tests.outputs.tests_any_changed == 'true'
+        run: |
+          echo "${{ toJson(steps.changed-tests.outputs) }}"
+          echo "Changed python test modules: ${{ steps.changed-tests.outputs.tests_all_modified_files }}"
+
+      - name: Debug GitHub context
+        run: |
+          echo "Git reference: ${{ github.ref }}"
+          echo "Git head ref: ${{ github.head_ref }}"
+          echo "Git base ref: ${{ github.base_ref }}"
 
       - name: Log in to Docker Hub
+        if: ${{  steps.changed-tests.outputs.tests_any_changed == 'true' && github.event.pull_request.head.repo.full_name == github.repository }}
         uses: docker/login-action@v3
-        if: ${{ github.event.pull_request.head.repo.full_name == github.repository }}
         with:
           username: winsvega
           password: ${{ secrets.DOCKERHUB_PASSWORD }}
 
       - name: Install deps
+        if: steps.changed-tests.outputs.tests_any_changed == 'true'
         run: |
           echo $(pwd)
           echo ${{ github.workspace }}
 
       - name: Set up uv
+        if: steps.changed-tests.outputs.tests_any_changed == 'true'
         uses: ./.github/actions/setup-uv
 
       - name: Set up Python
+        if: steps.changed-tests.outputs.tests_any_changed == 'true'
         run: uv python install 3.10
 
       - name: Install EEST
+        if: steps.changed-tests.outputs.tests_any_changed == 'true'
         run: |
           uv sync --no-progress
           uv run python --version
 
       # Required to fill .py tests
-      - name: Build GO EVM
-        uses: ./.github/actions/build-evm-client/geth
-        id: evm-builder
-        with:
-          type: "main"
-
       - name: Build EVMONE EVM
         uses: ./.github/actions/build-evm-client/evmone
+        if: steps.changed-tests.outputs.tests_any_changed == 'true'
         id: evm-builder2
         with:
-          type: "main"
+          targets: "evmone-t8n"
 
       - name: Checkout ethereum/tests
         uses: actions/checkout@v4
+        if: steps.changed-tests.outputs.tests_any_changed == 'true'
         with:
           repository: ethereum/tests
           path: testpath
@@ -66,6 +105,7 @@ jobs:
 
       - name: Checkout ethereum/legacytests
         uses: actions/checkout@v4
+        if: steps.changed-tests.outputs.tests_any_changed == 'true'      
         with:
           repository: ethereum/legacytests
           path: legacytestpath
@@ -74,16 +114,19 @@ jobs:
 
       # This command diffs the file and filters in new lines
       - name: Parse converted tests from converted-ethereum-tests.txt
+        if: steps.changed-tests.outputs.tests_any_changed == 'true'
         run: |
           echo "New lines introduced in converted-ethereum-tests.txt:"
-          lines=$(git diff origin/${{ github.base_ref }} HEAD -- converted-ethereum-tests.txt | grep "^+" | grep -v "^+++")
-          files=$(echo "$lines" | grep -oP '(?<=\+).+\.json')
-
-          if [ -z "$files" ]; then
-              echo "Error: No new JSON files found in converted-ethereum-tests.txt"
-              exit 1
+          lines=$(git diff origin/${{ github.base_ref }} HEAD -- converted-ethereum-tests.txt | grep "^+" | grep -v "^+++" || true)
+          if [ -z "$lines" ]; then
+              echo "No new lines in converted-ethereum-tests.txt, check updates instead:"
+              echo "converted_skip=true" >> $GITHUB_ENV
+              exit 0
+          else
+              echo "converted_skip=false" >> $GITHUB_ENV
           fi
 
+          files=$(echo "$lines" | grep -oP '(?<=\+).+\.json')
           for file in $files; do
               echo $file
           done
@@ -123,53 +166,26 @@ jobs:
 
       # This command diffs the .py scripts introduced by a PR
       - name: Parse and fill introduced test sources
+        if: steps.changed-tests.outputs.tests_any_changed == 'true'
+        env:
+          CHANGED_TEST_FILES: ${{ steps.changed-tests.outputs.tests_all_changed_files }}
         run: |
-          python3 -m venv ./venv/
-          source ./venv/bin/activate
-
-          if [ "${{ github.event.pull_request.head.repo.full_name }}" != "${{ github.repository }}" ]; then
-              # Fetch changes when PR comes from remote repo
-              git fetch origin +refs/heads/${{ github.base_ref }}:refs/remotes/origin/${{ github.base_ref }}
-              git fetch origin +refs/pull/${{ github.event.pull_request.number }}/head:refs/remotes/origin/PR-${{ github.event.pull_request.number }}
-              files=$(git diff --name-status origin/${{ github.base_ref }}...origin/PR-${{ github.event.pull_request.number }} -- tests/ | grep -E '^[AM]' | grep '\.py$')
-          else
-              # Fetch the base branch and the head branch
-              git fetch origin ${{ github.base_ref }}:refs/remotes/origin/${{ github.base_ref }}
-              git fetch origin ${{ github.head_ref }}:refs/remotes/origin/${{ github.head_ref }}
-
-              # Perform the diff
-              files=$(git diff --name-status origin/${{ github.base_ref }}...origin/${{ github.head_ref }} -- tests/ | grep -E '^[AM]' | grep '\.py$')
-          fi
-
-
-          echo "Modified or new .py files in tests folder:"
-          echo "$files" | while read line; do
-            file=$(echo "$line" | cut -c 3-)
-            echo $file
-          done
+          source $GITHUB_ENV
+          files=$(echo "$CHANGED_TEST_FILES" | tr ',' '\n')
 
           # fill new tests
           # using `|| true` here because if no tests found, pyspec fill returns error code
           mkdir -p fixtures/state_tests
           mkdir -p fixtures/eof_tests
-          echo "$files" | while read line; do
-            file=$(echo "$line" | cut -c 3-)
-            uv run fill $file --until=Cancun --evm-bin evmone-t8n --solc-version=0.8.25 || true >> filloutput.log 2>&1
-            (uv run fill $file --fork=CancunEIP7692 --evm-bin evmone-t8n --solc-version=0.8.25 -k eof_test || true) > >(tee -a filloutput.log filloutputEOF.log) 2>&1
-          done
+
+          echo "uv run fill $files --until=Cancun --evm-bin evmone-t8n >> filloutput.log 2>&1"
+          uv run fill $files --until=Cancun --evm-bin evmone-t8n >> filloutput.log 2>&1
+          cat filloutput.log
 
           if grep -q "FAILURES" filloutput.log; then
               echo "Error: failed to generate .py tests."
               exit 1
           fi
-          if [ "${{ matrix.driver }}" = "retesteth" ] && grep -q "passed" filloutputEOF.log; then
-              echo "Disabling retesteth coverage check as EOF tests detected!"
-              echo "retesteth_skip=true" >> $GITHUB_ENV
-              exit 0
-          else
-              echo "retesteth_skip=false" >> $GITHUB_ENV
-          fi
-
 
           filesState=$(find fixtures/state_tests -type f -name "*.json")
           filesEOF=$(find fixtures/eof_tests -type f -name "*.json")
@@ -178,13 +194,78 @@ jobs:
               exit 1
           fi
 
+          # Include basic evm operations into coverage by default
+          # As when we translate from yul/solidity some dup/push opcodes could become untouched
+          uv run fill tests/homestead/coverage/test_coverage.py --until=Cancun --evm-bin evmone-t8n
+
           PATCH_TEST_PATH=${{ github.workspace }}/evmtest_coverage/coverage/PATCH_TESTS
           mkdir -p $PATCH_TEST_PATH
           find fixtures/state_tests -type f -name "*.json" -exec cp {} $PATCH_TEST_PATH \;
           find fixtures/eof_tests -type f -name "*.json" -exec cp {} $PATCH_TEST_PATH \;
 
+      - name: Parse and fill introduced test sources from before the PR
+        if: ${{ steps.changed-tests.outputs.tests_any_changed == 'true' && env.converted_skip == 'true' }}
+        env:
+          CHANGED_TEST_FILES: ${{ steps.changed-tests.outputs.tests_all_modified_files }}
+        run: |
+            echo "--------------------"
+            echo "converted-ethereum-tests.txt seem untouched, try to fill pre-patched version of .py files:"
+
+            source $GITHUB_ENV
+            files=$(echo "$CHANGED_TEST_FILES" | tr ',' '\n')
+
+            git checkout main
+            PREV_COMMIT=$(git rev-parse HEAD)
+            echo "Checkout head $PREV_COMMIT"
+
+            # Take only those files that exist in the filesystem (ignore newly created files)
+            files_fixed=$(echo "$files" | tr ' ' '\n' | while read file; do
+              if [ -f "$file" ]; then
+                echo "$file"
+              fi
+            done | tr '\n' ' ')
+
+            echo "Select files that were changed and exist on the main branch:"
+            echo $files_fixed
+
+            rm -r fixtures
+            rm filloutput.log
+            mkdir -p fixtures/state_tests
+            mkdir -p fixtures/eof_tests
+
+            if [ -n "$files_fixed" ]; then
+                echo "uv run fill $files_fixed --until=Cancun --evm-bin evmone-t8n >> filloutput.log 2>&1"
+                uv run fill $files_fixed --until=Cancun --evm-bin evmone-t8n >> filloutput.log 2>&1
+                cat filloutput.log
+
+                if grep -q "FAILURES" filloutput.log; then
+                  echo "Error: failed to generate .py tests from before the PR."
+                  exit 1
+                fi
+
+                if grep -q "ERROR collecting test session" filloutput.log; then
+                  echo "Error: failed to generate .py tests from before the PR."
+                  exit 1
+                fi
+            else
+                echo "No tests affected from before patch!"
+            fi
+
+            filesState=$(find fixtures/state_tests -type f -name "*.json")
+            filesEOF=$(find fixtures/eof_tests -type f -name "*.json")
+
+            BASE_TEST_PATH=${{ github.workspace }}/evmtest_coverage/coverage/BASE_TESTS
+            mkdir -p $BASE_TEST_PATH
+            find fixtures/state_tests -type f -name "*.json" -exec cp {} $BASE_TEST_PATH \;
+            find fixtures/eof_tests -type f -name "*.json" -exec cp {} $BASE_TEST_PATH \;
+            for file in $BASE_TEST_PATH/*.json; do
+                if [ -e "$file" ]; then
+                    mv "$file" "${file%.json}_$PREV_COMMIT.json"
+                fi
+            done
+
       - name: Print tests that will be covered
-        if: ${{ env.retesteth_skip == 'false' || matrix.driver == 'native' }}
+        if: ${{ steps.changed-tests.outputs.tests_any_changed == 'true' }}
         run: |
           echo "Original BASE tests:"
           ls ${{ github.workspace }}/evmtest_coverage/coverage/BASE_TESTS
@@ -194,44 +275,44 @@ jobs:
 
       - name: Run coverage of the BASE tests
         uses: addnab/docker-run-action@v3
-        if: ${{ env.retesteth_skip == 'false' || matrix.driver == 'native' }}
+        if: ${{ steps.changed-tests.outputs.tests_any_changed == 'true' }}
         with:
           image: winsvega/evmone-coverage-script:latest
           options: -v ${{ github.workspace }}/evmtest_coverage/coverage:/tests
-          run: /entrypoint.sh --mode=cover --driver=${{ matrix.driver }} --testpath=/tests/BASE_TESTS --outputname=BASE
+          run: /entrypoint.sh --mode=cover --driver=native --testpath=/tests/BASE_TESTS --outputname=BASE
 
       - name: Run coverage of the PATCH tests
         uses: addnab/docker-run-action@v3
-        if: ${{ env.retesteth_skip == 'false' || matrix.driver == 'native' }}
+        if: ${{ steps.changed-tests.outputs.tests_any_changed == 'true' }}
         with:
           image: winsvega/evmone-coverage-script:latest
           options: -v ${{ github.workspace }}/evmtest_coverage/coverage:/tests
-          run: /entrypoint.sh --mode=cover --driver=${{ matrix.driver }} --testpath=/tests/PATCH_TESTS --outputname=PATCH
+          run: /entrypoint.sh --mode=cover --driver=native --testpath=/tests/PATCH_TESTS --outputname=PATCH
 
       - name: Run coverage DIFF of the PATCH tests compared to BASE tests
         uses: addnab/docker-run-action@v3
-        if: ${{ env.retesteth_skip == 'false' || matrix.driver == 'native' }}
+        if: ${{  steps.changed-tests.outputs.tests_any_changed == 'true' }}
         with:
           image: winsvega/evmone-coverage-script:latest
           options: -v ${{ github.workspace }}/evmtest_coverage/coverage:/tests
           run: /entrypoint.sh --mode=diff --basefile=coverage_BASE.lcov --patchfile=coverage_PATCH.lcov
 
       - name: Chmod coverage results
-        if: ${{ env.retesteth_skip == 'false' || matrix.driver == 'native' }}
+        if: ${{ steps.changed-tests.outputs.tests_any_changed == 'true' }}
         run: |
           user=$(whoami)
           sudo chown -R $user:$user ${{ github.workspace }}/evmtest_coverage/coverage
 
       - name: Upload coverage results
         uses: actions/upload-artifact@v3
-        if: ${{ env.retesteth_skip == 'false' || matrix.driver == 'native' }}
+        if: ${{ steps.changed-tests.outputs.tests_any_changed == 'true' }}
         with:
-          name: coverage-diff
+          name: coverage-diff-native
           path: ${{ github.workspace }}/evmtest_coverage/coverage
 
       - name: Verify coverage results
         uses: addnab/docker-run-action@v3
-        if: ${{ env.retesteth_skip == 'false' || matrix.driver == 'native' }}
+        if: ${{ steps.changed-tests.outputs.tests_any_changed == 'true' }}
         with:
           image: winsvega/evmone-coverage-script:latest
           options: -v ${{ github.workspace }}/evmtest_coverage/coverage:/tests
