Skip to content

Commit 6b0f7b2

Browse files
fix(op-acceptor): skip RunAll package when all tests excluded by --exclude-gates (#565)
* fix(op-acceptor): skip RunAll package invocation when all tests are excluded When --exclude-gates is used, named tests in excluded gates are added to SkipTests on their package's RunAll validator. If every test in a package is in the skip list, go test is still invoked with -skip, which causes TestMain to run (and any test infrastructure it sets up) even though no tests will execute. On teardown, this infrastructure can hang (e.g. a devnet with active snap sync connections that block p2p server shutdown). Fix: before running a RunAll validator with a non-empty SkipTests list, use "go test -list ." to enumerate the package's test functions cheaply. If every listed function appears in SkipTests, return TestStatusSkip immediately without invoking go test. Adds a regression test that verifies the package is reported as skipped (not failed) when all its tests are covered by the skip filter. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: use go list + file scan instead of go test -list to avoid TestMain go test -list invokes the test binary which calls TestMain. Packages that use presets.DoMain start a devnet in TestMain before m.Run(), so go test -list would itself hang on the teardown we were trying to prevent. Replace with a two-step approach that never runs the test binary: 1. go list -json to resolve the package's test source files (no build) 2. Scan those files with a regex for func Test* declarations This correctly enumerates test functions without triggering TestMain or any test infrastructure. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: exclude TestMain from test function scan TestMain matches the ^func (Test\w+)( regex but is the harness entry point, not a runnable test. Without this exclusion, any package defining TestMain (like reqressyncdisabled) would never be detected as all-skipped because TestMain is not in SkipTests — the pre-check returns false and the devnet starts anyway. Exclude fn == "TestMain" explicitly and add it to the regression test to prevent the same oversight. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude <claude@anthropic.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 348f6c2 commit 6b0f7b2

File tree

2 files changed

+176
-0
lines changed

2 files changed

+176
-0
lines changed

op-acceptor/runner/runner.go

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package runner
22

33
import (
4+
"bufio"
45
"bytes"
56
"context"
67
"encoding/json"
@@ -715,6 +716,24 @@ func (r *runner) RunTest(ctx context.Context, metadata types.ValidatorMetadata)
715716
// runAllTestsInPackage discovers and runs all tests in a package
716717
// Executes the entire package as a single go test process to preserve intra-package parallelism.
717718
func (r *runner) runAllTestsInPackage(ctx context.Context, metadata types.ValidatorMetadata) (*types.TestResult, error) {
719+
// If the validator has a non-empty SkipTests list, check whether every test
720+
// in the package would be skipped. If so, skip the entire invocation to avoid
721+
// spinning up expensive test infrastructure (e.g. devnets) that will never
722+
// actually run any tests and may hang on teardown.
723+
if len(metadata.SkipTests) > 0 {
724+
if all, err := r.allTestsWouldBeSkipped(ctx, metadata); err != nil {
725+
r.log.Warn("Could not determine if all tests are skipped; proceeding normally",
726+
"package", metadata.Package, "err", err)
727+
} else if all {
728+
r.log.Info("All tests in package are excluded by skip filter; skipping package invocation",
729+
"package", metadata.Package, "skipTests", metadata.SkipTests)
730+
return &types.TestResult{
731+
Metadata: metadata,
732+
Status: types.TestStatusSkip,
733+
}, nil
734+
}
735+
}
736+
718737
pkgMeta := metadata
719738
pkgMeta.RunAll = false
720739
pkgMeta.FuncName = ""
@@ -728,6 +747,91 @@ func (r *runner) runAllTestsInPackage(ctx context.Context, metadata types.Valida
728747
return res, err
729748
}
730749

750+
// testFuncPattern matches top-level Go test function declarations. We only
751+
// match Test* here because SkipTests comes from explicit YAML entries which
752+
// are invariably Test* functions; Benchmark* and Fuzz* are not skipped via
753+
// -skip in normal acceptance test runs.
754+
var testFuncPattern = regexp.MustCompile(`^func (Test\w+)\(`)
755+
756+
// allTestsWouldBeSkipped returns true if every top-level Test* function in the
757+
// package source is present in metadata.SkipTests, meaning no tests would
758+
// actually run.
759+
//
760+
// Critically, this must NOT invoke the test binary because TestMain can start
761+
// expensive infrastructure (devnets) that hangs on teardown when 0 tests run.
762+
// Instead we use "go list -json" to resolve the package's test source files
763+
// and scan them with a regex — no compilation or execution required.
764+
func (r *runner) allTestsWouldBeSkipped(ctx context.Context, metadata types.ValidatorMetadata) (bool, error) {
765+
// Resolve test source files via "go list -json" (no build, no test binary).
766+
listCmd, cleanup := r.testCommandContext(ctx, r.goBinary, "list", "-json", metadata.Package)
767+
defer cleanup()
768+
769+
var listOut bytes.Buffer
770+
listCmd.Stdout = &listOut
771+
listCmd.Stderr = io.Discard
772+
773+
if err := listCmd.Run(); err != nil {
774+
return false, fmt.Errorf("go list -json failed for %s: %w", metadata.Package, err)
775+
}
776+
777+
var pkgInfo struct {
778+
Dir string
779+
TestGoFiles []string
780+
XTestGoFiles []string
781+
}
782+
if err := json.Unmarshal(listOut.Bytes(), &pkgInfo); err != nil {
783+
return false, fmt.Errorf("parsing go list output for %s: %w", metadata.Package, err)
784+
}
785+
786+
skipSet := make(map[string]struct{}, len(metadata.SkipTests))
787+
for _, s := range metadata.SkipTests {
788+
skipSet[s] = struct{}{}
789+
}
790+
791+
testFiles := append(pkgInfo.TestGoFiles, pkgInfo.XTestGoFiles...)
792+
foundAny := false
793+
for _, file := range testFiles {
794+
fns, err := scanTestFunctions(filepath.Join(pkgInfo.Dir, file))
795+
if err != nil {
796+
return false, fmt.Errorf("scanning %s: %w", file, err)
797+
}
798+
for _, fn := range fns {
799+
// TestMain is the test harness entry point, not an actual test function.
800+
// The Go testing framework never runs it as a test, so exclude it.
801+
if fn == "TestMain" {
802+
continue
803+
}
804+
foundAny = true
805+
if _, skip := skipSet[fn]; !skip {
806+
return false, nil // at least one test would run
807+
}
808+
}
809+
}
810+
811+
// If no Test* functions were found at all, do not treat that as "all
812+
// skipped" — let the normal execution path handle the empty-package case.
813+
return foundAny, nil
814+
}
815+
816+
// scanTestFunctions opens a Go test source file and returns the names of all
817+
// top-level Test* functions found in it.
818+
func scanTestFunctions(path string) ([]string, error) {
819+
f, err := os.Open(path)
820+
if err != nil {
821+
return nil, err
822+
}
823+
defer f.Close()
824+
825+
var funcs []string
826+
scanner := bufio.NewScanner(f)
827+
for scanner.Scan() {
828+
if m := testFuncPattern.FindStringSubmatch(scanner.Text()); m != nil {
829+
funcs = append(funcs, m[1])
830+
}
831+
}
832+
return funcs, scanner.Err()
833+
}
834+
731835
// runTestList runs a list of tests and aggregates their results
732836
func (r *runner) runTestList(ctx context.Context, metadata types.ValidatorMetadata, testNames []string) (*types.TestResult, error) {
733837
if len(testNames) == 0 {

op-acceptor/runner/runner_test.go

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2354,3 +2354,75 @@ func TestChildPkg(t *testing.T) { t.Log("child ok") }
23542354
assert.True(t, hasParent, "should contain TestParentPkg from parent package")
23552355
assert.True(t, hasChild, "should contain TestChildPkg from child subpackage")
23562356
}
2357+
2358+
// TestRunAllTests_SkipWhenAllTestsExcluded verifies that when every test in a RunAll
2359+
// package appears in the SkipTests list, the package is not invoked at all and the
2360+
// result is reported as skipped. This prevents expensive test infrastructure (like
2361+
// devnets) from starting when no tests would actually run.
2362+
func TestRunAllTests_SkipWhenAllTestsExcluded(t *testing.T) {
2363+
ctx := context.Background()
2364+
2365+
// Package has one test: TestOnlyTest.
2366+
// It is listed in a "quarantine" gate so that exclude-gates sets SkipTests.
2367+
// The "production" gate uses a RunAll validator for the same package.
2368+
configContent := []byte(`
2369+
gates:
2370+
- id: quarantine
2371+
tests:
2372+
- package: ./feature
2373+
name: TestOnlyTest
2374+
- id: production
2375+
tests:
2376+
- package: ./feature
2377+
`)
2378+
// Include a TestMain to verify it is not mistaken for a test function and
2379+
// does not prevent the all-skipped detection from working.
2380+
testContent := []byte(`
2381+
package feature_test
2382+
2383+
import (
2384+
"os"
2385+
"testing"
2386+
)
2387+
2388+
func TestMain(m *testing.M) { os.Exit(m.Run()) }
2389+
2390+
func TestOnlyTest(t *testing.T) { t.Log("only test") }
2391+
`)
2392+
2393+
testDir := t.TempDir()
2394+
initGoModule(t, testDir, "test")
2395+
featureDir := filepath.Join(testDir, "feature")
2396+
require.NoError(t, os.MkdirAll(featureDir, 0755))
2397+
require.NoError(t, os.WriteFile(filepath.Join(featureDir, "example_test.go"), testContent, 0644))
2398+
2399+
validatorConfigPath := filepath.Join(testDir, "validators.yaml")
2400+
require.NoError(t, os.WriteFile(validatorConfigPath, configContent, 0644))
2401+
2402+
reg, err := registry.NewRegistry(registry.Config{
2403+
ValidatorConfigFile: validatorConfigPath,
2404+
ExcludeGates: []string{"quarantine"},
2405+
})
2406+
require.NoError(t, err)
2407+
2408+
r, err := NewTestRunner(Config{
2409+
Registry: reg,
2410+
WorkDir: testDir,
2411+
Log: testlog.Logger(t, slog.LevelDebug),
2412+
})
2413+
require.NoError(t, err)
2414+
2415+
result, err := r.RunAllTests(ctx)
2416+
require.NoError(t, err)
2417+
2418+
// The production gate should have a skipped result for the package (not a failure).
2419+
require.Contains(t, result.Gates, "production")
2420+
gate := result.Gates["production"]
2421+
require.Len(t, gate.Tests, 1)
2422+
var pkgResult *types.TestResult
2423+
for _, v := range gate.Tests {
2424+
pkgResult = v
2425+
}
2426+
require.NotNil(t, pkgResult)
2427+
assert.Equal(t, types.TestStatusSkip, pkgResult.Status, "package with all tests excluded should be reported as skipped, not failed")
2428+
}

0 commit comments

Comments
 (0)