From 092b56fabdfab9132efabb7c20e3dffb5fd47c5a Mon Sep 17 00:00:00 2001 From: haveyaseen Date: Thu, 30 Jul 2026 13:22:32 +0200 Subject: [PATCH 1/2] test(ci): measure cross-package coverage and add unit tests Use -coverpkg over the CI package set in ci:test, check:coverage, and coverage_summary.sh so statement coverage credits packages exercised by tests outside their own test binary. Align coverage_summary with CI by including nodert. Add unit tests for nodert wire protobuf roundtrips, hasher scope keys and remaining hashUncached switch arms, and gowork go.mod replace and workspace planning helpers. --- Taskfile.yml | 9 +- forst/internal/gowork/plan_test.go | 134 ++++++++- .../internal/hasher/hasher_exhaustive_test.go | 165 +++++++++++ .../hasher/scope_key_coverage_test.go | 152 ++++++++++ forst/nodert/pb/wire_pb_test.go | 279 ++++++++++++++++++ forst/scripts/coverage_summary.sh | 3 +- 6 files changed, 738 insertions(+), 4 deletions(-) create mode 100644 forst/internal/hasher/hasher_exhaustive_test.go create mode 100644 forst/internal/hasher/scope_key_coverage_test.go create mode 100644 forst/nodert/pb/wire_pb_test.go diff --git a/Taskfile.yml b/Taskfile.yml index 6773c95b..019c26d7 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -823,7 +823,10 @@ tasks: MIN_COVERAGE: '{{.MIN_COVERAGE | default "72"}}' cmds: - | - test -f profile.cov || go test -coverprofile=profile.cov ./cmd/forst/... ./internal/... -count=1 + if [ ! -f profile.cov ]; then + PKGS=$(go list ./cmd/forst/... ./internal/... ./nodert/... | paste -sd, -) + go test -coverprofile=profile.cov -coverpkg="$PKGS" ./cmd/forst/... ./internal/... ./nodert/... -count=1 + fi - bash scripts/check_coverage_threshold.sh profile.cov {{.MIN_COVERAGE}} test:go:short: @@ -839,7 +842,9 @@ tasks: deps: [build:node-runtime] cmds: - task: build:vscode - - go test -race -covermode atomic -coverprofile=profile.cov -timeout=10m ./cmd/forst/... ./internal/... ./nodert/... + - | + PKGS=$(go list ./cmd/forst/... ./internal/... ./nodert/... | paste -sd, -) + go test -race -covermode atomic -coverpkg="$PKGS" -coverprofile=profile.cov -timeout=10m ./cmd/forst/... ./internal/... ./nodert/... ci:e2e: desc: Run CI E2E suite (runtime examples, node interop, sidecar, providers) diff --git a/forst/internal/gowork/plan_test.go b/forst/internal/gowork/plan_test.go index 11358544..3514a3d2 100644 --- a/forst/internal/gowork/plan_test.go +++ b/forst/internal/gowork/plan_test.go @@ -392,7 +392,7 @@ func TestWriteRunGoMod_absoluteReplaceWhenCrossTree(t *testing.T) { if !strings.Contains(s, want) { t.Fatalf("missing absolute replace forst => %s:\n%s", compilerDir, s) } - if _, err := exec.LookPath("go"); err == nil { + if _, err := exec.LookPath("go"); err == nil { cmd := exec.Command("go", "mod", "tidy") cmd.Dir = sandbox cmd.Env = append(os.Environ(), "GOWORK=off") @@ -401,3 +401,135 @@ func TestWriteRunGoMod_absoluteReplaceWhenCrossTree(t *testing.T) { } } } + +func TestAppendGoModReplaces_appendsAndDedupes(t *testing.T) { + dir := t.TempDir() + pkgDir := filepath.Join(dir, "gen", "api") + if err := os.MkdirAll(pkgDir, 0o755); err != nil { + t.Fatal(err) + } + path := filepath.Join(dir, "go.mod") + if err := os.WriteFile(path, []byte("module example.com/app\n\ngo 1.26.0\n\nreplace demo/api => ./old\n"), 0o644); err != nil { + t.Fatal(err) + } + replaces := []PackageReplace{ + {ImportPath: "demo/api", Dir: pkgDir}, + {ImportPath: "demo/auth", Dir: filepath.Join(dir, "gen", "auth")}, + } + if err := AppendGoModReplaces(path, replaces); err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + s := string(data) + if strings.Contains(s, "replace demo/api =>") && strings.Count(s, "replace demo/api =>") > 1 { + t.Fatalf("duplicate demo/api replace:\n%s", s) + } + if !strings.Contains(s, "replace demo/auth =>") { + t.Fatalf("missing demo/auth replace:\n%s", s) + } +} + +func TestAppendGoModReplaces_emptySliceNoOp(t *testing.T) { + path := filepath.Join(t.TempDir(), "go.mod") + if err := os.WriteFile(path, []byte("module m\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := AppendGoModReplaces(path, nil); err != nil { + t.Fatal(err) + } +} + +func TestAppendGoModReplaces_missingFileErrors(t *testing.T) { + if err := AppendGoModReplaces(filepath.Join(t.TempDir(), "missing.mod"), []PackageReplace{ + {ImportPath: "x", Dir: t.TempDir()}, + }); err == nil { + t.Fatal("expected error for missing go.mod") + } +} + +func TestAppendGoModReplaces_relativePathGetsDotPrefix(t *testing.T) { + dir := t.TempDir() + pkgDir := filepath.Join(dir, "pkg") + if err := os.MkdirAll(pkgDir, 0o755); err != nil { + t.Fatal(err) + } + path := filepath.Join(dir, "go.mod") + if err := os.WriteFile(path, []byte("module m\n\ngo 1.26.0\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := AppendGoModReplaces(path, []PackageReplace{{ImportPath: "demo/pkg", Dir: pkgDir}}); err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(data), "replace demo/pkg => ./pkg") { + t.Fatalf("expected ./pkg prefix:\n%s", data) + } +} + +func TestWorkspaceUseDirs(t *testing.T) { + t.Parallel() + _, err := WorkspaceUseDirs("/root", "/session", ForstRuntimeLink{}) + if err == nil { + t.Fatal("expected error when ReplaceDir empty") + } + uses, err := WorkspaceUseDirs("/root", "/session", ForstRuntimeLink{ReplaceDir: "/forst"}) + if err != nil { + t.Fatal(err) + } + if len(uses) != 2 || uses[0] != "/session" || uses[1] != "/forst" { + t.Fatalf("uses = %#v", uses) + } +} + +func TestWriteGoWork_emptyUseDirsErrors(t *testing.T) { + if err := WriteGoWork(filepath.Join(t.TempDir(), "go.work"), nil); err == nil { + t.Fatal("expected error for empty use dirs") + } +} + +func TestChildEnv_workspaceModeSetsGOWORK(t *testing.T) { + work := filepath.Join(t.TempDir(), "go.work") + env := ChildEnv([]string{"GOWORK=/parent/go.work"}, LinkPlan{ + Mode: LinkWorkspace, + Workspace: work, + }, "/app") + found := false + for _, e := range env { + if e == "GOWORK="+work { + found = true + } + if strings.HasPrefix(e, "GOWORK=") && e != "GOWORK="+work { + t.Fatalf("unexpected GOWORK: %s", e) + } + } + if !found { + t.Fatalf("expected GOWORK=%s in %v", work, env) + } +} + +func TestChildEnv_stripsReadonlyGOFLAGS(t *testing.T) { + env := ChildEnv([]string{"GOFLAGS=-mod=readonly -v"}, LinkPlan{Mode: LinkReplace}, "") + for _, e := range env { + if strings.Contains(e, "-mod=readonly") { + t.Fatalf("readonly GOFLAGS leaked: %s", e) + } + } +} + +func TestGoModReplaceNeedsAbsolute(t *testing.T) { + if !goModReplaceNeedsAbsolute("/private/var/tmp", "/home/example/module") { + t.Fatal("expected absolute replace when sandbox is under /private and target is not") + } + if goModReplaceNeedsAbsolute("/a/b", "relative") { + t.Fatal("relative target should not need absolute") + } + if !goModReplaceNeedsAbsolute("/a/b", "/c/d") { + t.Fatal("expected absolute replace when mod and target roots differ") + } +} diff --git a/forst/internal/hasher/hasher_exhaustive_test.go b/forst/internal/hasher/hasher_exhaustive_test.go new file mode 100644 index 00000000..fff8c85d --- /dev/null +++ b/forst/internal/hasher/hasher_exhaustive_test.go @@ -0,0 +1,165 @@ +package hasher + +import ( + "testing" + + "forst/internal/ast" +) + +// TestHashNode_exhaustiveSwitchCases hits remaining hashUncached switch arms (value + pointer). +func TestHashNode_exhaustiveSwitchCases(t *testing.T) { + t.Parallel() + h := New() + baseStr := ast.TypeString + intType := ast.TypeNode{Ident: ast.TypeInt} + + tests := []struct { + name string + node ast.Node + }{ + {"TypeDefBinaryExpr", ast.TypeDefBinaryExpr{ + Op: ast.TokenBitwiseOr, + Left: ast.TypeDefShapeExpr{Shape: ast.ShapeNode{Fields: map[string]ast.ShapeFieldNode{}}}, + Right: ast.TypeDefShapeExpr{Shape: ast.ShapeNode{Fields: map[string]ast.ShapeFieldNode{ + "x": {Type: &ast.TypeNode{Ident: ast.TypeInt}}, + }}}, + }}, + {"FunctionLiteralNode", ast.FunctionLiteralNode{ + Params: []ast.ParamNode{ast.SimpleParamNode{Ident: ast.Ident{ID: "x"}, Type: intType}}, + Body: []ast.Node{ast.ReturnNode{Values: []ast.ExpressionNode{ast.IntLiteralNode{Value: 1}}}}, + }}, + {"FunctionCallNode_callee", ast.FunctionCallNode{ + Callee: ast.FunctionLiteralNode{Body: []ast.Node{}}, + Arguments: []ast.ExpressionNode{ast.IntLiteralNode{Value: 1}}, + }}, + {"FunctionCallNode_ptr", &ast.FunctionCallNode{ + Function: ast.Ident{ID: "f"}, Arguments: []ast.ExpressionNode{}, + }}, + {"MethodCallNode", ast.MethodCallNode{ + Receiver: ast.VariableNode{Ident: ast.Ident{ID: "r"}}, + Method: ast.Ident{ID: "M"}, + Arguments: []ast.ExpressionNode{ + ast.StringLiteralNode{Value: "a"}, + }, + }}, + {"MethodCallNode_ptr", &ast.MethodCallNode{ + Receiver: ast.VariableNode{Ident: ast.Ident{ID: "r"}}, Method: ast.Ident{ID: "M"}, + }}, + {"IndexExpressionNode", ast.IndexExpressionNode{ + Target: ast.VariableNode{Ident: ast.Ident{ID: "xs"}}, Index: ast.IntLiteralNode{Value: 0}, + }}, + {"SliceExpressionNode", ast.SliceExpressionNode{Target: ast.VariableNode{Ident: ast.Ident{ID: "s"}}}}, + {"SpreadExpressionNode", ast.SpreadExpressionNode{Expr: ast.VariableNode{Ident: ast.Ident{ID: "a"}}}}, + {"FieldAccessNode", ast.FieldAccessNode{ + Target: ast.VariableNode{Ident: ast.Ident{ID: "o"}}, Field: ast.Ident{ID: "f"}, + }}, + {"TypeExpressionNode", ast.TypeExpressionNode{Type: intType}}, + {"ConstGroupNode", ast.ConstGroupNode{ + Specs: []ast.ConstSpec{ + {Name: ast.Ident{ID: "A"}, Value: ast.IntLiteralNode{Value: 1}}, + {Name: ast.Ident{ID: "B"}, Value: ast.IotaLiteralNode{}}, + }, + }}, + {"IotaLiteralNode", ast.IotaLiteralNode{}}, + {"ReferenceNode", ast.ReferenceNode{Value: ast.VariableNode{Ident: ast.Ident{ID: "p"}}}}, + {"ReferenceNode_ptr", &ast.ReferenceNode{Value: ast.VariableNode{Ident: ast.Ident{ID: "p"}}}}, + {"UseNode", ast.UseNode{ + Ident: &ast.Ident{ID: "ctx"}, ContractType: ast.TypeNode{Ident: "Context"}, + }}, + {"WithNode", ast.WithNode{ + Wiring: ast.ShapeNode{Fields: map[string]ast.ShapeFieldNode{}}, + Body: []ast.Node{ast.IntLiteralNode{Value: 1}}, + }}, + {"ArrayLiteralNode", ast.ArrayLiteralNode{ + Type: intType, + Value: []ast.ExpressionNode{ast.IntLiteralNode{Value: 1}, ast.IntLiteralNode{Value: 2}}, + }}, + {"ArrayLiteralNode_ptr", &ast.ArrayLiteralNode{Value: []ast.ExpressionNode{ast.IntLiteralNode{Value: 3}}}}, + {"DereferenceNode", ast.DereferenceNode{Value: ast.VariableNode{Ident: ast.Ident{ID: "p"}}}}, + {"NilLiteralNode", ast.NilLiteralNode{}}, + {"SwitchNode", ast.SwitchNode{ + Init: ast.AssignmentNode{ + IsShort: true, + LValues: []ast.ExpressionNode{ast.VariableNode{Ident: ast.Ident{ID: "x"}}}, + RValues: []ast.ExpressionNode{ast.IntLiteralNode{Value: 1}}, + }, + Tag: ast.VariableNode{Ident: ast.Ident{ID: "x"}}, + Clauses: []ast.SwitchClauseNode{ + {Values: []ast.ExpressionNode{ast.IntLiteralNode{Value: 1}}, Body: []ast.Node{ast.FallthroughNode{}}}, + }, + }}, + {"SwitchNode_ptr", func() ast.Node { + sw := ast.SwitchNode{ + Tag: ast.VariableNode{Ident: ast.Ident{ID: "x"}}, + Clauses: []ast.SwitchClauseNode{ + {Body: []ast.Node{&ast.GotoNode{Label: &ast.Ident{ID: "L"}}}}, + }, + } + return &sw + }()}, + {"FallthroughNode", ast.FallthroughNode{}}, + {"GotoNode", &ast.GotoNode{Label: &ast.Ident{ID: "loop"}}}, + {"LabeledStmtNode", &ast.LabeledStmtNode{ + Label: &ast.Ident{ID: "L"}, + Stmt: ast.ReturnNode{Values: []ast.ExpressionNode{ast.IntLiteralNode{Value: 0}}}, + }}, + {"FloatLiteralNode_ptr", &ast.FloatLiteralNode{Value: 1.5}}, + {"RuneLiteralNode_ptr", &ast.RuneLiteralNode{Value: int64('x')}}, + {"StringLiteralNode_ptr", &ast.StringLiteralNode{Value: "s"}}, + {"BoolLiteralNode_ptr", &ast.BoolLiteralNode{Value: true}}, + {"IntLiteralNode_ptr", &ast.IntLiteralNode{Value: 7}}, + {"DestructuredParamNode", ast.DestructuredParamNode{ + Fields: []string{"a", "b"}, Type: intType, + }}, + {"EnsureBlockNode_ptr", &ast.EnsureBlockNode{Body: []ast.Node{ast.IntLiteralNode{Value: 2}}}}, + {"ShapeNode_full", ast.ShapeNode{Fields: map[string]ast.ShapeFieldNode{ + "name": {Type: &ast.TypeNode{Ident: ast.TypeString}}, + "nested": {Shape: &ast.ShapeNode{Fields: map[string]ast.ShapeFieldNode{ + "x": {Type: &ast.TypeNode{Ident: ast.TypeInt}}, + }}}, + }}}, + {"ShapeNode_ptr", &ast.ShapeNode{Fields: map[string]ast.ShapeFieldNode{ + "k": {Type: &ast.TypeNode{Ident: ast.TypeInt}}, + }}}, + {"ShapeFieldNode_shape", ast.ShapeFieldNode{Shape: &ast.ShapeNode{Fields: map[string]ast.ShapeFieldNode{}}}}, + {"AssertionNode_full", ast.AssertionNode{ + BaseType: &baseStr, + Constraints: []ast.ConstraintNode{ + {Name: "Max", Args: []ast.ConstraintArgumentNode{{Shape: &ast.ShapeNode{}}}}, + }, + }}, + {"ConstraintNode", ast.ConstraintNode{Name: "NonEmpty"}}, + {"TypeDefShapeExpr", ast.TypeDefShapeExpr{Shape: ast.ShapeNode{Fields: map[string]ast.ShapeFieldNode{ + "id": {Type: &ast.TypeNode{Ident: ast.TypeInt}}, + }}}}, + {"FunctionNode_receiver", ast.FunctionNode{ + Ident: ast.Ident{ID: "meth"}, + Receiver: &ast.SimpleParamNode{ + Ident: ast.Ident{ID: "s"}, Type: ast.TypeNode{Ident: "S"}, + }, + ReturnTypes: []ast.TypeNode{intType}, + Body: []ast.Node{}, + }}, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got, err := h.HashNode(tt.node) + if err != nil { + t.Fatal(err) + } + if got == 0 { + t.Fatal("zero hash") + } + got2, err := h.HashNode(tt.node) + if err != nil { + t.Fatal(err) + } + if got != got2 { + t.Fatalf("not deterministic: %v vs %v", got, got2) + } + }) + } +} diff --git a/forst/internal/hasher/scope_key_coverage_test.go b/forst/internal/hasher/scope_key_coverage_test.go new file mode 100644 index 00000000..f10caed6 --- /dev/null +++ b/forst/internal/hasher/scope_key_coverage_test.go @@ -0,0 +1,152 @@ +package hasher + +import ( + "testing" + + "forst/internal/ast" +) + +func TestHashScopeKey_allScopeOwningCases(t *testing.T) { + t.Parallel() + h := New() + assertion := ast.AssertionNode{Constraints: []ast.ConstraintNode{{Name: "Min"}}} + errMsg := ast.EnsureErrorCall{ErrorType: "E", ErrorArgs: []ast.ExpressionNode{ast.IntLiteralNode{Value: 1}}} + var errNode ast.EnsureErrorNode = errMsg + receiver := ast.SimpleParamNode{Ident: ast.Ident{ID: "self"}, Type: ast.TypeNode{Ident: ast.TypeString}} + retType := ast.TypeNode{Ident: ast.TypeInt} + + fn := ast.FunctionNode{ + Ident: ast.Ident{ID: "f"}, + Receiver: &receiver, + Params: []ast.ParamNode{ast.SimpleParamNode{Ident: ast.Ident{ID: "x"}, Type: ast.TypeNode{Ident: ast.TypeInt}}}, + ReturnTypes: []ast.TypeNode{retType}, + Body: []ast.Node{ast.ReturnNode{Values: []ast.ExpressionNode{ast.IntLiteralNode{Value: 0}}}}, + } + fnPtr := &fn + var nilFn *ast.FunctionNode + + ensure := ast.EnsureNode{ + Variable: ast.VariableNode{Ident: ast.Ident{ID: "v"}}, + Assertion: assertion, + Error: &errNode, + } + ensurePtr := &ensure + var nilEnsure *ast.EnsureNode + + block := ast.EnsureBlockNode{Body: []ast.Node{ast.IntLiteralNode{Value: 1}}} + blockPtr := &block + var nilBlock *ast.EnsureBlockNode + + tg := ast.TypeGuardNode{ + Ident: "G", + Subject: ast.DestructuredParamNode{ + Fields: []string{"z", "a"}, + Type: ast.TypeNode{Ident: ast.TypeInt}, + }, + Params: []ast.ParamNode{ + ast.SimpleParamNode{Ident: ast.Ident{ID: "b"}, Type: ast.TypeNode{Ident: ast.TypeInt}}, + ast.DestructuredParamNode{Fields: []string{"c"}, Type: ast.TypeNode{Ident: ast.TypeString}}, + }, + Body: []ast.Node{ast.ReturnNode{Values: []ast.ExpressionNode{ast.BoolLiteralNode{Value: true}}}}, + } + tgPtr := &tg + var nilTG *ast.TypeGuardNode + + lit := ast.FunctionLiteralNode{ + Params: []ast.ParamNode{ast.SimpleParamNode{Ident: ast.Ident{ID: "y"}, Type: ast.TypeNode{Ident: ast.TypeBool}}}, + ReturnTypes: []ast.TypeNode{ast.TypeNode{Ident: ast.TypeBool}}, + Body: []ast.Node{ast.ReturnNode{Values: []ast.ExpressionNode{ast.BoolLiteralNode{Value: false}}}}, + } + litPtr := &lit + var nilLit *ast.FunctionLiteralNode + + cases := []struct { + name string + node ast.Node + }{ + {"FunctionNode_value", fn}, + {"FunctionNode_ptr", fnPtr}, + {"FunctionNode_nil_ptr", nilFn}, + {"EnsureNode_value", ensure}, + {"EnsureNode_ptr", ensurePtr}, + {"EnsureNode_nil_ptr", nilEnsure}, + {"EnsureBlockNode_value", block}, + {"EnsureBlockNode_ptr", blockPtr}, + {"EnsureBlockNode_nil_ptr", nilBlock}, + {"TypeGuardNode_value", tg}, + {"TypeGuardNode_ptr", tgPtr}, + {"TypeGuardNode_nil_ptr", nilTG}, + {"FunctionLiteralNode_value", lit}, + {"FunctionLiteralNode_ptr", litPtr}, + {"FunctionLiteralNode_nil_ptr", nilLit}, + {"default_VariableNode", ast.VariableNode{Ident: ast.Ident{ID: "x"}}}, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got, err := h.HashScopeKey(tc.node) + if err != nil { + t.Fatal(err) + } + got2, err := h.HashScopeKey(tc.node) + if err != nil { + t.Fatal(err) + } + if got != got2 { + t.Fatalf("not deterministic: %x vs %x", got, got2) + } + if tc.node != nil && got == 0 && got != NodeHash(NilHash) { + t.Fatalf("unexpected zero hash for %s", tc.name) + } + }) + } + + fnOther := fn + fnOther.Ident = ast.Ident{ID: "g"} + hOther, err := h.HashScopeKey(fnOther) + if err != nil { + t.Fatal(err) + } + hFn, err := h.HashScopeKey(fn) + if err != nil { + t.Fatal(err) + } + if hOther == hFn { + t.Fatal("distinct functions must not share scope key") + } +} + +func TestHashScopeKeyDisambiguated_mixesIdentity(t *testing.T) { + t.Parallel() + h := New() + fn := &ast.FunctionNode{Ident: ast.Ident{ID: "f"}} + base, err := h.HashScopeKey(fn) + if err != nil { + t.Fatal(err) + } + dis, err := h.HashScopeKeyDisambiguated(fn, base) + if err != nil { + t.Fatal(err) + } + if dis == base { + t.Fatal("disambiguated key should differ from base when node has identity") + } + dis2, err := h.HashScopeKeyDisambiguated(fn, base) + if err != nil { + t.Fatal(err) + } + if dis != dis2 { + t.Fatal("disambiguated key not deterministic") + } + + fn2 := &ast.FunctionNode{Ident: ast.Ident{ID: "f"}} + disOther, err := h.HashScopeKeyDisambiguated(fn2, base) + if err != nil { + t.Fatal(err) + } + if dis == disOther { + t.Fatal("distinct nodes with same base should disambiguate differently") + } +} diff --git a/forst/nodert/pb/wire_pb_test.go b/forst/nodert/pb/wire_pb_test.go new file mode 100644 index 00000000..298a79f6 --- /dev/null +++ b/forst/nodert/pb/wire_pb_test.go @@ -0,0 +1,279 @@ +package pb + +import ( + "fmt" + "testing" + + "google.golang.org/protobuf/proto" +) + +func TestWireMessages_marshalUnmarshalRoundtrip(t *testing.T) { + t.Parallel() + + t.Run("Frame_request_body", func(t *testing.T) { + t.Parallel() + in := &Frame{ + Id: 42, + Body: &Frame_Request{ + Request: &WireRequest{ + Method: "initialize", + PayloadJson: []byte(`{"v":1}`), + }, + }, + } + roundtripFrame(t, in) + }) + + t.Run("Frame_response_body", func(t *testing.T) { + t.Parallel() + in := &Frame{ + Id: 7, + Body: &Frame_Response{ + Response: &WireResponse{ + Result: &WireResponse_OkJson{OkJson: []byte(`{"ok":true}`)}, + }, + }, + } + roundtripFrame(t, in) + }) + + t.Run("WireRequest", func(t *testing.T) { + t.Parallel() + in := &WireRequest{Method: "ping", PayloadJson: []byte("null")} + out := roundtrip(t, in, &WireRequest{}) + if out.GetMethod() != in.Method || string(out.GetPayloadJson()) != string(in.PayloadJson) { + t.Fatalf("roundtrip mismatch: %+v vs %+v", in, out) + } + }) + + t.Run("WireResponse_ok_json", func(t *testing.T) { + t.Parallel() + in := &WireResponse{Result: &WireResponse_OkJson{OkJson: []byte(`[1,2]`)}} + out := roundtrip(t, in, &WireResponse{}) + if string(out.GetOkJson()) != string(in.GetOkJson()) || out.GetErr() != nil { + t.Fatalf("ok roundtrip mismatch: %+v", out) + } + }) + + t.Run("WireResponse_err", func(t *testing.T) { + t.Parallel() + in := &WireResponse{ + Result: &WireResponse_Err{ + Err: &ErrorDetail{Code: 9, Message: "boom", DataJson: []byte(`{"x":1}`)}, + }, + } + out := roundtrip(t, in, &WireResponse{}) + errDetail := out.GetErr() + if errDetail == nil || errDetail.Code != 9 || errDetail.Message != "boom" { + t.Fatalf("err roundtrip mismatch: %+v", out) + } + }) + + t.Run("ErrorDetail", func(t *testing.T) { + t.Parallel() + in := &ErrorDetail{Code: 1, Message: "m", DataJson: []byte("{}")} + out := roundtrip(t, in, &ErrorDetail{}) + if out.GetCode() != 1 || out.GetMessage() != "m" || string(out.GetDataJson()) != "{}" { + t.Fatalf("ErrorDetail roundtrip mismatch: %+v", out) + } + }) + + t.Run("InitializeResult", func(t *testing.T) { + t.Parallel() + in := &InitializeResult{Protocol: "forst-node-v2"} + out := roundtrip(t, in, &InitializeResult{}) + if out.GetProtocol() != in.Protocol { + t.Fatalf("InitializeResult roundtrip mismatch: %+v", out) + } + }) + + t.Run("GenNextBatchResult", func(t *testing.T) { + t.Parallel() + in := &GenNextBatchResult{ + Steps: []*GenStepWire{ + {Kind: "log", Message: "hi", ValueJson: []byte("1"), DataJson: []byte("2")}, + }, + } + out := roundtrip(t, in, &GenNextBatchResult{}) + steps := out.GetSteps() + if len(steps) != 1 || steps[0].GetKind() != "log" || steps[0].GetMessage() != "hi" { + t.Fatalf("GenNextBatchResult roundtrip mismatch: %+v", out) + } + }) + + t.Run("GenStepWire", func(t *testing.T) { + t.Parallel() + in := &GenStepWire{Kind: "step", ValueJson: []byte("v"), Message: "msg", DataJson: []byte("d")} + out := roundtrip(t, in, &GenStepWire{}) + if out.GetKind() != in.Kind || out.GetMessage() != in.Message { + t.Fatalf("GenStepWire roundtrip mismatch: %+v", out) + } + }) +} + +func TestWireMessages_protoReflectAndGetters(t *testing.T) { + t.Parallel() + + assertProtoMethods := func(t *testing.T, msg proto.Message) { + t.Helper() + if fmt.Sprint(msg) == "" { + t.Fatal("String() returned empty") + } + if msg.ProtoReflect().Descriptor().ParentFile().Path() == "" { + t.Fatal("Descriptor path empty") + } + if d, _ := msg.(interface{ Descriptor() ([]byte, []int) }).Descriptor(); len(d) == 0 { + t.Fatal("Descriptor() returned empty bytes") + } + msg.ProtoReflect() + } + + t.Run("populated", func(t *testing.T) { + t.Parallel() + frame := &Frame{ + Id: 1, + Body: &Frame_Request{Request: &WireRequest{Method: "m", PayloadJson: []byte("p")}}, + } + assertProtoMethods(t, frame) + if frame.GetId() != 1 || frame.GetRequest().GetMethod() != "m" || frame.GetResponse() != nil { + t.Fatalf("Frame getters: id=%d req=%v resp=%v", frame.GetId(), frame.GetRequest(), frame.GetResponse()) + } + + respFrame := &Frame{Body: &Frame_Response{Response: &WireResponse{Result: &WireResponse_Err{Err: &ErrorDetail{Code: 2}}}}} + if respFrame.GetRequest() != nil || respFrame.GetResponse().GetErr().GetCode() != 2 { + t.Fatalf("Frame response getters mismatch") + } + + req := &WireRequest{Method: "x", PayloadJson: []byte("y")} + assertProtoMethods(t, req) + if req.GetMethod() != "x" || string(req.GetPayloadJson()) != "y" { + t.Fatal("WireRequest getters") + } + + okResp := &WireResponse{Result: &WireResponse_OkJson{OkJson: []byte("ok")}} + assertProtoMethods(t, okResp) + if string(okResp.GetOkJson()) != "ok" || okResp.GetErr() != nil { + t.Fatal("WireResponse ok getters") + } + + errResp := &WireResponse{Result: &WireResponse_Err{Err: &ErrorDetail{Message: "e"}}} + if errResp.GetOkJson() != nil || errResp.GetErr().GetMessage() != "e" { + t.Fatal("WireResponse err getters") + } + + errDetail := &ErrorDetail{Code: 3, Message: "m", DataJson: []byte("d")} + assertProtoMethods(t, errDetail) + if errDetail.GetCode() != 3 || errDetail.GetMessage() != "m" || string(errDetail.GetDataJson()) != "d" { + t.Fatal("ErrorDetail getters") + } + + init := &InitializeResult{Protocol: "p"} + assertProtoMethods(t, init) + if init.GetProtocol() != "p" { + t.Fatal("InitializeResult getters") + } + + batch := &GenNextBatchResult{Steps: []*GenStepWire{{Kind: "k"}}} + assertProtoMethods(t, batch) + if len(batch.GetSteps()) != 1 { + t.Fatal("GenNextBatchResult getters") + } + + step := &GenStepWire{Kind: "k", ValueJson: []byte("v"), Message: "m", DataJson: []byte("d")} + assertProtoMethods(t, step) + if step.GetKind() != "k" || step.GetMessage() != "m" { + t.Fatal("GenStepWire getters") + } + }) + + t.Run("nil_receivers", func(t *testing.T) { + t.Parallel() + var frame *Frame + if frame.GetId() != 0 || frame.GetBody() != nil || frame.GetRequest() != nil || frame.GetResponse() != nil { + t.Fatal("nil Frame getters") + } + _ = (*Frame)(nil).ProtoReflect() + + var req *WireRequest + if req.GetMethod() != "" || req.GetPayloadJson() != nil { + t.Fatal("nil WireRequest getters") + } + + var resp *WireResponse + if resp.GetResult() != nil || resp.GetOkJson() != nil || resp.GetErr() != nil { + t.Fatal("nil WireResponse getters") + } + + var errDetail *ErrorDetail + if errDetail.GetCode() != 0 || errDetail.GetMessage() != "" || errDetail.GetDataJson() != nil { + t.Fatal("nil ErrorDetail getters") + } + + var init *InitializeResult + if init.GetProtocol() != "" { + t.Fatal("nil InitializeResult getters") + } + + var batch *GenNextBatchResult + if batch.GetSteps() != nil { + t.Fatal("nil GenNextBatchResult getters") + } + + var step *GenStepWire + if step.GetKind() != "" || step.GetValueJson() != nil || step.GetMessage() != "" || step.GetDataJson() != nil { + t.Fatal("nil GenStepWire getters") + } + }) + + t.Run("Reset_clears_fields", func(t *testing.T) { + t.Parallel() + frame := &Frame{ + Id: 99, + Body: &Frame_Request{Request: &WireRequest{Method: "m"}}, + } + frame.Reset() + if frame.Id != 0 || frame.Body != nil { + t.Fatalf("Frame.Reset: %+v", frame) + } + + req := &WireRequest{Method: "m", PayloadJson: []byte("x")} + req.Reset() + if req.Method != "" || req.PayloadJson != nil { + t.Fatalf("WireRequest.Reset: %+v", req) + } + }) +} + +func roundtripFrame(t *testing.T, in *Frame) { + t.Helper() + out := roundtrip(t, in, &Frame{}) + if out.Id != in.Id { + t.Fatalf("Frame id: got %d want %d", out.Id, in.Id) + } + switch inBody := in.Body.(type) { + case *Frame_Request: + got := out.GetRequest() + if got == nil || got.Method != inBody.Request.Method { + t.Fatalf("Frame request roundtrip: %+v", out) + } + case *Frame_Response: + got := out.GetResponse() + if got == nil { + t.Fatalf("Frame response roundtrip missing body: %+v", out) + } + default: + t.Fatalf("unexpected frame body type %T", in.Body) + } +} + +func roundtrip[T proto.Message](t *testing.T, in T, out T) T { + t.Helper() + data, err := proto.Marshal(in) + if err != nil { + t.Fatalf("marshal: %v", err) + } + if err := proto.Unmarshal(data, out); err != nil { + t.Fatalf("unmarshal: %v", err) + } + return out +} diff --git a/forst/scripts/coverage_summary.sh b/forst/scripts/coverage_summary.sh index 68347702..6556f846 100644 --- a/forst/scripts/coverage_summary.sh +++ b/forst/scripts/coverage_summary.sh @@ -4,7 +4,8 @@ # Optional: MIN_COVERAGE=80 exits 1 if total is below the threshold. set -euo pipefail cd "$(dirname "$0")/.." -go test -coverprofile=profile.cov ./cmd/forst/... ./internal/... -count=1 >/dev/null +PKGS=$(go list ./cmd/forst/... ./internal/... ./nodert/... | paste -sd, -) +go test -coverprofile=profile.cov -coverpkg="$PKGS" ./cmd/forst/... ./internal/... ./nodert/... -count=1 >/dev/null total_line=$(go tool cover -func=profile.cov | tail -1) echo "$total_line" pct=$(echo "$total_line" | grep -oE '[0-9]+\.[0-9]+%' | head -1 | tr -d '%') From 15bd3491c0615c485158ff900b885333285f9fcf Mon Sep 17 00:00:00 2001 From: haveyaseen Date: Fri, 31 Jul 2026 10:23:25 +0200 Subject: [PATCH 2/2] fix(lsp): make build metadata thread-safe under parallel tests Replace mutable Version/Commit/Date package globals with mutex-protected SetBuildMetadata and snapshot reads. Initialize, health, and version helpers now read metadata through the snapshot so parallel tests under -race no longer race with handleInitialize. main.go sets LSP metadata via SetBuildMetadata; tests restore prior values with t.Cleanup instead of writing globals directly. --- forst/cmd/forst/lsp/health.go | 7 +++-- forst/cmd/forst/lsp/initialize.go | 8 +++-- forst/cmd/forst/lsp/server.go | 10 ------ forst/cmd/forst/lsp/version.go | 47 +++++++++++++++++++++++++---- forst/cmd/forst/lsp/version_test.go | 18 ++++------- forst/cmd/forst/main.go | 8 ++--- forst/cmd/forst/main_test.go | 24 +++++---------- 7 files changed, 65 insertions(+), 57 deletions(-) diff --git a/forst/cmd/forst/lsp/health.go b/forst/cmd/forst/lsp/health.go index 5d5370bf..bfe8d793 100644 --- a/forst/cmd/forst/lsp/health.go +++ b/forst/cmd/forst/lsp/health.go @@ -16,12 +16,13 @@ func (s *LSPServer) handleHealth(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) + meta := buildMetadataSnapshot() response := map[string]any{ "status": "healthy", "service": "forst-lsp", - "version": Version, - "commit": Commit, - "date": Date, + "version": meta.version, + "commit": meta.commit, + "date": meta.date, "timestamp": time.Now().UTC().Format(time.RFC3339), } diff --git a/forst/cmd/forst/lsp/initialize.go b/forst/cmd/forst/lsp/initialize.go index 2db1792b..84711c3a 100644 --- a/forst/cmd/forst/lsp/initialize.go +++ b/forst/cmd/forst/lsp/initialize.go @@ -43,6 +43,8 @@ func (s *LSPServer) handleInitialize(request LSPRequest) LSPServerResponse { }, } + meta := buildMetadataSnapshot() + return LSPServerResponse{ JSONRPC: "2.0", ID: request.ID, @@ -50,9 +52,9 @@ func (s *LSPServer) handleInitialize(request LSPRequest) LSPServerResponse { "capabilities": capabilities, "serverInfo": map[string]any{ "name": "forst-lsp", - "version": Version, - "commit": Commit, - "date": Date, + "version": meta.version, + "commit": meta.commit, + "date": meta.date, }, }, } diff --git a/forst/cmd/forst/lsp/server.go b/forst/cmd/forst/lsp/server.go index 6d1f3608..0f9e40bf 100644 --- a/forst/cmd/forst/lsp/server.go +++ b/forst/cmd/forst/lsp/server.go @@ -76,16 +76,6 @@ type peerAnalysisCacheEntry struct { ctx *forstDocumentContext } -// Version information for LSP server -var ( - // Version is the current version of Forst - Version = "dev" - // Commit is the git commit hash - Commit = "unknown" - // Date is the build date - Date = "unknown" -) - // NewLSPServer creates a new LSP server func NewLSPServer(port string, log *logrus.Logger) *LSPServer { debugger := NewCompilerDebugger(true) diff --git a/forst/cmd/forst/lsp/version.go b/forst/cmd/forst/lsp/version.go index 1688858d..83a5af0a 100644 --- a/forst/cmd/forst/lsp/version.go +++ b/forst/cmd/forst/lsp/version.go @@ -1,18 +1,52 @@ package lsp -import "github.com/sirupsen/logrus" +import ( + "sync" + + "github.com/sirupsen/logrus" +) + +type buildMetadata struct { + version string + commit string + date string +} + +var ( + buildInfoMu sync.RWMutex + buildInfo = buildMetadata{ + version: "dev", + commit: "unknown", + date: "unknown", + } +) + +// SetBuildMetadata sets injected compiler build metadata (version, commit, date). +func SetBuildMetadata(version, commit, date string) { + buildInfoMu.Lock() + buildInfo = buildMetadata{version: version, commit: commit, date: date} + buildInfoMu.Unlock() +} + +func buildMetadataSnapshot() buildMetadata { + buildInfoMu.RLock() + defer buildInfoMu.RUnlock() + return buildInfo +} // BuildInfo returns injected compiler build metadata (version, commit, date). func BuildInfo() (version, commit, date string) { - return Version, Commit, Date + meta := buildMetadataSnapshot() + return meta.version, meta.commit, meta.date } // BuildInfoMap returns build metadata as a JSON-friendly map. func BuildInfoMap() map[string]string { + meta := buildMetadataSnapshot() return map[string]string{ - "version": Version, - "commit": Commit, - "date": Date, + "version": meta.version, + "commit": meta.commit, + "date": meta.date, } } @@ -21,5 +55,6 @@ func LogBuildInfo(log *logrus.Logger) { if log == nil { return } - log.Infof("forst %s %s %s", Version, Commit, Date) + meta := buildMetadataSnapshot() + log.Infof("forst %s %s %s", meta.version, meta.commit, meta.date) } diff --git a/forst/cmd/forst/lsp/version_test.go b/forst/cmd/forst/lsp/version_test.go index 49c37195..1f3da4c4 100644 --- a/forst/cmd/forst/lsp/version_test.go +++ b/forst/cmd/forst/lsp/version_test.go @@ -10,13 +10,10 @@ import ( func TestLogBuildInfo_logsVersionCommitDate(t *testing.T) { t.Parallel() - Version = "1.2.3" - Commit = "abc123" - Date = "2026-07-08" + origVersion, origCommit, origDate := BuildInfo() + SetBuildMetadata("1.2.3", "abc123", "2026-07-08") t.Cleanup(func() { - Version = "dev" - Commit = "unknown" - Date = "unknown" + SetBuildMetadata(origVersion, origCommit, origDate) }) var buf bytes.Buffer @@ -35,13 +32,10 @@ func TestLogBuildInfo_logsVersionCommitDate(t *testing.T) { func TestBuildInfoMap(t *testing.T) { t.Parallel() - Version = "v" - Commit = "c" - Date = "d" + origVersion, origCommit, origDate := BuildInfo() + SetBuildMetadata("v", "c", "d") t.Cleanup(func() { - Version = "dev" - Commit = "unknown" - Date = "unknown" + SetBuildMetadata(origVersion, origCommit, origDate) }) m := BuildInfoMap() if m["version"] != "v" || m["commit"] != "c" || m["date"] != "d" { diff --git a/forst/cmd/forst/main.go b/forst/cmd/forst/main.go index fb9f6a68..a4fce933 100644 --- a/forst/cmd/forst/main.go +++ b/forst/cmd/forst/main.go @@ -143,9 +143,7 @@ func runMain(argv []string) int { setLogLevel(log, *logLevel) // Set version information in LSP package - lsp.Version = Version - lsp.Commit = Commit - lsp.Date = Date + lsp.SetBuildMetadata(Version, Commit, Date) if err := startLSPFunc(*port, log); err != nil { return 1 @@ -176,9 +174,7 @@ func runMain(argv []string) int { } // Set version information in LSP package - lsp.Version = Version - lsp.Commit = Commit - lsp.Date = Date + lsp.SetBuildMetadata(Version, Commit, Date) if err := handleDumpCommand(*filePath, *compression, *format, *phase, *summary, log); err != nil { log.Error(err) diff --git a/forst/cmd/forst/main_test.go b/forst/cmd/forst/main_test.go index 79cb49d8..a549761d 100644 --- a/forst/cmd/forst/main_test.go +++ b/forst/cmd/forst/main_test.go @@ -3,6 +3,7 @@ package main import ( "encoding/json" "fmt" + "forst/cmd/forst/lsp" "forst/internal/compiler" "forst/internal/ftconfig" "io" @@ -329,25 +330,14 @@ func TestCompilerArgsParsing(t *testing.T) { func TestLSPVersionInjection(t *testing.T) { // Test that version information is correctly injected into LSP package - originalVersion := Version - originalCommit := Commit - originalDate := Date - defer func() { - Version = originalVersion - Commit = originalCommit - Date = originalDate - }() + origVersion, origCommit, origDate := lsp.BuildInfo() + t.Cleanup(func() { + lsp.SetBuildMetadata(origVersion, origCommit, origDate) + }) - // Set test values - Version = "test-version" - Commit = "test-commit" - Date = "test-date" + lsp.SetBuildMetadata("test-version", "test-commit", "test-date") - // Test the version injection logic that would be used in main - // This simulates the logic: lsp.Version = Version, etc. - lspVersion := Version - lspCommit := Commit - lspDate := Date + lspVersion, lspCommit, lspDate := lsp.BuildInfo() if lspVersion != "test-version" { t.Errorf("Expected LSP version test-version, got %s", lspVersion)