diff --git a/README.md b/README.md index 2dc93fe..c8e1867 100644 --- a/README.md +++ b/README.md @@ -111,6 +111,9 @@ go run github.com/keyskey/gobce/cmd/gobce@latest analyze --coverprofile coverage jq -e '.estimatedBranchCoverage >= 70' gobce.json ``` +`uncoveredBranches` is nested by `importPath` and file `path`. To flatten all branch entries with `jq`, for example: +`jq '[.uncoveredBranches.packages[] | .files[] | .branches[]]' gobce.json` + For stable CI behavior, prefer pinned versions (`@vX.Y.Z`) over `@latest`. ## Versioning Policy diff --git a/analyze.go b/analyze.go index 588f5d4..8db260d 100644 --- a/analyze.go +++ b/analyze.go @@ -10,20 +10,29 @@ func Analyze(input AnalyzeInput) (Result, error) { return Result{}, err } - uncovered := make([]UncoveredBranch, 0, len(analysisResult.UncoveredBranches)) - for _, b := range analysisResult.UncoveredBranches { - uncovered = append(uncovered, UncoveredBranch{ - File: b.File, - Line: b.Line, - Kind: b.Kind, - Recommendation: b.Recommendation, - }) - } - return Result{ Language: analysisResult.Language, StatementCoverage: analysisResult.StatementCoverage, EstimatedBranchCoverage: analysisResult.EstimatedBranchCoverage, - UncoveredBranches: uncovered, + UncoveredBranches: convertUncoveredRollup(analysisResult.UncoveredBranches), }, nil } + +func convertUncoveredRollup(in analyzer.UncoveredBranchesReport) UncoveredBranchesReport { + pkgs := make([]UncoveredPackage, len(in.Packages)) + for i, p := range in.Packages { + files := make([]UncoveredFile, len(p.Files)) + for j, f := range p.Files { + br := make([]UncoveredBranch, len(f.Branches)) + for k, b := range f.Branches { + br[k] = UncoveredBranch{ + Line: b.Line, + Kind: b.Kind, + } + } + files[j] = UncoveredFile{Path: f.Path, Branches: br} + } + pkgs[i] = UncoveredPackage{ImportPath: p.ImportPath, Files: files} + } + return UncoveredBranchesReport{Packages: pkgs} +} diff --git a/analyzer_test.go b/analyzer_test.go index ce2e247..4fb8943 100644 --- a/analyzer_test.go +++ b/analyzer_test.go @@ -44,18 +44,11 @@ func score(v int) int { if result.Language != "go" { t.Fatalf("language: got %q", result.Language) } - if len(result.UncoveredBranches) == 0 { + if totalUncoveredBranches(result) == 0 { t.Fatalf("expected uncovered branches") } - var hasIfFalse bool - for _, b := range result.UncoveredBranches { - if b.Kind == "if_false_path" { - hasIfFalse = true - break - } - } - if !hasIfFalse { + if !hasUncoveredBranchKind(result, "if_false_path") { t.Fatalf("expected if_false_path in uncovered branches") } } @@ -86,14 +79,7 @@ func score(v int) int { if v > 10 { return 1 } else { return 2 } } t.Fatalf("analyze: %v", err) } - var hasIfFalse bool - for _, b := range result.UncoveredBranches { - if b.Kind == "if_false_path" { - hasIfFalse = true - break - } - } - if !hasIfFalse { + if !hasUncoveredBranchKind(result, "if_false_path") { t.Fatalf("expected if_false_path in uncovered branches") } } @@ -132,7 +118,7 @@ func score(v int) int { t.Fatalf("analyze should not fail on unresolved source path: %v", err) } - if len(result.UncoveredBranches) == 0 { + if totalUncoveredBranches(result) == 0 { t.Fatalf("expected uncovered branches from resolvable source file") } } @@ -325,10 +311,24 @@ func score(v int) int { } } +func totalUncoveredBranches(r Result) int { + n := 0 + for _, p := range r.UncoveredBranches.Packages { + for _, f := range p.Files { + n += len(f.Branches) + } + } + return n +} + func hasUncoveredBranchKind(result Result, kind string) bool { - for _, b := range result.UncoveredBranches { - if b.Kind == kind { - return true + for _, p := range result.UncoveredBranches.Packages { + for _, f := range p.Files { + for _, b := range f.Branches { + if b.Kind == kind { + return true + } + } } } return false diff --git a/docs/gobce.md b/docs/gobce.md index a493109..db773da 100644 --- a/docs/gobce.md +++ b/docs/gobce.md @@ -46,13 +46,24 @@ go run ./cmd/gobce analyze \ "language": "go", "statementCoverage": 82.1, "estimatedBranchCoverage": 68.4, - "uncoveredBranches": [ - { - "file": "internal/order/validator.go", - "line": 42, - "kind": "if_false_path" - } - ] + "uncoveredBranches": { + "packages": [ + { + "importPath": "github.com/example/app/internal/order", + "files": [ + { + "path": "internal/order/validator.go", + "branches": [ + { + "line": 42, + "kind": "if_false_path" + } + ] + } + ] + } + ] + } } ``` @@ -88,7 +99,7 @@ generated-code filtering improvements 4. coverage block と branch candidate span を対応付ける。 5. 各 branch side が実行されたかを推定する。 6. estimated C1 percentage を計算する。 -7. file, line, kind, recommendation 付きの uncovered branch finding を出す。 +7. import path とファイルごとにまとめた uncovered branch finding(line, kind)を出す。 ``` ## 重要な制約 diff --git a/docs/how-gobce-works.md b/docs/how-gobce-works.md index 83410f6..330fc9e 100644 --- a/docs/how-gobce-works.md +++ b/docs/how-gobce-works.md @@ -93,13 +93,24 @@ sample.go:6.8,8.3 ... count=0 (false 側の範囲) ```json { - "uncoveredBranches": [ - { - "file": "sample.go", - "line": 6, - "kind": "if_false_path" - } - ] + "uncoveredBranches": { + "packages": [ + { + "importPath": "example.com/m", + "files": [ + { + "path": "sample.go", + "branches": [ + { + "line": 6, + "kind": "if_false_path" + } + ] + } + ] + } + ] + } } ``` @@ -115,8 +126,7 @@ sample.go:6.8,8.3 ... count=0 (false 側の範囲) `gobce` が分岐候補に対して covered/uncovered を推定して計算した割合。 - `uncoveredBranches` - テスト実行時にまだ実行されていないと推定された分岐の一覧。 - どのファイル・どの行・どの kind かが入ります。 + テスト実行時にまだ実行されていないと推定された分岐の一覧。`go.mod` の module path が分かる場合は `importPath` とモジュール相対の `path` に階層化し、各ファイルの `branches` に行番号と `kind` が入ります。 ## 「推定」になる理由 @@ -135,7 +145,7 @@ A. 直線的な処理のコードはテスト実行時に実行されていて ### Q. まず何を見ればいい? -A. `uncoveredBranches` から見れば OK です。 +A. `uncoveredBranches.packages` 以下を辿れば OK です。 分岐の未到達ポイントが具体的に出るので、テスト追加の優先順位を付けやすくなります。 ## 開発者向けの読み進め順 diff --git a/internal/analyzer/analyze.go b/internal/analyzer/analyze.go index 4291373..4bc6b13 100644 --- a/internal/analyzer/analyze.go +++ b/internal/analyzer/analyze.go @@ -7,10 +7,11 @@ func Analyze(input Input) (Result, error) { return Result{}, errors.New("coverprofile path is required") } - blocks, err := parseCoverProfile(input.CoverProfilePath) + parsed, err := parseCoverProfile(input.CoverProfilePath) if err != nil { return Result{}, err } + blocks := parsed.Blocks statementCoverage := computeStatementCoverage(blocks) candidates, err := collectBranchCandidates(blocks) @@ -19,20 +20,22 @@ func Analyze(input Input) (Result, error) { } covered := 0 - uncovered := make([]UncoveredBranch, 0) + sources := make([]uncoveredSource, 0) for _, c := range candidates { if c.Covered { covered++ continue } - uncovered = append(uncovered, UncoveredBranch{ - File: c.FilePath, - Line: c.Line, - Kind: c.Kind, + sources = append(sources, uncoveredSource{ + file: c.FilePath, + line: c.Line, + kind: c.Kind, }) } + rollup := groupUncoveredSources(sources, parsed.ModuleRoot, parsed.ModulePath) + estimated := 100.0 if len(candidates) > 0 { estimated = percent(float64(covered), float64(len(candidates))) @@ -42,6 +45,6 @@ func Analyze(input Input) (Result, error) { Language: "go", StatementCoverage: statementCoverage, EstimatedBranchCoverage: estimated, - UncoveredBranches: uncovered, + UncoveredBranches: rollup, }, nil } diff --git a/internal/analyzer/coverprofile.go b/internal/analyzer/coverprofile.go index 8389142..b3be921 100644 --- a/internal/analyzer/coverprofile.go +++ b/internal/analyzer/coverprofile.go @@ -10,10 +10,17 @@ import ( "strings" ) -func parseCoverProfile(path string) ([]coverageBlock, error) { +// ParsedCoverProfile holds normalized coverage blocks and module context used for path grouping. +type ParsedCoverProfile struct { + Blocks []coverageBlock + ModuleRoot string + ModulePath string +} + +func parseCoverProfile(path string) (ParsedCoverProfile, error) { f, err := os.Open(path) if err != nil { - return nil, fmt.Errorf("open coverprofile: %w", err) + return ParsedCoverProfile{}, fmt.Errorf("open coverprofile: %w", err) } defer f.Close() @@ -32,24 +39,31 @@ func parseCoverProfile(path string) ([]coverageBlock, error) { b, err := parseCoverProfileLine(line) if err != nil { - return nil, fmt.Errorf("parse coverprofile line %d: %w", lineNo, err) + return ParsedCoverProfile{}, fmt.Errorf("parse coverprofile line %d: %w", lineNo, err) } blocks = append(blocks, b) } if err := scanner.Err(); err != nil { - return nil, fmt.Errorf("scan coverprofile: %w", err) + return ParsedCoverProfile{}, fmt.Errorf("scan coverprofile: %w", err) } if len(blocks) == 0 { - return nil, errors.New("coverprofile contains no coverage blocks") + return ParsedCoverProfile{}, errors.New("coverprofile contains no coverage blocks") } - normalizedBlocks, err := normalizeBlockPaths(blocks, path) + coverDir := filepath.Dir(path) + moduleRoot, modulePath := discoverModuleContext(coverDir) + + normalizedBlocks, err := normalizeBlockPaths(blocks, path, moduleRoot, modulePath) if err != nil { - return nil, err + return ParsedCoverProfile{}, err } - return normalizedBlocks, nil + return ParsedCoverProfile{ + Blocks: normalizedBlocks, + ModuleRoot: moduleRoot, + ModulePath: modulePath, + }, nil } func parseCoverProfileLine(line string) (coverageBlock, error) { diff --git a/internal/analyzer/path_resolver.go b/internal/analyzer/path_resolver.go index ff393d4..4259883 100644 --- a/internal/analyzer/path_resolver.go +++ b/internal/analyzer/path_resolver.go @@ -6,9 +6,8 @@ import ( "strings" ) -func normalizeBlockPaths(blocks []coverageBlock, coverProfilePath string) ([]coverageBlock, error) { +func normalizeBlockPaths(blocks []coverageBlock, coverProfilePath string, moduleRoot string, modulePath string) ([]coverageBlock, error) { coverDir := filepath.Dir(coverProfilePath) - moduleRoot, modulePath := discoverModuleContext(coverDir) normalized := make([]coverageBlock, 0, len(blocks)) for _, b := range blocks { diff --git a/internal/analyzer/rollup.go b/internal/analyzer/rollup.go new file mode 100644 index 0000000..e98ae1c --- /dev/null +++ b/internal/analyzer/rollup.go @@ -0,0 +1,98 @@ +package analyzer + +import ( + "path/filepath" + "sort" + "strings" +) + +type uncoveredSource struct { + file string + line int + kind string +} + +func groupUncoveredSources(rows []uncoveredSource, moduleRoot, modulePath string) UncoveredBranchesReport { + if len(rows) == 0 { + return UncoveredBranchesReport{Packages: []UncoveredPackage{}} + } + + type fileKey string + pkgFiles := map[string]map[fileKey][]UncoveredBranch{} + + for _, row := range rows { + importPath, displayPath := classifySourceFile(row.file, moduleRoot, modulePath) + if pkgFiles[importPath] == nil { + pkgFiles[importPath] = map[fileKey][]UncoveredBranch{} + } + k := fileKey(displayPath) + pkgFiles[importPath][k] = append(pkgFiles[importPath][k], UncoveredBranch{ + Line: row.line, + Kind: row.kind, + }) + } + + pkgKeys := make([]string, 0, len(pkgFiles)) + for k := range pkgFiles { + pkgKeys = append(pkgKeys, k) + } + sort.Strings(pkgKeys) + + out := UncoveredBranchesReport{Packages: make([]UncoveredPackage, 0, len(pkgKeys))} + for _, importPath := range pkgKeys { + filesMap := pkgFiles[importPath] + fileKeys := make([]string, 0, len(filesMap)) + for fk := range filesMap { + fileKeys = append(fileKeys, string(fk)) + } + sort.Strings(fileKeys) + + ufiles := make([]UncoveredFile, 0, len(fileKeys)) + for _, p := range fileKeys { + branches := filesMap[fileKey(p)] + sort.Slice(branches, func(i, j int) bool { + if branches[i].Line != branches[j].Line { + return branches[i].Line < branches[j].Line + } + return branches[i].Kind < branches[j].Kind + }) + ufiles = append(ufiles, UncoveredFile{ + Path: p, + Branches: branches, + }) + } + out.Packages = append(out.Packages, UncoveredPackage{ + ImportPath: importPath, + Files: ufiles, + }) + } + return out +} + +func classifySourceFile(absFile, moduleRoot, modulePath string) (importPath, displayPath string) { + absFile = filepath.Clean(absFile) + moduleRoot = filepath.Clean(moduleRoot) + + if moduleRoot == "" || modulePath == "" { + return fallbackPackageKey(absFile), filepath.ToSlash(absFile) + } + + rel, err := filepath.Rel(moduleRoot, absFile) + if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return fallbackPackageKey(absFile), filepath.ToSlash(absFile) + } + + rel = filepath.Clean(rel) + displayPath = filepath.ToSlash(rel) + dir := filepath.Dir(rel) + if dir == "." { + importPath = modulePath + } else { + importPath = modulePath + "/" + filepath.ToSlash(dir) + } + return importPath, displayPath +} + +func fallbackPackageKey(absFile string) string { + return "_" + filepath.ToSlash(filepath.Dir(absFile)) +} diff --git a/internal/analyzer/rollup_test.go b/internal/analyzer/rollup_test.go new file mode 100644 index 0000000..9619580 --- /dev/null +++ b/internal/analyzer/rollup_test.go @@ -0,0 +1,45 @@ +package analyzer + +import ( + "path/filepath" + "testing" +) + +func TestClassifySourceFileUnderModule(t *testing.T) { + root := filepath.Join("testdata", "modroot") + mod := "github.com/example/lib" + abs := filepath.Join(root, "internal", "x", "a.go") + + gotImport, gotPath := classifySourceFile(abs, root, mod) + wantImport := "github.com/example/lib/internal/x" + wantPath := "internal/x/a.go" + if gotImport != wantImport { + t.Fatalf("importPath: got %q want %q", gotImport, wantImport) + } + if gotPath != wantPath { + t.Fatalf("displayPath: got %q want %q", gotPath, wantPath) + } +} + +func TestGroupUncoveredSourcesSorts(t *testing.T) { + rows := []uncoveredSource{ + {file: "/p/a.go", line: 2, kind: "b"}, + {file: "/p/a.go", line: 2, kind: "a"}, + {file: "/p/b.go", line: 1, kind: "x"}, + } + got := groupUncoveredSources(rows, "", "") + if len(got.Packages) != 1 { + t.Fatalf("packages: %d", len(got.Packages)) + } + p := got.Packages[0] + if len(p.Files) != 2 { + t.Fatalf("files: %d", len(p.Files)) + } + if p.Files[0].Path != "/p/a.go" || p.Files[1].Path != "/p/b.go" { + t.Fatalf("file order: %#v", p.Files) + } + br := p.Files[0].Branches + if len(br) != 2 || br[0].Kind != "a" || br[1].Kind != "b" { + t.Fatalf("branch sort: %#v", br) + } +} diff --git a/internal/analyzer/types.go b/internal/analyzer/types.go index 24badb3..f0e1305 100644 --- a/internal/analyzer/types.go +++ b/internal/analyzer/types.go @@ -8,14 +8,27 @@ type Result struct { Language string StatementCoverage float64 EstimatedBranchCoverage float64 - UncoveredBranches []UncoveredBranch + UncoveredBranches UncoveredBranchesReport +} + +// UncoveredBranchesReport groups uncovered branch findings by import path and file path. +type UncoveredBranchesReport struct { + Packages []UncoveredPackage `json:"packages"` +} + +type UncoveredPackage struct { + ImportPath string `json:"importPath"` + Files []UncoveredFile `json:"files"` +} + +type UncoveredFile struct { + Path string `json:"path"` + Branches []UncoveredBranch `json:"branches"` } type UncoveredBranch struct { - File string - Line int - Kind string - Recommendation string + Line int `json:"line"` + Kind string `json:"kind"` } type coverageBlock struct { diff --git a/types.go b/types.go index 3e561ea..9d8a62e 100644 --- a/types.go +++ b/types.go @@ -5,15 +5,28 @@ type AnalyzeInput struct { } type Result struct { - Language string `json:"language"` - StatementCoverage float64 `json:"statementCoverage"` - EstimatedBranchCoverage float64 `json:"estimatedBranchCoverage"` - UncoveredBranches []UncoveredBranch `json:"uncoveredBranches"` + Language string `json:"language"` + StatementCoverage float64 `json:"statementCoverage"` + EstimatedBranchCoverage float64 `json:"estimatedBranchCoverage"` + UncoveredBranches UncoveredBranchesReport `json:"uncoveredBranches"` +} + +// UncoveredBranchesReport groups uncovered branch findings by import path and file path. +type UncoveredBranchesReport struct { + Packages []UncoveredPackage `json:"packages"` +} + +type UncoveredPackage struct { + ImportPath string `json:"importPath"` + Files []UncoveredFile `json:"files"` +} + +type UncoveredFile struct { + Path string `json:"path"` + Branches []UncoveredBranch `json:"branches"` } type UncoveredBranch struct { - File string `json:"file"` - Line int `json:"line"` - Kind string `json:"kind"` - Recommendation string `json:"recommendation,omitempty"` + Line int `json:"line"` + Kind string `json:"kind"` }