test: cover stage and executor reliability regressions

cmd/joj3/conf/conf_test.go

@@ -132,14 +132,32 @@ func TestParseConventionalCommit(t *testing.T) {
 }
 
 func TestMatchGroupsUsesExactTokens(t *testing.T) {
-	conf := &Conf{Stages: []ConfStage{
-		{Name: "short", Groups: []string{"c"}},
-		{Name: "cpp", Groups: []string{"cpp"}},
-		{Name: "lint", Groups: []string{"lint"}},
-	}}
-	got := MatchGroups(conf, &ConventionalCommit{Group: "cpp, lint"})
-	want := []string{"cpp", "lint"}
-	if !reflect.DeepEqual(got, want) {
-		t.Fatalf("MatchGroups() = %v, want %v", got, want)
+	tests := []struct {
+		name  string
+		group string
+		want  []string
+	}{
+		{name: "exact not substring", group: "cpp", want: []string{"cpp"}},
+		{name: "case insensitive", group: "CPP", want: []string{"cpp"}},
+		{name: "comma and space", group: "cpp, lint", want: []string{"cpp", "lint"}},
+		{name: "semicolon", group: "cpp;lint", want: []string{"cpp", "lint"}},
+		{name: "pipe", group: "cpp|lint", want: []string{"cpp", "lint"}},
+		{name: "tab", group: "cpp\tlint", want: []string{"cpp", "lint"}},
+		{name: "duplicate token", group: "cpp,cpp", want: []string{"cpp"}},
+		{name: "all", group: "ALL", want: []string{"c", "cpp", "lint"}},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			conf := &Conf{Stages: []ConfStage{
+				{Name: "short", Groups: []string{"c"}},
+				{Name: "cpp", Groups: []string{"cpp"}},
+				{Name: "lint", Groups: []string{"lint"}},
+			}}
+			got := MatchGroups(conf, &ConventionalCommit{Group: tt.group})
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Fatalf("MatchGroups() = %v, want %v", got, tt.want)
+			}
+		})
 	}
 }

cmd/joj3/stage_test.go

@@ -1,6 +1,8 @@
 package main
 
 import (
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 
@@ -25,3 +27,45 @@ func TestRunStagesPreservesMainAndOutputErrors(t *testing.T) {
 		t.Fatalf("runStages() error = %v, want main and output errors", err)
 	}
 }
+
+func TestRunStagesPreservesPhaseErrorsAndWritesMainFailure(t *testing.T) {
+	broken := func(name, executor string) conf.ConfStage {
+		s := conf.ConfStage{Name: name}
+		s.Executor.Name = executor
+		return s
+	}
+	outputPath := filepath.Join(t.TempDir(), "result.json")
+	c := &conf.Conf{
+		SandboxExecServer: "localhost:5051",
+		OutputPath:        outputPath,
+		PreStages:         []conf.ConfStage{broken("pre", "missing-pre")},
+		Stages:            []conf.ConfStage{broken("main", "missing-main")},
+		PostStages:        []conf.ConfStage{broken("post", "missing-post")},
+	}
+	callbackCalled := false
+	_, forceQuit, err := runStages(c, nil, func(results []stage.StageResult, forceQuit string) {
+		callbackCalled = true
+		if len(results) != 1 || results[0].Name != "Internal Error" || forceQuit != "Internal Error" {
+			t.Fatalf("callback results = %v, forceQuit = %q", results, forceQuit)
+		}
+	})
+	if err == nil {
+		t.Fatal("runStages() unexpectedly succeeded")
+	}
+	for _, want := range []string{"missing-pre", "missing-main", "missing-post"} {
+		if !strings.Contains(err.Error(), want) {
+			t.Errorf("runStages() error %q does not contain %q", err, want)
+		}
+	}
+	if forceQuit != "Internal Error" || !callbackCalled {
+		t.Fatalf("forceQuit = %q, callbackCalled = %v", forceQuit, callbackCalled)
+	}
+	content, readErr := os.ReadFile(outputPath)
+	if readErr != nil {
+		t.Fatal(readErr)
+	}
+	if !strings.Contains(string(content), "Internal Error") ||
+		!strings.Contains(string(content), "missing-main") {
+		t.Fatalf("output = %s", content)
+	}
+}

internal/executor/local/executor_test.go

@@ -0,0 +1,52 @@
+package local
+
+import (
+	"context"
+	"errors"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/joint-online-judge/JOJ3/internal/stage"
+)
+
+func TestRunRejectsEmptyArgs(t *testing.T) {
+	_, err := (&Local{}).Run(context.Background(), []stage.Cmd{{}})
+	if err == nil || err.Error() != "command args must not be empty" {
+		t.Fatalf("Run() error = %v", err)
+	}
+}
+
+func TestRunCancellationKillsProcessGroup(t *testing.T) {
+	dir := t.TempDir()
+	ready := filepath.Join(dir, "ready")
+	marker := filepath.Join(dir, "marker")
+	ctx, cancel := context.WithCancel(context.Background())
+	done := make(chan error, 1)
+	go func() {
+		_, err := (&Local{}).Run(ctx, []stage.Cmd{{
+			Args: []string{"/bin/sh", "-c", "touch \"$1\"; (sleep 0.3; touch \"$2\") & wait", "sh", ready, marker},
+		}})
+		done <- err
+	}()
+
+	deadline := time.Now().Add(2 * time.Second)
+	for {
+		if _, err := os.Stat(ready); err == nil {
+			break
+		}
+		if time.Now().After(deadline) {
+			t.Fatal("command did not start")
+		}
+		time.Sleep(10 * time.Millisecond)
+	}
+	cancel()
+	if err := <-done; !errors.Is(err, context.Canceled) {
+		t.Fatalf("Run() error = %v, want context canceled", err)
+	}
+	time.Sleep(500 * time.Millisecond)
+	if _, err := os.Stat(marker); !errors.Is(err, os.ErrNotExist) {
+		t.Fatalf("child process survived cancellation: %v", err)
+	}
+}

internal/executor/sandbox/convert_test.go

@@ -1,6 +1,9 @@
 package sandbox
 
 import (
+	"fmt"
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 
@@ -19,3 +22,50 @@ func TestConvertPBCmdReturnsSourceReadError(t *testing.T) {
 		t.Fatalf("convertPBCmd() error = %v, want source read error", err)
 	}
 }
+
+func TestConvertPBCmdReturnsStandardFileReadError(t *testing.T) {
+	missing := t.TempDir() + "/missing"
+	_, err := convertPBCmd([]stage.Cmd{{
+		Args:  []string{"true"},
+		Stdin: &stage.CmdFile{Src: &missing},
+	}})
+	if err == nil || !strings.Contains(err.Error(), "standard file") ||
+		!strings.Contains(err.Error(), "read source file") {
+		t.Fatalf("convertPBCmd() error = %v, want standard source read error", err)
+	}
+}
+
+func TestConvertPBCmdReturnsCopyInDirectoryWalkError(t *testing.T) {
+	missing := t.TempDir() + "/missing"
+	_, err := convertPBCmd([]stage.Cmd{{
+		Args:      []string{"true"},
+		CopyInDir: missing,
+	}})
+	if err == nil || !strings.Contains(err.Error(), "walk") {
+		t.Fatalf("convertPBCmd() error = %v, want directory walk error", err)
+	}
+}
+
+func TestConvertPBCmdPreservesMoreThanSCMMaxFDFileCount(t *testing.T) {
+	const fileCount = 301
+	dir := t.TempDir()
+	for i := range fileCount {
+		path := filepath.Join(dir, fmt.Sprintf("file-%03d", i))
+		if err := os.WriteFile(path, []byte("content"), 0o600); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	cmds, err := convertPBCmd([]stage.Cmd{{
+		Args:      []string{"true"},
+		CopyIn:    make(map[string]stage.CmdFile),
+		CopyInDir: dir,
+	}})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(cmds) != 1 || len(cmds[0].GetCopyIn()) != fileCount {
+		t.Fatalf("converted %d commands with %d files, want 1 command with %d files",
+			len(cmds), len(cmds[0].GetCopyIn()), fileCount)
+	}
+}

internal/executor/sandbox/executor_test.go

@@ -1,13 +1,36 @@
 package sandbox
 
 import (
+	"context"
+	"errors"
 	"math"
 	"testing"
 	"time"
 
+	"github.com/criyle/go-judge/pb"
 	"github.com/joint-online-judge/JOJ3/internal/stage"
+	"google.golang.org/grpc"
+	"google.golang.org/protobuf/types/known/emptypb"
 )
 
+type fakeExecutorClient struct {
+	pb.ExecutorClient
+	exec       func(context.Context, *pb.Request) (*pb.Response, error)
+	fileDelete func(context.Context, *pb.FileID) (*emptypb.Empty, error)
+}
+
+func (f fakeExecutorClient) Exec(
+	ctx context.Context, req *pb.Request, _ ...grpc.CallOption,
+) (*pb.Response, error) {
+	return f.exec(ctx, req)
+}
+
+func (f fakeExecutorClient) FileDelete(
+	ctx context.Context, id *pb.FileID, _ ...grpc.CallOption,
+) (*emptypb.Empty, error) {
+	return f.fileDelete(ctx, id)
+}
+
 func TestExecRPCTimeout(t *testing.T) {
 	tests := []struct {
 		name string
@@ -25,6 +48,16 @@ func TestExecRPCTimeout(t *testing.T) {
 			cmds: []stage.Cmd{{CPULimit: uint64(time.Minute)}},
 			want: 2*time.Minute + rpcTimeoutMargin,
 		},
+		{
+			name: "clock limit takes precedence over cpu limit",
+			cmds: []stage.Cmd{{ClockLimit: uint64(time.Minute), CPULimit: uint64(10 * time.Minute)}},
+			want: time.Minute + rpcTimeoutMargin,
+		},
+		{
+			name: "cpu multiplication overflow",
+			cmds: []stage.Cmd{{CPULimit: math.MaxUint64}},
+			want: time.Duration(math.MaxInt64),
+		},
 		{
 			name: "duration overflow",
 			cmds: []stage.Cmd{{ClockLimit: math.MaxUint64}},
@@ -40,3 +73,57 @@ func TestExecRPCTimeout(t *testing.T) {
 		})
 	}
 }
+
+func TestRunAppliesComputedRPCDeadline(t *testing.T) {
+	var remaining time.Duration
+	client := fakeExecutorClient{
+		exec: func(ctx context.Context, _ *pb.Request) (*pb.Response, error) {
+			deadline, ok := ctx.Deadline()
+			if !ok {
+				t.Fatal("Exec context has no deadline")
+			}
+			remaining = time.Until(deadline)
+			return &pb.Response{}, nil
+		},
+	}
+	executor := &Sandbox{
+		execClient: client,
+		cachedMap:  make(map[string]string),
+	}
+	want := 2*time.Minute + rpcTimeoutMargin
+	_, err := executor.Run(context.Background(), []stage.Cmd{{
+		Args:     []string{"true"},
+		CPULimit: uint64(time.Minute),
+	}})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if remaining > want || remaining < want-time.Second {
+		t.Fatalf("RPC deadline remaining = %v, want approximately %v", remaining, want)
+	}
+}
+
+func TestCleanupJoinsDeleteErrorsAndClearsCache(t *testing.T) {
+	deleteErr := errors.New("delete failed")
+	deleted := 0
+	client := fakeExecutorClient{
+		fileDelete: func(context.Context, *pb.FileID) (*emptypb.Empty, error) {
+			deleted++
+			return nil, deleteErr
+		},
+	}
+	executor := &Sandbox{
+		execClient: client,
+		cachedMap:  map[string]string{"one": "1", "two": "2"},
+	}
+	err := executor.Cleanup(context.Background())
+	if !errors.Is(err, deleteErr) {
+		t.Fatalf("Cleanup() error = %v, want delete error", err)
+	}
+	if deleted != 2 {
+		t.Fatalf("FileDelete called %d times, want 2", deleted)
+	}
+	if len(executor.cachedMap) != 0 {
+		t.Fatalf("cached files not cleared: %v", executor.cachedMap)
+	}
+}

internal/stage/run_test.go

@@ -2,6 +2,7 @@ package stage
 
 import (
 	"context"
+	"errors"
 	"strings"
 	"testing"
 )
@@ -21,6 +22,10 @@ func (contractTestParser) Run([]ExecutorResult, any) ([]ParserResult, bool, erro
 }
 
 func TestRunRejectsParserResultCountMismatch(t *testing.T) {
+	originalExecutors, originalParsers := executorMap, parserMap
+	executorMap, parserMap = map[string]Executor{}, map[string]Parser{}
+	t.Cleanup(func() { executorMap, parserMap = originalExecutors, originalParsers })
+
 	const executorName = "contract-test-executor"
 	const parserName = "contract-test-parser"
 	RegisterExecutor(executorName, contractTestExecutor{})
@@ -38,3 +43,107 @@ func TestRunRejectsParserResultCountMismatch(t *testing.T) {
 		t.Fatalf("Run() force quit = %q, want test", forceQuit)
 	}
 }
+
+type executorFunc struct {
+	run     func(context.Context, []Cmd) ([]ExecutorResult, error)
+	cleanup func(context.Context) error
+}
+
+func (e executorFunc) Run(ctx context.Context, cmds []Cmd) ([]ExecutorResult, error) {
+	return e.run(ctx, cmds)
+}
+
+func (e executorFunc) Cleanup(ctx context.Context) error {
+	if e.cleanup == nil {
+		return nil
+	}
+	return e.cleanup(ctx)
+}
+
+type parserFunc func([]ExecutorResult, any) ([]ParserResult, bool, error)
+
+func (p parserFunc) Run(results []ExecutorResult, conf any) ([]ParserResult, bool, error) {
+	return p(results, conf)
+}
+
+func isolateRegistries(t *testing.T) {
+	t.Helper()
+	originalExecutors, originalParsers := executorMap, parserMap
+	executorMap, parserMap = map[string]Executor{}, map[string]Parser{}
+	t.Cleanup(func() { executorMap, parserMap = originalExecutors, originalParsers })
+}
+
+func TestCleanupJoinsExecutorErrors(t *testing.T) {
+	isolateRegistries(t)
+	errOne := errors.New("cleanup one")
+	errTwo := errors.New("cleanup two")
+	RegisterExecutor("one", executorFunc{
+		run:     func(context.Context, []Cmd) ([]ExecutorResult, error) { return nil, nil },
+		cleanup: func(context.Context) error { return errOne },
+	})
+	RegisterExecutor("two", executorFunc{
+		run:     func(context.Context, []Cmd) ([]ExecutorResult, error) { return nil, nil },
+		cleanup: func(context.Context) error { return errTwo },
+	})
+
+	err := Cleanup(context.Background())
+	if !errors.Is(err, errOne) || !errors.Is(err, errTwo) {
+		t.Fatalf("Cleanup() error = %v, want both cleanup errors", err)
+	}
+}
+
+func TestRunStopsAfterParserContractViolation(t *testing.T) {
+	isolateRegistries(t)
+	RegisterExecutor("executor", executorFunc{
+		run: func(context.Context, []Cmd) ([]ExecutorResult, error) {
+			return []ExecutorResult{{Status: StatusAccepted}}, nil
+		},
+	})
+	RegisterParser("bad", parserFunc(func([]ExecutorResult, any) ([]ParserResult, bool, error) {
+		return nil, false, nil
+	}))
+	secondCalled := false
+	RegisterParser("second", parserFunc(func([]ExecutorResult, any) ([]ParserResult, bool, error) {
+		secondCalled = true
+		return []ParserResult{{}}, false, nil
+	}))
+
+	_, forceQuit, err := Run(context.Background(), []Stage{{
+		Name:     "contract",
+		Executor: StageExecutor{Name: "executor", Cmds: []Cmd{{}}},
+		Parsers:  []StageParser{{Name: "bad"}, {Name: "second"}},
+	}})
+	if err == nil || forceQuit != "contract" {
+		t.Fatalf("Run() = forceQuit %q, error %v", forceQuit, err)
+	}
+	if secondCalled {
+		t.Fatal("parser after contract violation was called")
+	}
+}
+
+func TestRunSetsForceQuitForMissingComponents(t *testing.T) {
+	tests := []struct {
+		name  string
+		stage Stage
+	}{
+		{name: "executor", stage: Stage{Name: "missing-executor", Executor: StageExecutor{Name: "unknown"}}},
+		{
+			name: "parser",
+			stage: Stage{
+				Name:     "missing-parser",
+				Executor: StageExecutor{Name: "executor", Cmds: []Cmd{{}}},
+				Parsers:  []StageParser{{Name: "unknown"}},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			isolateRegistries(t)
+			RegisterExecutor("executor", contractTestExecutor{})
+			_, forceQuit, err := Run(context.Background(), []Stage{tt.stage})
+			if err == nil || forceQuit != tt.stage.Name {
+				t.Fatalf("Run() = forceQuit %q, error %v", forceQuit, err)
+			}
+		})
+	}
+}