[NIT-2851] Use flag instead of env var for system_test

.github/workflows/ci.yml

@@ -142,33 +142,27 @@ jobs:
 
       - name: run tests without race detection and path state scheme
         if: matrix.test-mode == 'defaults'
-        env:
-          TEST_STATE_SCHEME: path
         run: |
           echo "Running tests with Path Scheme" >> full.log
-          ${{ github.workspace }}/.github/workflows/gotestsum.sh --tags cionly --timeout 20m --cover
+          ${{ github.workspace }}/.github/workflows/gotestsum.sh --tags cionly --timeout 20m --cover --flags -TEST_STATE_SCHEME=hash
 
       - name: run tests without race detection and hash state scheme
         if: matrix.test-mode == 'defaults'
-        env:
-          TEST_STATE_SCHEME: hash
         run: |
           echo "Running tests with Hash Scheme" >> full.log
-          ${{ github.workspace }}/.github/workflows/gotestsum.sh --tags cionly --timeout 20m
+          ${{ github.workspace }}/.github/workflows/gotestsum.sh --tags cionly --timeout 20m --flags -TEST_STATE_SCHEME=hash
 
       - name: run tests with race detection and hash state scheme
         if: matrix.test-mode == 'race'
-        env:
-          TEST_STATE_SCHEME: hash
         run: |
           echo "Running tests with Hash Scheme" >> full.log
-          ${{ github.workspace }}/.github/workflows/gotestsum.sh --race --timeout 30m
+          ${{ github.workspace }}/.github/workflows/gotestsum.sh --race --timeout 30m --flags -TEST_STATE_SCHEME=hash
 
       - name: run redis tests
         if: matrix.test-mode == 'defaults'
         run: |
           echo "Running redis tests" >> full.log
-          TEST_REDIS=redis://localhost:6379/0 gotestsum --format short-verbose -- -p 1 -run TestRedis ./arbnode/... ./system_tests/... -coverprofile=coverage-redis.txt -covermode=atomic -coverpkg=./...
+          gotestsum --format short-verbose -- -p 1 -run TestRedis ./arbnode/... ./system_tests/... -coverprofile=coverage-redis.txt -covermode=atomic -coverpkg=./... -TEST_REDIS=redis://localhost:6379/0
 
       - name: create block input json file
         if: matrix.test-mode == 'defaults'

.github/workflows/gotestsum.sh

@@ -10,6 +10,7 @@ check_missing_value() {
 timeout=""
 tags=""
 run=""
+flags=""
 race=false
 cover=false
 while [[ $# -gt 0 ]]; do
@@ -32,6 +33,12 @@ while [[ $# -gt 0 ]]; do
       run=$1
       shift
       ;;
+    --flags)
+      shift
+      check_missing_value $# "$1" "--flags"
+      flags=$1
+      shift
+      ;;
     --race)
       race=true
       shift
@@ -63,6 +70,10 @@ for package in $packages; do
     cmd="$cmd -run=$run"
   fi
 
+  if [ "$flags" != "" ]; then
+    cmd="$cmd $flags"
+  fi
+
   if [ "$race" == true ]; then
     cmd="$cmd -race"
   fi

Makefile

@@ -240,7 +240,7 @@ test-go-stylus: test-go-deps
 
 .PHONY: test-go-redis
 test-go-redis: test-go-deps
-	TEST_REDIS=redis://localhost:6379/0 gotestsum --format short-verbose --no-color=false -- -p 1 -run TestRedis ./system_tests/... ./arbnode/...
+	gotestsum --format short-verbose --no-color=false -- -p 1 -run TestRedis ./system_tests/... ./arbnode/... -TEST_REDIS=redis://localhost:6379/0
 	@printf $(done)
 
 .PHONY: test-gen-proofs

arbos/programs/cgo_test.go

@@ -7,12 +7,16 @@
 package programs
 
 import (
+	"flag"
 	"fmt"
-	"os"
 	"strings"
 	"testing"
 )
 
+var (
+	compileFlag = flag.String("TEST_COMPILE", "", "[STORE|LOAD] to allow store/load in compile test")
+)
+
 func TestConstants(t *testing.T) {
 	err := testConstants()
 	if err != nil {
@@ -22,20 +26,20 @@ func TestConstants(t *testing.T) {
 
 // normal test will not write anything to disk
 // to test cross-compilation:
-// * run test with TEST_COMPILE=STORE on one machine
+// * run test with -TEST_COMPILE=STORE on one machine
 // * copy target/testdata to the other machine
-// * run test with TEST_COMPILE=LOAD on the other machine
+// * run test with -TEST_COMPILE=LOAD on the other machine
 func TestCompileArch(t *testing.T) {
-	compile_env := os.Getenv("TEST_COMPILE")
-	if compile_env == "" {
-		fmt.Print("use TEST_COMPILE=[STORE|LOAD] to allow store/load in compile test")
+	flag.Parse()
+	if *compileFlag == "" {
+		fmt.Print("use -TEST_COMPILE=[STORE|LOAD] to allow store/load in compile test")
 	}
-	store := strings.Contains(compile_env, "STORE")
+	store := strings.Contains(*compileFlag, "STORE")
 	err := testCompileArch(store)
 	if err != nil {
 		t.Fatal(err)
 	}
-	if store || strings.Contains(compile_env, "LOAD") {
+	if store || strings.Contains(*compileFlag, "LOAD") {
 		err = testCompileLoad()
 		if err != nil {
 			t.Fatal(err)

das/aggregator_test.go

@@ -7,6 +7,7 @@ import (
 	"bytes"
 	"context"
 	"errors"
+	"flag"
 	"fmt"
 	"io"
 	"math/rand"
@@ -22,6 +23,12 @@ import (
 	"github.com/offchainlabs/nitro/blsSignatures"
 )
 
+var (
+	seedFlag    = flag.String("SEED", "", "Seed for random number generator")
+	runsFlag    = flag.String("RUNS", "", "Number of runs for test")
+	loggingFlag = flag.String("LOGGING", "", "Enable logging")
+)
+
 func TestDAS_BasicAggregationLocal(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
@@ -243,25 +250,23 @@ func testConfigurableStorageFailures(t *testing.T, shouldFailAggregation bool) {
 func initTest(t *testing.T) int {
 	t.Parallel()
 	seed := time.Now().UnixNano()
-	seedStr := os.Getenv("SEED")
-	if len(seedStr) > 0 {
+	flag.Parse()
+	if len(*seedFlag) > 0 {
 		var err error
-		intSeed, err := strconv.Atoi(seedStr)
+		intSeed, err := strconv.Atoi(*seedFlag)
 		Require(t, err, "Failed to parse string")
 		seed = int64(intSeed)
 	}
 	rand.Seed(seed)
 
-	runsStr := os.Getenv("RUNS")
 	runs := 2 ^ 32
-	if len(runsStr) > 0 {
+	if len(*runsFlag) > 0 {
 		var err error
-		runs, err = strconv.Atoi(runsStr)
+		runs, err = strconv.Atoi(*runsFlag)
 		Require(t, err, "Failed to parse string")
 	}
 
-	loggingStr := os.Getenv("LOGGING")
-	if len(loggingStr) > 0 {
+	if len(*loggingFlag) > 0 {
 		enableLogging()
 	}
 

system_tests/common_test.go

@@ -1681,11 +1681,11 @@ func doUntil(t *testing.T, delay time.Duration, max int, lambda func() bool) {
 }
 
 func TestMain(m *testing.M) {
-	logLevelEnv := os.Getenv("TEST_LOGLEVEL")
-	if logLevelEnv != "" {
-		logLevel, err := strconv.ParseInt(logLevelEnv, 10, 32)
+	flag.Parse()
+	if *logLevelFlag != "" {
+		logLevel, err := strconv.ParseInt(*logLevelFlag, 10, 32)
 		if err != nil || logLevel > int64(log.LevelCrit) {
-			log.Warn("TEST_LOGLEVEL exists but out of bound, ignoring", "logLevel", logLevelEnv, "max", log.LvlTrace)
+			log.Warn("-TEST_LOGLEVEL exists but out of bound, ignoring", "logLevel", *logLevelFlag, "max", log.LvlTrace)
 		}
 		glogger := log.NewGlogHandler(
 			log.NewTerminalHandler(io.Writer(os.Stderr), false))
@@ -1721,6 +1721,7 @@ var (
 	recordBlockInputsWithBaseDir                  = flag.String("recordBlockInputs.WithBaseDir", "", "Base directory for validationInputsWriter")
 	recordBlockInputsWithTimestampDirEnabled      = flag.Bool("recordBlockInputs.WithTimestampDirEnabled", true, "Whether to add timestamp directory while recording block inputs")
 	recordBlockInputsWithBlockIdInFileNameEnabled = flag.Bool("recordBlockInputs.WithBlockIdInFileNameEnabled", true, "Whether to record block inputs using test specific block_id")
+	logLevelFlag                                  = flag.String("TEST_LOGLEVEL", "", "Log level for tests")
 )
 
 // recordBlock writes a json file with all of the data needed to validate a block.

system_tests/das_test.go

@@ -7,20 +7,15 @@ import (
 	"context"
 	"encoding/base64"
 	"errors"
-	"io"
-	"log/slog"
 	"math/big"
 	"net"
 	"net/http"
-	"os"
-	"strconv"
 	"testing"
 	"time"
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/ethclient"
-	"github.com/ethereum/go-ethereum/log"
 
 	"github.com/offchainlabs/nitro/arbnode"
 	"github.com/offchainlabs/nitro/blsSignatures"
@@ -195,7 +190,7 @@ func checkBatchPosting(t *testing.T, ctx context.Context, l1client, l2clientA *e
 }
 
 func TestDASComplexConfigAndRestMirror(t *testing.T) {
-	initTest(t)
+	t.Parallel()
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
@@ -305,24 +300,6 @@ func TestDASComplexConfigAndRestMirror(t *testing.T) {
 	Require(t, err)
 }
 
-func enableLogging(logLvl int) {
-	glogger := log.NewGlogHandler(
-		log.NewTerminalHandler(io.Writer(os.Stderr), false))
-	glogger.Verbosity(slog.Level(logLvl))
-	log.SetDefault(log.NewLogger(glogger))
-}
-
-func initTest(t *testing.T) {
-	t.Parallel()
-	loggingStr := os.Getenv("LOGGING")
-	if len(loggingStr) > 0 {
-		var err error
-		logLvl, err := strconv.Atoi(loggingStr)
-		Require(t, err, "Failed to parse string")
-		enableLogging(logLvl)
-	}
-}
-
 func TestDASBatchPosterFallback(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()

system_tests/full_challenge_impl_test.go

@@ -8,7 +8,6 @@ import (
 	"context"
 	"io"
 	"math/big"
-	"os"
 	"strings"
 	"testing"
 	"time"
@@ -18,7 +17,6 @@ import (
 	"github.com/ethereum/go-ethereum/core/rawdb"
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/ethclient"
-	"github.com/ethereum/go-ethereum/log"
 	"github.com/ethereum/go-ethereum/node"
 	"github.com/ethereum/go-ethereum/params"
 	"github.com/ethereum/go-ethereum/rlp"
@@ -234,11 +232,6 @@ func setupSequencerInboxStub(ctx context.Context, t *testing.T, l1Info *Blockcha
 }
 
 func RunChallengeTest(t *testing.T, asserterIsCorrect bool, useStubs bool, challengeMsgIdx int64, wasmRootDir string) {
-	glogger := log.NewGlogHandler(
-		log.NewTerminalHandler(io.Writer(os.Stderr), false))
-	glogger.Verbosity(log.LvlInfo)
-	log.SetDefault(log.NewLogger(glogger))
-
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 

util/redisutil/test_redis.go

@@ -5,21 +5,25 @@ package redisutil
 
 import (
 	"context"
+	"flag"
 	"fmt"
-	"os"
 	"testing"
 
 	"github.com/alicebob/miniredis/v2"
 
 	"github.com/offchainlabs/nitro/util/testhelpers"
 )
 
-// CreateTestRedis Provides external redis url, this is only done in TEST_REDIS env,
+var (
+	redisFlag = flag.String("TEST_REDIS", "", "Redis URL for testing")
+)
+
+// CreateTestRedis Provides external redis url, this is only done with -TEST_REDIS flag,
 // else creates a new miniredis and returns its url.
 func CreateTestRedis(ctx context.Context, t *testing.T) string {
-	redisUrl := os.Getenv("TEST_REDIS")
-	if redisUrl != "" {
-		return redisUrl
+	flag.Parse()
+	if *redisFlag != "" {
+		return *redisFlag
 	}
 	redisServer, err := miniredis.Run()
 	testhelpers.RequireImpl(t, err)

util/testhelpers/env/env.go

@@ -4,19 +4,22 @@
 package env
 
 import (
-	"os"
+	"flag"
 
 	"github.com/ethereum/go-ethereum/core/rawdb"
 	"github.com/ethereum/go-ethereum/log"
 )
 
+var (
+	stateSchemeFlag = flag.String("TEST_STATE_SCHEME", "", "State scheme to use for tests")
+)
+
 // There are two CI steps, one to run tests using the path state scheme, and one to run tests using the hash state scheme.
 // An environment variable controls that behavior.
 func GetTestStateScheme() string {
-	envTestStateScheme := os.Getenv("TEST_STATE_SCHEME")
 	stateScheme := rawdb.HashScheme
-	if envTestStateScheme == rawdb.PathScheme || envTestStateScheme == rawdb.HashScheme {
-		stateScheme = envTestStateScheme
+	if *stateSchemeFlag == rawdb.PathScheme || *stateSchemeFlag == rawdb.HashScheme {
+		stateScheme = *stateSchemeFlag
 	}
 	log.Debug("test state scheme", "testStateScheme", stateScheme)
 	return stateScheme