diff --git a/op-acceptor/Dockerfile b/op-acceptor/Dockerfile index 2f33cafa..5ec72046 100644 --- a/op-acceptor/Dockerfile +++ b/op-acceptor/Dockerfile @@ -9,7 +9,9 @@ COPY op-acceptor/ . RUN just build -FROM alpine:3.21 +FROM golang:1.23.5-alpine3.21 + +RUN apk add --no-cache ca-certificates build-base git # Install Go binary # (we copy Go directly from the builder stage for simplicity and consistency) @@ -19,6 +21,7 @@ ENV GOPATH="/go" ENV PATH="${GOPATH}/bin:${PATH}" WORKDIR /app + RUN addgroup -S app && adduser -S app -G app && \ mkdir -p /devnets && \ chown -R app:app /devnets @@ -30,7 +33,12 @@ chown -R app:app /go COPY --from=builder /app/bin/op-acceptor /app/ COPY op-acceptor/example-validators.yaml /app/ + +# Set ownership of the /app directory to allow the application to write logs and other files at runtime +RUN chown -R app:app /app/ + USER app VOLUME /devnets + ENTRYPOINT ["/app/op-acceptor"] diff --git a/op-acceptor/config.go b/op-acceptor/config.go index 14b1c377..afbea091 100644 --- a/op-acceptor/config.go +++ b/op-acceptor/config.go @@ -22,8 +22,9 @@ type Config struct { AllowSkips bool // Allow tests to be skipped instead of failing when preconditions are not met DefaultTimeout time.Duration // Default timeout for individual tests, can be overridden by test config LogDir string // Directory to store test logs - - Log log.Logger + OutputTestLogs bool // Whether to output test logs to the console + TestLogLevel string // Log level to be used for the tests + Log log.Logger } // NewConfig creates a new Config instance @@ -72,6 +73,8 @@ func NewConfig(ctx *cli.Context, log log.Logger, testDir string, validatorConfig RunOnce: runOnce, AllowSkips: ctx.Bool(flags.AllowSkips.Name), DefaultTimeout: ctx.Duration(flags.DefaultTimeout.Name), + OutputTestLogs: ctx.Bool(flags.OutputTestLogs.Name), + TestLogLevel: ctx.String(flags.TestLogLevel.Name), LogDir: logDir, Log: log, }, nil diff --git a/op-acceptor/flags/flags.go b/op-acceptor/flags/flags.go index 9a7e1bd7..97121ee3 100644 --- a/op-acceptor/flags/flags.go +++ b/op-acceptor/flags/flags.go @@ -66,6 +66,18 @@ var ( EnvVars: opservice.PrefixEnvVar(EnvVarPrefix, "LOGDIR"), Usage: "Directory to store test logs. Defaults to 'logs' if not specified.", } + TestLogLevel = &cli.StringFlag{ + Name: "test-log-level", + Value: "info", + EnvVars: opservice.PrefixEnvVar(EnvVarPrefix, "TEST_LOG_LEVEL"), + Usage: "Log level to be used for the tests. Defaults to 'info'.", + } + OutputTestLogs = &cli.BoolFlag{ + Name: "output-test-logs", + Value: false, + EnvVars: opservice.PrefixEnvVar(EnvVarPrefix, "OUTPUT_TEST_LOGS"), + Usage: "Output test logs to the console. Defaults to false.", + } ) var requiredFlags = []cli.Flag{ @@ -80,6 +92,8 @@ var optionalFlags = []cli.Flag{ AllowSkips, DefaultTimeout, LogDir, + TestLogLevel, + OutputTestLogs, } var Flags []cli.Flag diff --git a/op-acceptor/nat.go b/op-acceptor/nat.go index f3fed117..816ef86f 100644 --- a/op-acceptor/nat.go +++ b/op-acceptor/nat.go @@ -76,14 +76,16 @@ func New(ctx context.Context, config *Config, version string, shutdownCallback f // Create runner with registry testRunner, err := runner.NewTestRunner(runner.Config{ - Registry: reg, - WorkDir: config.TestDir, - Log: config.Log, - TargetGate: config.TargetGate, - GoBinary: config.GoBinary, - AllowSkips: config.AllowSkips, - NetworkName: networkName, - DevnetEnv: env, + Registry: reg, + WorkDir: config.TestDir, + Log: config.Log, + TargetGate: config.TargetGate, + GoBinary: config.GoBinary, + AllowSkips: config.AllowSkips, + OutputTestLogs: config.OutputTestLogs, + TestLogLevel: config.TestLogLevel, + NetworkName: networkName, + DevnetEnv: env, }) if err != nil { return nil, fmt.Errorf("failed to create test runner: %w", err) diff --git a/op-acceptor/runner/logging_test.go b/op-acceptor/runner/logging_test.go index ad07ae52..c03fbaef 100644 --- a/op-acceptor/runner/logging_test.go +++ b/op-acceptor/runner/logging_test.go @@ -2,6 +2,8 @@ package runner import ( "context" + "os" + "path/filepath" "testing" "github.com/ethereum-optimism/infra/op-acceptor/types" @@ -68,3 +70,133 @@ gates: assert.Contains(t, failingTest.Stdout, "This is some stdout output that should be captured") assert.Contains(t, failingTest.Stdout, "This is a second line of output") } + +// TestLogLevelEnvironment verifies that the TEST_LOG_LEVEL environment variable is correctly set and used +func TestLogLevelEnvironment(t *testing.T) { + ctx := context.Background() + + // Create a simple test file in the work directory + testContent := []byte(` +package main + +import ( + "os" + "testing" +) + +func TestLogLevelEnvironment(t *testing.T) { + // Get log level from environment + logLevel := os.Getenv("TEST_LOG_LEVEL") + if logLevel == "" { + t.Log("TEST_LOG_LEVEL not set") + } else { + t.Log("TEST_LOG_LEVEL set to", logLevel) + } +} +`) + configContent := []byte(` +gates: + - id: logging-gate + description: "Gate with a test that outputs logs" + suites: + logging-suite: + description: "Suite with a test that outputs logs" + tests: + - name: TestLogLevelEnvironment + package: "./main" +`) + + r := setupTestRunner(t, testContent, configContent) + r.testLogLevel = "debug" + err := os.WriteFile(filepath.Join(r.workDir, "main_test.go"), testContent, 0644) + require.NoError(t, err) + + result, err := r.RunTest(ctx, types.ValidatorMetadata{ + ID: "test1", + Gate: "logging-gate", + FuncName: "TestLogLevelEnvironment", + Package: ".", + }) + + require.NoError(t, err) + assert.Equal(t, types.TestStatusPass, result.Status) + assert.Equal(t, "test1", result.Metadata.ID) + assert.Equal(t, "logging-gate", result.Metadata.Gate) + assert.Equal(t, ".", result.Metadata.Package) + assert.False(t, result.Metadata.RunAll) + assert.Contains(t, result.Stdout, "TEST_LOG_LEVEL set to debug") +} + +// TestOutputTestLogs verifies that test logs are properly captured and output when enabled +func TestOutputTestLogs(t *testing.T) { + // Create a test file that outputs logs at different levels + testContent := []byte(` +package feature_test + +import ( + "testing" +) + +func TestWithLogs(t *testing.T) { + t.Log("This is a test output") +} +`) + + configContent := []byte(` +gates: + - id: logging-gate + description: "Gate with a test that outputs logs" + suites: + logging-suite: + description: "Suite with a test that outputs logs" + tests: + - name: TestWithLogs + package: "./feature" +`) + + // Setup the test runner with OutputTestLogs enabled + r := setupTestRunner(t, testContent, configContent) + r.outputTestLogs = true + + // Run the test + result, err := r.RunAllTests(context.Background()) + require.NoError(t, err) + assert.Equal(t, types.TestStatusPass, result.Status) + + // Verify the structure + require.Contains(t, result.Gates, "logging-gate") + gate := result.Gates["logging-gate"] + require.Contains(t, gate.Suites, "logging-suite") + suite := gate.Suites["logging-suite"] + + // Get the test result + var testResult *types.TestResult + for _, test := range suite.Tests { + testResult = test + break + } + require.NotNil(t, testResult) + + // Verify the test passed and has logs captured + assert.Equal(t, types.TestStatusPass, testResult.Status) + assert.NotEmpty(t, testResult.Stdout) + assert.Contains(t, testResult.Stdout, "This is a test output") + + // Now run with OutputTestLogs disabled + r.outputTestLogs = false + result, err = r.RunAllTests(context.Background()) + require.NoError(t, err) + assert.Equal(t, types.TestStatusPass, result.Status) + + // Get the test result again + var testResult2 *types.TestResult + for _, test := range result.Gates["logging-gate"].Suites["logging-suite"].Tests { + testResult2 = test + break + } + require.NotNil(t, testResult) + + // Verify that logs are not captured when OutputTestLogs is disabled + assert.Equal(t, types.TestStatusPass, testResult2.Status) + assert.Contains(t, testResult2.Stdout, "This is a test output") +} diff --git a/op-acceptor/runner/runner.go b/op-acceptor/runner/runner.go index d10a896f..8d702437 100644 --- a/op-acceptor/runner/runner.go +++ b/op-acceptor/runner/runner.go @@ -5,9 +5,11 @@ import ( "context" "encoding/json" "fmt" + "io" "net/url" "os" "os/exec" + "path/filepath" "strings" "time" @@ -92,30 +94,34 @@ type TestRunnerWithFileLogger interface { // runner struct implements TestRunner interface type runner struct { - registry *registry.Registry - validators []types.ValidatorMetadata - workDir string // Directory for running tests - log log.Logger - runID string - goBinary string // Path to the Go binary - allowSkips bool // Whether to allow skipping tests when preconditions are not met - fileLogger *logging.FileLogger // Logger for storing test results - networkName string // Name of the network being tested - env *env.DevnetEnv - tracer trace.Tracer + registry *registry.Registry + validators []types.ValidatorMetadata + workDir string // Directory for running tests + log log.Logger + runID string + goBinary string // Path to the Go binary + allowSkips bool // Whether to allow skipping tests when preconditions are not met + outputTestLogs bool // Whether to output test logs to the console + testLogLevel string // Log level to be used for the tests + fileLogger *logging.FileLogger // Logger for storing test results + networkName string // Name of the network being tested + env *env.DevnetEnv + tracer trace.Tracer } // Config holds configuration for creating a new runner type Config struct { - Registry *registry.Registry - TargetGate string - WorkDir string - Log log.Logger - GoBinary string // path to the Go binary - AllowSkips bool // Whether to allow skipping tests when preconditions are not met - FileLogger *logging.FileLogger // Logger for storing test results - NetworkName string // Name of the network being tested - DevnetEnv *env.DevnetEnv + Registry *registry.Registry + TargetGate string + WorkDir string + Log log.Logger + GoBinary string // path to the Go binary + AllowSkips bool // Whether to allow skipping tests when preconditions are not met + OutputTestLogs bool // Whether to output test logs to the console + TestLogLevel string // Log level to be used for the tests + FileLogger *logging.FileLogger // Logger for storing test results + NetworkName string // Name of the network being tested + DevnetEnv *env.DevnetEnv } // NewTestRunner creates a new test runner instance @@ -155,16 +161,18 @@ func NewTestRunner(cfg Config) (TestRunner, error) { "allowSkips", cfg.AllowSkips, "goBinary", cfg.GoBinary, "networkName", networkName) return &runner{ - registry: cfg.Registry, - validators: validators, - workDir: cfg.WorkDir, - log: cfg.Log, - goBinary: cfg.GoBinary, - allowSkips: cfg.AllowSkips, - fileLogger: cfg.FileLogger, - networkName: networkName, - env: cfg.DevnetEnv, - tracer: otel.Tracer("test runner"), + registry: cfg.Registry, + validators: validators, + workDir: cfg.WorkDir, + log: cfg.Log, + goBinary: cfg.GoBinary, + allowSkips: cfg.AllowSkips, + outputTestLogs: cfg.OutputTestLogs, + testLogLevel: cfg.TestLogLevel, + fileLogger: cfg.FileLogger, + networkName: networkName, + env: cfg.DevnetEnv, + tracer: otel.Tracer("test runner"), }, nil } @@ -351,6 +359,10 @@ func (r *runner) getTestKey(validator types.ValidatorMetadata) string { return validator.FuncName } +func isLocalPath(pkg string) bool { + return strings.HasPrefix(pkg, "./") || strings.HasPrefix(pkg, "/") || strings.HasPrefix(pkg, "../") +} + // RunTest implements the TestRunner interface func (r *runner) RunTest(ctx context.Context, metadata types.ValidatorMetadata) (*types.TestResult, error) { // Use defer and recover to catch panics and convert them to errors @@ -379,7 +391,21 @@ func (r *runner) RunTest(ctx context.Context, metadata types.ValidatorMetadata) } }() + // Check if the path is available locally + if isLocalPath(metadata.Package) { + fullPath := filepath.Join(r.workDir, metadata.Package) + if _, statErr := os.Stat(fullPath); os.IsNotExist(statErr) { + r.log.Error("Local package path does not exist, failing test", "validator", metadata.ID, "package", metadata.Package, "fullPath", fullPath) + return &types.TestResult{ + Metadata: metadata, + Status: types.TestStatusFail, + Error: fmt.Errorf("local package path does not exist: %s", fullPath), + }, nil + } + } + r.log.Info("Running validator", "validator", metadata.ID) + start := time.Now() if metadata.RunAll { result, err = r.runAllTestsInPackage(ctx, metadata) @@ -516,8 +542,20 @@ func (r *runner) runSingleTest(ctx context.Context, metadata types.ValidatorMeta defer cleanup() var stdout, stderr bytes.Buffer - cmd.Stdout = &stdout - cmd.Stderr = &stderr + if r.outputTestLogs { + stdoutLogger := &logWriter{logFn: func(msg string) { + r.log.Info("Test output", "test", metadata.FuncName, "output", msg) + }} + stderrLogger := &logWriter{logFn: func(msg string) { + r.log.Error("Test error output", "test", metadata.FuncName, "error", msg) + }} + + cmd.Stdout = io.MultiWriter(&stdout, stdoutLogger) + cmd.Stderr = io.MultiWriter(&stderr, stderrLogger) + } else { + cmd.Stdout = &stdout + cmd.Stderr = &stderr + } r.log.Info("Running test", "test", metadata.FuncName) r.log.Debug("Running test command", @@ -563,10 +601,12 @@ func (r *runner) runSingleTest(ctx context.Context, metadata types.ValidatorMeta // If we couldn't parse the output for some reason, create a minimal failing result if parsedResult == nil { + r.log.Error("test exited with non-zero exit code", "exitCode", cmd.ProcessState.ExitCode()) parsedResult = &types.TestResult{ Metadata: metadata, Status: types.TestStatusFail, Error: fmt.Errorf("failed to parse test output"), + Stdout: stdout.String(), } } @@ -652,7 +692,9 @@ func (r *runner) parseTestOutput(output []byte, metadata types.ValidatorMetadata "status", result.Status, "subtests", len(result.SubTests), "hasAnyValidEvent", hasAnyValidEvent, - "hasError", result.Error != nil) + "hasError", result.Error != nil, + "error", result.Error, + ) return result } @@ -1144,7 +1186,11 @@ func (r *runner) testCommandContext(ctx context.Context, name string, arg ...str cmd := exec.CommandContext(ctx, name, arg...) cmd.Dir = r.workDir + // Always set the TEST_LOG_LEVEL environment variable + runEnv := append([]string{fmt.Sprintf("TEST_LOG_LEVEL=%s", r.testLogLevel)}, os.Environ()...) + if r.env == nil { + cmd.Env = telemetry.InstrumentEnvironment(ctx, runEnv) return cmd, func() {} } @@ -1161,8 +1207,7 @@ func (r *runner) testCommandContext(ctx context.Context, name string, arg ...str r.log.Error("Failed to write env to temp file", "error", err) } - env := append( - os.Environ(), + runEnv = append(runEnv, // override the env URL with the one from the temp file fmt.Sprintf("%s=%s", env.EnvURLVar, envFile.Name()), // override the control resolution scheme with the original one @@ -1170,7 +1215,7 @@ func (r *runner) testCommandContext(ctx context.Context, name string, arg ...str // Set DEVNET_EXPECT_PRECONDITIONS_MET=true to make tests fail instead of skip when preconditions are not met "DEVNET_EXPECT_PRECONDITIONS_MET=true", ) - cmd.Env = telemetry.InstrumentEnvironment(ctx, env) + cmd.Env = telemetry.InstrumentEnvironment(ctx, runEnv) } cleanup := func() { if envFile != nil { @@ -1184,3 +1229,31 @@ func (r *runner) testCommandContext(ctx context.Context, name string, arg ...str // Make sure the runner type implements both interfaces var _ TestRunner = &runner{} var _ TestRunnerWithFileLogger = &runner{} + +type logWriter struct { + logFn func(msg string) + buf []byte +} + +func (w *logWriter) Write(p []byte) (n int, err error) { + w.buf = append(w.buf, p...) + for { + idx := bytes.IndexByte(w.buf, '\n') + if idx == -1 { + break + } + line := w.buf[:idx] + w.buf = w.buf[idx+1:] + + // Try to parse as a test event + event, err := parseTestEvent(line) + if err == nil && event.Action == ActionOutput { + // If it's a valid test event with output action, use the Output field + w.logFn(event.Output) + } else { + // If not a valid test event or not an output action, use the raw line + w.logFn(string(line)) + } + } + return len(p), nil +} diff --git a/op-acceptor/runner/runner_test.go b/op-acceptor/runner/runner_test.go index 76e92181..289b4464 100644 --- a/op-acceptor/runner/runner_test.go +++ b/op-acceptor/runner/runner_test.go @@ -1583,3 +1583,39 @@ gates: validators := reg.GetValidators() require.NotEmpty(t, validators, "Registry should have validators") } + +func TestRunTest_PackagePath_Local(t *testing.T) { + r := setupDefaultTestRunner(t) + + origPath := os.Getenv("PATH") + defer os.Setenv("PATH", origPath) + + testCases := []struct { + name string + packagePath string + expectStatus types.TestStatus + expectErrMsg string + }{ + { + name: "Local path does not exist", + packagePath: "./does-not-exist", + expectStatus: types.TestStatusFail, + expectErrMsg: "local package path does not exist", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result, err := r.RunTest(context.Background(), types.ValidatorMetadata{ + ID: "test", + Gate: "test-gate", + FuncName: "TestFunc", + Package: tc.packagePath, + }) + require.NoError(t, err) + assert.Equal(t, tc.expectStatus, result.Status) + assert.Error(t, result.Error) + assert.Contains(t, result.Error.Error(), tc.expectErrMsg) + }) + } +}