From 7a10c3180a55ba7102952b4805e77f37c7d4e8b4 Mon Sep 17 00:00:00 2001
From: smartcontracts <kelvin@optimism.io>
Date: Wed, 18 Sep 2024 09:50:42 -0600
Subject: [PATCH] ci: simplify contracts bedrock tests job (#11966)

Simplifies the contracts-bedrock-tests job so that it takes more
parameters but makes the test command unified. Also replaces the
complex script for checking for modified fuzz tests and simply
heavily fuzzes all tests within any test files that have changed.
---
 .circleci/config.yml                          |  47 ++++--
 packages/contracts-bedrock/justfile           |   4 -
 .../testing/test-heavy-fuzz-modified-tests.sh | 147 ------------------
 3 files changed, 31 insertions(+), 167 deletions(-)
 delete mode 100755 packages/contracts-bedrock/scripts/testing/test-heavy-fuzz-modified-tests.sh

diff --git a/.circleci/config.yml b/.circleci/config.yml
index 07f6a55a3f9a..1a3cd04e6c9a 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -587,9 +587,13 @@ jobs:
         description: Number of test jobs to run in parallel
         type: integer
         default: 4
-      test_command:
+      test_list:
         description: List of test files to run
         type: string
+      test_fuzz_runs:
+        description: Number of fuzz runs to apply
+        type: integer
+        default: 512
       test_timeout:
         description: Timeout for running tests
         type: string
@@ -599,6 +603,15 @@ jobs:
       - checkout
       - attach_workspace: { at: "." }
       - install-contracts-dependencies
+      - run:
+          name: Check if test list is empty
+          command: |
+            TEST_FILES=$(<<parameters.test_list>>)
+            if [ -z "$TEST_FILES" ]; then
+              echo "No test files to run. Exiting early."
+              circleci-agent step halt
+            fi
+          working_directory: packages/contracts-bedrock
       - check-changed:
           patterns: contracts-bedrock,op-node
       - restore_cache:
@@ -610,11 +623,11 @@ jobs:
             - golang-build-cache-contracts-bedrock-tests-{{ checksum "go.sum" }}
             - golang-build-cache-contracts-bedrock-tests-
       - run:
-          name: print dependencies
+          name: Print dependencies
           command: just dep-status
           working_directory: packages/contracts-bedrock
       - run:
-          name: print forge version
+          name: Print forge version
           command: forge --version
           working_directory: packages/contracts-bedrock
       - run:
@@ -622,18 +635,24 @@ jobs:
           command: bash scripts/ops/pull-artifacts.sh
           working_directory: packages/contracts-bedrock
       - run:
-          name: build go-ffi
+          name: Build go-ffi
           command: just build-go-ffi
           working_directory: packages/contracts-bedrock
       - run:
-          name: run tests
-          command: <<parameters.test_command>>
+          name: Run tests
+          command: |
+            TEST_FILES=$(<<parameters.test_list>>)
+            TEST_FILES=$(echo "$TEST_FILES" | circleci tests split --split-by=timings)
+            TEST_FILES=$(echo "$TEST_FILES" | sed 's|^test/||')
+            MATCH_PATH="./test/{$(echo "$TEST_FILES" | paste -sd "," -)}"
+            export FOUNDRY_INVARIANT_RUNS=<<parameters.test_fuzz_runs>>
+            forge test --deny-warnings --fuzz-runs <<parameters.test_fuzz_runs>> --match-path "$MATCH_PATH"
           environment:
             FOUNDRY_PROFILE: ci
           working_directory: packages/contracts-bedrock
           no_output_timeout: <<parameters.test_timeout>>
       - run:
-          name: print failed test traces
+          name: Print failed test traces
           command: just test-rerun
           environment:
             FOUNDRY_PROFILE: ci
@@ -1626,23 +1645,19 @@ workflows:
           # Test everything except PreimageOracle.t.sol since it's slow.
           name: contracts-bedrock-tests
           test_parallelism: 4
-          test_command: |
-            TEST_FILES=$(find . -name "*.t.sol" -not -name "PreimageOracle.t.sol")
-            TEST_FILES=$(echo "$TEST_FILES" | circleci tests split --split-by=timings)
-            TEST_FILES=$(echo "$TEST_FILES" | sed 's|./test/||')
-            MATCH_PATH="./test/{$(echo "$TEST_FILES" | paste -sd "," -)}"
-            forge test --deny-warnings --match-path "$MATCH_PATH"
+          test_list: find test -name "*.t.sol" -not -name "PreimageOracle.t.sol"
       - contracts-bedrock-tests:
           # PreimageOracle test is slow, run it separately to unblock CI.
           name: contracts-bedrock-tests-preimage-oracle
           test_parallelism: 1
-          test_command: forge test --deny-warnings --match-path ./test/cannon/PreimageOracle.t.sol
+          test_list: find test -name "PreimageOracle.t.sol"
       - contracts-bedrock-tests:
-          # Heavily fuzz any fuzz tests that have been added or modified.
+          # Heavily fuzz any fuzz tests within added or modified test files.
           name: contracts-bedrock-tests-heavy-fuzz-modified
           test_parallelism: 1
+          test_list: git diff origin/develop...HEAD --name-only -- './test/**/*.t.sol' | sed 's|packages/contracts-bedrock/||'
           test_timeout: 1h
-          test_command: just test-heavy-fuzz-modified-tests
+          test_fuzz_runs: 10000
       - contracts-bedrock-coverage
       - contracts-bedrock-checks:
           requires:
diff --git a/packages/contracts-bedrock/justfile b/packages/contracts-bedrock/justfile
index bf96fd17cad1..7506681774b2 100644
--- a/packages/contracts-bedrock/justfile
+++ b/packages/contracts-bedrock/justfile
@@ -29,10 +29,6 @@ test-kontrol-no-build:
 test-rerun: build-go-ffi
   forge test --rerun -vvv
 
-# Run extra fuzz iterations for modified fuzz tests.
-test-heavy-fuzz-modified-tests: build-go-ffi
-  ./scripts/testing/test-heavy-fuzz-modified-tests.sh
-
 genesis:
   forge script scripts/L2Genesis.s.sol:L2Genesis --sig 'runWithStateDump()'
 
diff --git a/packages/contracts-bedrock/scripts/testing/test-heavy-fuzz-modified-tests.sh b/packages/contracts-bedrock/scripts/testing/test-heavy-fuzz-modified-tests.sh
deleted file mode 100755
index b7a8db6b5912..000000000000
--- a/packages/contracts-bedrock/scripts/testing/test-heavy-fuzz-modified-tests.sh
+++ /dev/null
@@ -1,147 +0,0 @@
-#!/usr/bin/env bash
-set -euo pipefail
-
-# This script is used to run extra fuzz test iterations on any fuzz tests that
-# have been added or modified in a PR. We typically want to run extra fuzz
-# iterations when new tests are added to make sure that they are not flaky with
-# some small percentage of fuzz runs.
-# NOTE: This script is NOT perfect and can only catch changes to fuzz tests
-# that are made within the test file itself. It won't catch changes to
-# dependencies or other external factors that might impact the behavior of the
-# fuzz test. This script may also run fuzz tests that have not actually been
-# modified.
-
-# Set the number of fuzz runs to run.
-FUZZ_RUNS=${1:-10000}
-
-# Set the number of invariant runs to run.
-INVARIANT_RUNS=${2:-10000}
-
-# Verify that FUZZ_RUNS is a number.
-if ! [[ "$FUZZ_RUNS" =~ ^[0-9]+$ ]]; then
-    echo "Fuzz runs must be a number"
-    exit 1
-fi
-
-# Trap any errors and exit.
-trap 'echo "Script failed at line $LINENO"' ERR
-
-# Get the various base directories.
-SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
-CONTRACTS_BASE=$(dirname "$(dirname "$SCRIPT_DIR")")
-ROOT_DIR=$(dirname "$(dirname "$CONTRACTS_BASE")")
-
-# Change to the root directory.
-cd "$ROOT_DIR"
-
-# Get a list of changed Solidity test files relative to the project root.
-CHANGED_FILES=$(git diff origin/develop...HEAD --name-only -- '*.sol')
-
-# Exit if no changed Solidity files are found.
-if [ -z "$CHANGED_FILES" ]; then
-    echo "No changed Solidity files found"
-    exit 0
-fi
-
-# Initialize an array to hold relevant function names.
-NEW_OR_MODIFIED_TEST_NAMES=""
-
-# these tests are too expensive to run heavily
-IGNORED_FUZZ_TESTS=("MIPS.t.sol" "MIPS2.t.sol")
-
-# Process each changed file.
-for FILE in $CHANGED_FILES; do
-    IGNORED=false
-    for TEST in "${IGNORED_FUZZ_TESTS[@]}"; do
-      FILENAME=$(basename "$FILE")
-      if [[ "$TEST" == "$FILENAME" ]]; then
-        IGNORED=true
-        break
-      fi
-    done
-    if $IGNORED; then
-      echo "skipping $FILE"
-      continue
-    fi
-    if [ ! -e "$FILE" ] ; then
-      echo "skipping $FILE since it was deleted"
-      continue
-    fi
-
-    # Get the diff for the file.
-    DIFF=$(git diff origin/develop...HEAD --unified=0 -- "$FILE")
-
-    # Figure out every modified line.
-    MODIFIED_LINES=$(echo "$DIFF" | \
-    awk '/^@@/ {
-        split($3, a, ",")
-        start = substr(a[1], 2)
-        if (length(a) > 1)
-            count = a[2]
-        else
-            count = 1
-        for (i = 0; i < count; i++)
-            print start + i
-    }' | sort -n | uniq | xargs)
-
-    # Extract function names and their line numbers from the entire file
-    FUNCTION_LINES=$(awk '/function testFuzz_|function invariant_/ {print FNR, $0}' "$FILE")
-
-    # If there are no function lines, skip the file.
-    if [ -z "$FUNCTION_LINES" ]; then
-        continue
-    fi
-
-    # Reverse the function lines so we can match the last function modified.
-    # We'd otherwise end up matching the first function with a line number less
-    # than the modified line number which is not what we want.
-    FUNCTION_LINES=$(echo "$FUNCTION_LINES" | sort -r)
-
-    # Process each modified line.
-    for MODIFIED_LINE_NUM in $MODIFIED_LINES; do
-        # Check all functions to find the last one where the line number of the
-        # function is less than or equal to the modified line number. This is
-        # the function that was most likely modified.
-        # NOTE: This is not perfect and may accidentally match a function that
-        # was not actually modified but it works well enough and at least won't
-        # accidentally miss any modified fuzz tests.
-        while IFS= read -r func; do
-            # Get the function line number and name.
-            FUNC_LINE_NUM=$(echo "$func" | awk '{print $1}')
-            FUNC_NAME=$(echo "$func" | awk '{print $3}' | sed 's/(.*//')
-
-            # Check if the modified line number is greater than or equal to the
-            # function line number. If it is, then we've found the closest fuzz
-            # test that was modified. Again, this is not perfect and may lead
-            # to false positives but won't lead to false negatives.
-            if [ "$MODIFIED_LINE_NUM" -ge "$FUNC_LINE_NUM" ]; then
-                NEW_OR_MODIFIED_TEST_NAMES+="$FUNC_NAME "
-                break
-            fi
-        done <<< "$FUNCTION_LINES"
-    done
-done
-
-# Remove duplicates and sort.
-NEW_OR_MODIFIED_TEST_NAMES=$(echo "$NEW_OR_MODIFIED_TEST_NAMES" | xargs -n1 | sort -u | xargs)
-
-# Exit if no new or modified fuzz tests are found.
-if [ -z "$NEW_OR_MODIFIED_TEST_NAMES" ]; then
-    echo "No new or modified fuzz tests found"
-    exit 0
-fi
-
-# Print the detected tests on different lines.
-echo "Detected new or modified fuzz tests:"
-for TEST_NAME in $NEW_OR_MODIFIED_TEST_NAMES; do
-    echo "  $TEST_NAME"
-done
-
-# Change to the contracts base directory.
-cd "$CONTRACTS_BASE"
-
-# Set the number of invariant runs.
-export FOUNDRY_INVARIANT_RUNS="$INVARIANT_RUNS"
-
-# Run the detected tests with extra fuzz runs
-forge test --match-test "${NEW_OR_MODIFIED_TEST_NAMES// /|}" --fuzz-runs "$FUZZ_RUNS"