From 99195d3b0deb8783e3703db2d66d7083295f7998 Mon Sep 17 00:00:00 2001 From: quobix Date: Tue, 14 Apr 2026 07:24:12 -0400 Subject: [PATCH 1/2] Address #227 and #262 Addressed a bottlenecking issue in the doctor causing a freeze and skipping broken specs in a report when using a timeline. --- AGENTS.md | 3 + cmd/console_test.go | 8 ++ cmd/engine.go | 54 ++++++++++ cmd/loaders.go | 54 ++++++++-- cmd/report.go | 114 ++++++++++++++++---- cmd/report_test.go | 109 +++++++++++++++++++ cmd/summary_test.go | 33 ++++++ git/github.go | 55 ++++++++-- git/github_test.go | 27 +++++ git/options.go | 9 ++ git/read_local.go | 200 ++++++++++++++++++----------------- git/read_local_test.go | 26 +++++ internal/testutil/gitrepo.go | 85 +++++++++++++++ model/report.go | 16 ++- 14 files changed, 658 insertions(+), 135 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 622aa75..76a9756 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -65,6 +65,8 @@ All `cmd/` implementation files use their canonical names (e.g., `cmd/summary.go - `report` should emit semantic `path` values whenever possible. - If semantic normalization rewrites an engine path, preserve the original path in `rawPath`. - Parameter identity must not depend on array index positions when a stable semantic name exists. +- Historical `report` output must surface partial-success state in `metaData` instead of relying on stderr warnings alone. +- `metaData.skippedCommits` is the machine-readable list of git/GitHub history revisions that were skipped because they failed to normalize or render. ### Left/right behavior @@ -76,6 +78,7 @@ All `cmd/` implementation files use their canonical names (e.g., `cmd/summary.go - Mixed-success histories should limp on with warnings when at least one comparable commit renders successfully. - `summary`, `markdown-report`, and `html-report` should fail only when every candidate commit fails to render/build. +- Historical `report` output should limp on when at least one history item renders, and must set `metaData.partial = true` when any commits were skipped. - “No prior comparable version” and “no changes found” are distinct states. ### HTML payload integrity diff --git a/cmd/console_test.go b/cmd/console_test.go index 4326a31..27c0148 100644 --- a/cmd/console_test.go +++ b/cmd/console_test.go @@ -94,9 +94,11 @@ func TestWrapConsoleStartError(t *testing.T) { func TestConsoleCommand_NoPriorVersionText(t *testing.T) { originalExtract := extractHistoryFromFile originalPopulate := populateHistory + originalPopulateDetailed := populateHistoryDetailed t.Cleanup(func() { extractHistoryFromFile = originalExtract populateHistory = originalPopulate + populateHistoryDetailed = originalPopulateDetailed }) extractHistoryFromFile = func(repoDirectory, filePath string, @@ -110,6 +112,12 @@ func TestConsoleCommand_NoPriorVersionText(t *testing.T) { ) ([]*model.Commit, []error) { return []*model.Commit{}, nil } + populateHistoryDetailed = func(commitHistory []*model.Commit, + progressChan chan *model.ProgressUpdate, errorChan chan model.ProgressError, opts git.HistoryOptions, + breakingConfig *whatChangedModel.BreakingRulesConfig, + ) (*git.HistoryBuildResult, []error) { + return &git.HistoryBuildResult{}, nil + } cmd := testRootCmd(GetConsoleCommand(), "--no-logo", "--no-color", "..", "sample-specs/petstorev3.json") output := captureStdout(t, func() { diff --git a/cmd/engine.go b/cmd/engine.go index a1f4eb5..8fa3e96 100644 --- a/cmd/engine.go +++ b/cmd/engine.go @@ -4,8 +4,10 @@ package cmd import ( + "bytes" "fmt" "os" + "strings" "github.com/pb33f/doctor/changerator" drModel "github.com/pb33f/doctor/model" @@ -13,6 +15,7 @@ import ( whatChangedModel "github.com/pb33f/libopenapi/what-changed/model" "github.com/pb33f/openapi-changes/internal/breakingrules" "github.com/pb33f/openapi-changes/model" + "go.yaml.in/yaml/v4" ) // changeratorResult owns the doctor-side resources created for a single comparison. @@ -51,6 +54,9 @@ func runChangerator(commit *model.Commit, breakingConfig *whatChangedModel.Break if err != nil { return nil, modelBuildError("original", commitSourceLabel(commit, false), err) } + if isSelfContainedIdenticalComparison(commit) { + return nil, nil + } rightDrDoc := drModel.NewDrDocumentAndGraph(rightModel) leftDrDoc := drModel.NewDrDocumentAndGraph(leftModel) @@ -92,6 +98,54 @@ func runChangerator(commit *model.Commit, breakingConfig *whatChangedModel.Break }, nil } +func isSelfContainedIdenticalComparison(commit *model.Commit) bool { + if commit == nil || !commit.Synthetic || len(commit.Data) == 0 || len(commit.OldData) == 0 { + return false + } + if !bytes.Equal(commit.Data, commit.OldData) { + return false + } + return !rootBytesContainExternalRefs(commit.OldData) && !rootBytesContainExternalRefs(commit.Data) +} + +func rootBytesContainExternalRefs(spec []byte) bool { + if len(spec) == 0 { + return false + } + var root yaml.Node + if err := yaml.Unmarshal(spec, &root); err != nil { + return true + } + return nodeContainsExternalRef(&root) +} + +func nodeContainsExternalRef(node *yaml.Node) bool { + if node == nil { + return false + } + if node.Kind == yaml.MappingNode { + for i := 0; i+1 < len(node.Content); i += 2 { + key := node.Content[i] + value := node.Content[i+1] + if key != nil && key.Value == "$ref" && value != nil && value.Kind == yaml.ScalarNode { + if value.Value != "" && !strings.HasPrefix(value.Value, "#") { + return true + } + } + if nodeContainsExternalRef(value) { + return true + } + } + return false + } + for _, child := range node.Content { + if nodeContainsExternalRef(child) { + return true + } + } + return false +} + func rewriteOutputLocations(ctr *changerator.Changerator, docChanges *whatChangedModel.DocumentChanges, rewriters []model.DocumentPathRewriter) { if len(rewriters) == 0 { return diff --git a/cmd/loaders.go b/cmd/loaders.go index b59381c..1d0d520 100644 --- a/cmd/loaders.go +++ b/cmd/loaders.go @@ -19,10 +19,17 @@ import ( ) var processGithubRepo = git.ProcessGithubRepo +var processGithubRepoDetailed = git.ProcessGithubRepoDetailed var extractHistoryFromFile = git.ExtractHistoryFromFile var populateHistory = git.PopulateHistory +var populateHistoryDetailed = git.PopulateHistoryDetailed var httpGet = http.Get +type loadedHistoryResult struct { + Commits []*model.Commit + SkippedCommits []string +} + // progressDrainer drains git progress channels that use synchronous sends. // Call close() before reading collected warnings or errors. type progressDrainer struct { @@ -116,7 +123,7 @@ func (d *progressDrainer) collectErrors(errs []error) error { return errors.Join(all...) } -func loadGitHubCommits(rawURL string, opts summaryOpts, breakingConfig *whatChangedModel.BreakingRulesConfig) ([]*model.Commit, error) { +func loadGitHubCommitsDetailed(rawURL string, opts summaryOpts, breakingConfig *whatChangedModel.BreakingRulesConfig) (*loadedHistoryResult, error) { specURL, err := url.Parse(rawURL) if err != nil { return nil, fmt.Errorf("invalid URL: %w", err) @@ -127,7 +134,7 @@ func loadGitHubCommits(rawURL string, opts summaryOpts, breakingConfig *whatChan } d := makeProgressDrainer() - commits, errs := processGithubRepo(user, repo, filePath, + result, errs := processGithubRepoDetailed(user, repo, filePath, d.ProgressChan, d.ErrorChan, git.HistoryOptions{ BaseCommit: opts.baseCommit, Limit: opts.limit, @@ -141,13 +148,28 @@ func loadGitHubCommits(rawURL string, opts summaryOpts, breakingConfig *whatChan return nil, err } + if result == nil { + return nil, nil + } + commits := result.Commits if opts.latest && len(commits) > 1 { commits = commits[:1] } - return commits, nil + return &loadedHistoryResult{ + Commits: commits, + SkippedCommits: result.SkippedCommits, + }, nil } -func loadGitHistoryCommits(gitPath, filePath string, opts summaryOpts, breakingConfig *whatChangedModel.BreakingRulesConfig) ([]*model.Commit, error) { +func loadGitHubCommits(rawURL string, opts summaryOpts, breakingConfig *whatChangedModel.BreakingRulesConfig) ([]*model.Commit, error) { + result, err := loadGitHubCommitsDetailed(rawURL, opts, breakingConfig) + if result == nil { + return nil, err + } + return result.Commits, err +} + +func loadGitHistoryCommitsDetailed(gitPath, filePath string, opts summaryOpts, breakingConfig *whatChangedModel.BreakingRulesConfig) (*loadedHistoryResult, error) { if gitPath == "" || filePath == "" { return nil, errors.New("please supply a path to a git repo and a path to a file") } @@ -178,7 +200,7 @@ func loadGitHistoryCommits(gitPath, filePath string, opts summaryOpts, breakingC } populateDrainer := makeProgressDrainer() - commits, errs = populateHistory(commits, + result, errs := populateHistoryDetailed(commits, populateDrainer.ProgressChan, populateDrainer.ErrorChan, git.HistoryOptions{ LimitTime: opts.limitTime, Base: opts.base, @@ -190,13 +212,31 @@ func loadGitHistoryCommits(gitPath, filePath string, opts summaryOpts, breakingC return nil, err } - if len(commits) == 0 { + if result == nil { return nil, nil } + commits = result.Commits + if len(commits) == 0 { + return &loadedHistoryResult{SkippedCommits: result.SkippedCommits}, nil + } if opts.latest { commits = commits[:1] } - return commits, nil + return &loadedHistoryResult{ + Commits: commits, + SkippedCommits: result.SkippedCommits, + }, nil +} + +func loadGitHistoryCommits(gitPath, filePath string, opts summaryOpts, breakingConfig *whatChangedModel.BreakingRulesConfig) ([]*model.Commit, error) { + result, err := loadGitHistoryCommitsDetailed(gitPath, filePath, opts, breakingConfig) + if result == nil { + return nil, err + } + if len(result.Commits) == 0 { + return nil, nil + } + return result.Commits, err } func loadLeftRightCommits(left, right string, opts summaryOpts) ([]*model.Commit, error) { diff --git a/cmd/report.go b/cmd/report.go index 6d5be3c..5b78b12 100644 --- a/cmd/report.go +++ b/cmd/report.go @@ -31,25 +31,50 @@ func changerateCommit(commit *model.Commit, breakingConfig *whatChangedModel.Bre return result, nil } -func changerateAndFlatten(commits []*model.Commit, breakingConfig *whatChangedModel.BreakingRulesConfig) ([]*model.FlatReport, error) { +type historicalFlattenResult struct { + Reports []*model.FlatReport + SkippedCommits []string +} + +func changerateAndFlattenHistory(commits []*model.Commit, breakingConfig *whatChangedModel.BreakingRulesConfig) (*historicalFlattenResult, error) { reports := make([]*model.FlatReport, 0, len(commits)) - var errs []error + var renderErrors []error + var skippedCommits []string + skippedSeen := make(map[string]struct{}) + processedComparables := 0 + for _, commit := range commits { - result, err := changerateCommit(commit, breakingConfig) + if commit == nil { + continue + } + comparable := commit.Document != nil && commit.OldDocument != nil + result, err := runChangerator(commit, breakingConfig) if err != nil { - errs = append(errs, err) + emitCommitWarning(commit, err) + renderErrors = append(renderErrors, wrapCommitError(commit, err)) + addSkippedCommitHash(&skippedCommits, skippedSeen, commit.Hash) continue } + if comparable { + processedComparables++ + } if result == nil { continue } + commit.Changes = result.DocChanges reports = append(reports, FlattenReportWithParameterNames(createReport(commit), result.Changerator.ParameterNames)) result.Release() } - if len(errs) > 0 { - return nil, errors.Join(errs...) + if len(renderErrors) > 0 { + if len(reports) == 0 && processedComparables == 0 { + return nil, fmt.Errorf("all %d commits failed to render report data: %w", len(renderErrors), errors.Join(renderErrors...)) + } + fmt.Fprintf(os.Stderr, "warning: %d commits failed to render report data\n", len(renderErrors)) } - return reports, nil + return &historicalFlattenResult{ + Reports: reports, + SkippedCommits: skippedCommits, + }, nil } func runLeftRightReport(left, right string, opts summaryOpts, breakingConfig *whatChangedModel.BreakingRulesConfig) (*model.FlatReport, error) { @@ -73,25 +98,39 @@ func runLeftRightReport(left, right string, opts summaryOpts, breakingConfig *wh } func runGitHistoryReport(gitPath, filePath string, opts summaryOpts, breakingConfig *whatChangedModel.BreakingRulesConfig) (*model.FlatHistoricalReport, error) { - commits, err := loadGitHistoryCommits(gitPath, filePath, opts, breakingConfig) + loaded, err := loadGitHistoryCommitsDetailed(gitPath, filePath, opts, breakingConfig) if err != nil { return nil, err } - if commits == nil { + if loaded == nil { return nil, nil } - reports, err := changerateAndFlatten(commits, breakingConfig) + if len(loaded.Commits) == 0 && len(loaded.SkippedCommits) == 0 { + return nil, nil + } + history, err := changerateAndFlattenHistory(loaded.Commits, breakingConfig) if err != nil { return nil, err } + skippedCommits := mergeSkippedCommitHashes(loaded.SkippedCommits, history.SkippedCommits) + if len(history.Reports) == 0 && len(skippedCommits) > 0 { + return nil, fmt.Errorf("all %d candidate commits were skipped or failed to render", len(skippedCommits)) + } - return &model.FlatHistoricalReport{ + report := &model.FlatHistoricalReport{ GitRepoPath: gitPath, GitFilePath: filePath, Filename: path.Base(filePath), DateGenerated: time.Now().Format(time.RFC3339), - Reports: reports, - }, nil + Reports: history.Reports, + } + if len(skippedCommits) > 0 { + report.MetaData = &model.HistoricalReportMetaData{ + Partial: true, + SkippedCommits: skippedCommits, + } + } + return report, nil } func runGithubHistoryReport(rawURL string, opts summaryOpts, breakingConfig *whatChangedModel.BreakingRulesConfig) (*model.FlatHistoricalReport, error) { @@ -104,22 +143,61 @@ func runGithubHistoryReport(rawURL string, opts summaryOpts, breakingConfig *wha return nil, fmt.Errorf("error extracting github details: %w", err) } - commits, err := loadGitHubCommits(rawURL, opts, breakingConfig) + loaded, err := loadGitHubCommitsDetailed(rawURL, opts, breakingConfig) if err != nil { return nil, err } - reports, err := changerateAndFlatten(commits, breakingConfig) + if loaded == nil { + return nil, nil + } + if len(loaded.Commits) == 0 && len(loaded.SkippedCommits) == 0 { + return nil, nil + } + history, err := changerateAndFlattenHistory(loaded.Commits, breakingConfig) if err != nil { return nil, err } + skippedCommits := mergeSkippedCommitHashes(loaded.SkippedCommits, history.SkippedCommits) + if len(history.Reports) == 0 && len(skippedCommits) > 0 { + return nil, fmt.Errorf("all %d candidate commits were skipped or failed to render", len(skippedCommits)) + } - return &model.FlatHistoricalReport{ + report := &model.FlatHistoricalReport{ GitRepoPath: fmt.Sprintf("%s/%s", user, repo), GitFilePath: filePath, Filename: path.Base(filePath), DateGenerated: time.Now().Format(time.RFC3339), - Reports: reports, - }, nil + Reports: history.Reports, + } + if len(skippedCommits) > 0 { + report.MetaData = &model.HistoricalReportMetaData{ + Partial: true, + SkippedCommits: skippedCommits, + } + } + return report, nil +} + +func addSkippedCommitHash(skippedCommits *[]string, seen map[string]struct{}, hash string) { + if hash == "" { + return + } + if _, ok := seen[hash]; ok { + return + } + seen[hash] = struct{}{} + *skippedCommits = append(*skippedCommits, hash) +} + +func mergeSkippedCommitHashes(groups ...[]string) []string { + var merged []string + seen := make(map[string]struct{}) + for _, group := range groups { + for _, hash := range group { + addSkippedCommitHash(&merged, seen, hash) + } + } + return merged } func printReportJSON(v any) error { diff --git a/cmd/report_test.go b/cmd/report_test.go index 41d1252..1bdf61b 100644 --- a/cmd/report_test.go +++ b/cmd/report_test.go @@ -14,6 +14,7 @@ import ( whatChangedModel "github.com/pb33f/libopenapi/what-changed/model" "github.com/pb33f/openapi-changes/git" + "github.com/pb33f/openapi-changes/internal/testutil" "github.com/pb33f/openapi-changes/model" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -33,8 +34,10 @@ func TestRunLeftRightReport_PropagatesChangeratorErrors(t *testing.T) { func TestRunGithubHistoryReport_PropagatesChangeratorErrors(t *testing.T) { originalProcess := processGithubRepo + originalProcessDetailed := processGithubRepoDetailed t.Cleanup(func() { processGithubRepo = originalProcess + processGithubRepoDetailed = originalProcessDetailed }) processGithubRepo = func(username, repo, filePath string, @@ -43,6 +46,12 @@ func TestRunGithubHistoryReport_PropagatesChangeratorErrors(t *testing.T) { ) ([]*model.Commit, []error) { return []*model.Commit{makeSwagger2Commit(t)}, nil } + processGithubRepoDetailed = func(username, repo, filePath string, + progressChan chan *model.ProgressUpdate, errorChan chan model.ProgressError, + opts git.HistoryOptions, breakingConfig *whatChangedModel.BreakingRulesConfig, + ) (*git.HistoryBuildResult, []error) { + return &git.HistoryBuildResult{Commits: []*model.Commit{makeSwagger2Commit(t)}}, nil + } report, err := runGithubHistoryReport("https://github.com/oai/openapi-specification/blob/main/examples/v2.0/json/petstore-expanded.json", summaryOpts{}, nil) @@ -80,6 +89,18 @@ func TestRunLeftRightReport_Success(t *testing.T) { assert.Equal(t, 16, report.Summary["paths"].Breaking) } +func TestRunLeftRightReport_IdenticalSpecsShortCircuit(t *testing.T) { + report, err := runLeftRightReport( + "../sample-specs/petstorev3.json", + "../sample-specs/petstorev3.json", + summaryOpts{noColor: true}, + nil, + ) + + require.NoError(t, err) + assert.Nil(t, report) +} + func TestRunLeftRightReport_SanitizesURLSourceMetadata(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path == "/left.yaml" { @@ -303,6 +324,94 @@ func TestRunGitHistoryReport_BasePathUsesRevisionScopedSiblingRefs(t *testing.T) assert.Contains(t, content, `"path":"$.paths['/thing'].get.responses['200'].content['application/json'].schema"`) } +func TestRunGitHistoryReport_PartialHistoryIncludesMetaData(t *testing.T) { + fixture := testutil.CreateInvalidHistoryGitSpecRepo(t) + + report, err := runGitHistoryReport(fixture.RepoDir, fixture.FileName, summaryOpts{base: fixture.RepoDir, noColor: true, limitTime: -1}, nil) + + require.NoError(t, err) + require.NotNil(t, report) + require.Len(t, report.Reports, 1) + require.NotNil(t, report.MetaData) + assert.True(t, report.MetaData.Partial) + assert.Equal(t, []string{fixture.InvalidHash}, report.MetaData.SkippedCommits) + assert.Equal(t, fixture.NewestHash, report.Reports[0].Commit.Hash) + + encoded, err := json.Marshal(report) + require.NoError(t, err) + assert.Contains(t, string(encoded), `"metaData"`) + assert.Contains(t, string(encoded), fixture.InvalidHash) +} + +func TestRunGitHistoryReport_AllInvalidHistoryFails(t *testing.T) { + repoDir := t.TempDir() + runGitInDir(t, repoDir, "init") + runGitInDir(t, repoDir, "config", "user.name", "Test User") + runGitInDir(t, repoDir, "config", "user.email", "test@example.com") + + fileName := "openapi.yaml" + specPath := filepath.Join(repoDir, fileName) + require.NoError(t, os.WriteFile(specPath, []byte("swagger: \"2.0\"\ninfo:\n title: broken\n version: \"1.0\"\npaths: {}\n"), 0o644)) + runGitInDir(t, repoDir, "add", fileName) + runGitInDir(t, repoDir, "commit", "-m", "invalid one") + require.NoError(t, os.WriteFile(specPath, []byte("swagger: \"2.0\"\ninfo:\n title: broken\n version: \"1.1\"\npaths: {}\n"), 0o644)) + runGitInDir(t, repoDir, "add", fileName) + runGitInDir(t, repoDir, "commit", "-m", "invalid two") + + report, err := runGitHistoryReport(repoDir, fileName, summaryOpts{base: repoDir, noColor: true, limitTime: -1}, nil) + + require.Error(t, err) + assert.Nil(t, report) + assert.Contains(t, err.Error(), "failed to render report data") +} + +func TestRunGithubHistoryReport_PartialHistoryIncludesMetaData(t *testing.T) { + originalProcess := processGithubRepoDetailed + t.Cleanup(func() { + processGithubRepoDetailed = originalProcess + }) + + commits := []*model.Commit{ + mustMakeDoctorOnlyCommitFromSpecs(t, "ccc333", `openapi: 3.0.3 +info: + title: test + version: "1.0.0" +paths: {} +`, `openapi: 3.0.3 +info: + title: test + version: "1.2.0" +paths: + /pets: + get: + responses: + "200": + description: ok +`), + } + commits[0].Message = "valid last" + commits[0].Author = "tester" + + processGithubRepoDetailed = func(username, repo, filePath string, + progressChan chan *model.ProgressUpdate, errorChan chan model.ProgressError, + opts git.HistoryOptions, breakingConfig *whatChangedModel.BreakingRulesConfig, + ) (*git.HistoryBuildResult, []error) { + return &git.HistoryBuildResult{ + Commits: commits, + SkippedCommits: []string{"bbb222"}, + }, nil + } + + report, err := runGithubHistoryReport("https://github.com/oai/openapi-specification/blob/main/examples/v3.0/petstore.yaml", summaryOpts{}, nil) + + require.NoError(t, err) + require.NotNil(t, report) + require.Len(t, report.Reports, 1) + require.NotNil(t, report.MetaData) + assert.True(t, report.MetaData.Partial) + assert.Equal(t, []string{"bbb222"}, report.MetaData.SkippedCommits) +} + func TestReportCommand_GitRefUsesLeftRightMode(t *testing.T) { wd, err := os.Getwd() require.NoError(t, err) diff --git a/cmd/summary_test.go b/cmd/summary_test.go index afb5631..75acf3b 100644 --- a/cmd/summary_test.go +++ b/cmd/summary_test.go @@ -90,9 +90,11 @@ func mustMakeDoctorOnlyCommitFromSpecs(t *testing.T, hash, left, right string) * func TestLoadGitHistoryCommits_ReturnsPopulateErrors(t *testing.T) { originalExtract := extractHistoryFromFile originalPopulate := populateHistory + originalPopulateDetailed := populateHistoryDetailed t.Cleanup(func() { extractHistoryFromFile = originalExtract populateHistory = originalPopulate + populateHistoryDetailed = originalPopulateDetailed }) extractHistoryFromFile = func(repoDirectory, filePath string, @@ -106,6 +108,12 @@ func TestLoadGitHistoryCommits_ReturnsPopulateErrors(t *testing.T) { ) ([]*model.Commit, []error) { return commitHistory, []error{errors.New("malformed spec"), errors.New("broken reference")} } + populateHistoryDetailed = func(commitHistory []*model.Commit, + progressChan chan *model.ProgressUpdate, errorChan chan model.ProgressError, opts git.HistoryOptions, + breakingConfig *whatChangedModel.BreakingRulesConfig, + ) (*git.HistoryBuildResult, []error) { + return nil, []error{errors.New("malformed spec"), errors.New("broken reference")} + } commits, err := loadGitHistoryCommits("..", "sample-specs/petstorev3.json", summaryOpts{}, nil) @@ -118,9 +126,11 @@ func TestLoadGitHistoryCommits_ReturnsPopulateErrors(t *testing.T) { func TestLoadGitHistoryCommits_ReturnsFatalProgressErrors(t *testing.T) { originalExtract := extractHistoryFromFile originalPopulate := populateHistory + originalPopulateDetailed := populateHistoryDetailed t.Cleanup(func() { extractHistoryFromFile = originalExtract populateHistory = originalPopulate + populateHistoryDetailed = originalPopulateDetailed }) extractHistoryFromFile = func(repoDirectory, filePath string, @@ -135,6 +145,13 @@ func TestLoadGitHistoryCommits_ReturnsFatalProgressErrors(t *testing.T) { errorChan <- model.ProgressError{Message: "unable to parse original document", Fatal: true} return commitHistory, nil } + populateHistoryDetailed = func(commitHistory []*model.Commit, + progressChan chan *model.ProgressUpdate, errorChan chan model.ProgressError, opts git.HistoryOptions, + breakingConfig *whatChangedModel.BreakingRulesConfig, + ) (*git.HistoryBuildResult, []error) { + errorChan <- model.ProgressError{Message: "unable to parse original document", Fatal: true} + return &git.HistoryBuildResult{Commits: commitHistory}, nil + } commits, err := loadGitHistoryCommits("..", "sample-specs/petstorev3.json", summaryOpts{}, nil) @@ -145,8 +162,10 @@ func TestLoadGitHistoryCommits_ReturnsFatalProgressErrors(t *testing.T) { func TestLoadGitHubCommits_ReturnsProcessErrors(t *testing.T) { originalProcess := processGithubRepo + originalProcessDetailed := processGithubRepoDetailed t.Cleanup(func() { processGithubRepo = originalProcess + processGithubRepoDetailed = originalProcessDetailed }) processGithubRepo = func(username, repo, filePath string, @@ -155,6 +174,12 @@ func TestLoadGitHubCommits_ReturnsProcessErrors(t *testing.T) { ) ([]*model.Commit, []error) { return nil, []error{errors.New("unable to build model")} } + processGithubRepoDetailed = func(username, repo, filePath string, + progressChan chan *model.ProgressUpdate, errorChan chan model.ProgressError, + opts git.HistoryOptions, breakingConfig *whatChangedModel.BreakingRulesConfig, + ) (*git.HistoryBuildResult, []error) { + return nil, []error{errors.New("unable to build model")} + } commits, err := loadGitHubCommits("https://github.com/oai/openapi-specification/blob/main/examples/v3.0/petstore.yaml", summaryOpts{}, nil) @@ -900,9 +925,11 @@ components: func TestNewSummaryCommand_NoComparableHistoryPrintsPriorVersionMessage(t *testing.T) { originalExtract := extractHistoryFromFile originalPopulate := populateHistory + originalPopulateDetailed := populateHistoryDetailed t.Cleanup(func() { extractHistoryFromFile = originalExtract populateHistory = originalPopulate + populateHistoryDetailed = originalPopulateDetailed }) extractHistoryFromFile = func(repoDirectory, filePath string, @@ -916,6 +943,12 @@ func TestNewSummaryCommand_NoComparableHistoryPrintsPriorVersionMessage(t *testi ) ([]*model.Commit, []error) { return []*model.Commit{}, nil } + populateHistoryDetailed = func(commitHistory []*model.Commit, + progressChan chan *model.ProgressUpdate, errorChan chan model.ProgressError, opts git.HistoryOptions, + breakingConfig *whatChangedModel.BreakingRulesConfig, + ) (*git.HistoryBuildResult, []error) { + return &git.HistoryBuildResult{}, nil + } cmd := testRootCmd(GetSummaryCommand(), "--no-logo", "--no-color", "..", "sample-specs/petstorev3.json") output := captureStdout(t, func() { diff --git a/git/github.go b/git/github.go index bb80ffc..1955e85 100644 --- a/git/github.go +++ b/git/github.go @@ -30,7 +30,20 @@ func convertGitHubRevisionsIntoModel(revisions []*doctorgithub.FileRevision, fil progressChan chan *model.ProgressUpdate, progressErrorChan chan model.ProgressError, opts HistoryOptions, breakingConfig *whatChangedModel.BreakingRulesConfig, ) ([]*model.Commit, []error) { + result, errs := convertGitHubRevisionsIntoModelDetailed(revisions, filePath, progressChan, progressErrorChan, opts, breakingConfig) + if result == nil { + return nil, errs + } + return result.Commits, errs +} + +func convertGitHubRevisionsIntoModelDetailed(revisions []*doctorgithub.FileRevision, filePath string, + progressChan chan *model.ProgressUpdate, progressErrorChan chan model.ProgressError, + opts HistoryOptions, breakingConfig *whatChangedModel.BreakingRulesConfig, +) (*HistoryBuildResult, []error) { normalized := make([]*model.Commit, 0, len(revisions)) + var skippedCommits []string + skippedSeen := make(map[string]struct{}) if len(revisions) > 0 { model.SendProgressUpdate("converting commits", @@ -41,6 +54,10 @@ func convertGitHubRevisionsIntoModel(revisions []*doctorgithub.FileRevision, fil if len(revision.FileBytes) == 0 { model.SendProgressWarning("converting commits", fmt.Sprintf("Skipping commit %s because GitHub returned empty file contents", revision.Commit.SHA), progressChan) + if _, ok := skippedSeen[revision.Commit.SHA]; !ok { + skippedSeen[revision.Commit.SHA] = struct{}{} + skippedCommits = append(skippedCommits, revision.Commit.SHA) + } continue } commit := &model.Commit{ @@ -62,29 +79,42 @@ func convertGitHubRevisionsIntoModel(revisions []*doctorgithub.FileRevision, fil model.SendProgressUpdate("converting commits", "building data models...", false, progressChan) } - normalized, errs = buildCommitChangelog(normalized, progressChan, progressErrorChan, opts, breakingConfig) + result, errs := buildCommitChangelogDetailed(normalized, progressChan, progressErrorChan, opts, breakingConfig) + if result == nil { + return nil, errs + } + for _, hash := range result.SkippedCommits { + if _, ok := skippedSeen[hash]; ok { + continue + } + skippedSeen[hash] = struct{}{} + skippedCommits = append(skippedCommits, hash) + } if len(errs) > 0 { model.SendProgressError("converting commits", fmt.Sprintf("%d errors detected when normalizing", len(errs)), progressErrorChan) } else { - if len(normalized) > 0 { + if len(result.Commits) > 0 { model.SendProgressUpdate("converting commits", - fmt.Sprintf("Success: %d commits normalized", len(normalized)), true, progressChan) + fmt.Sprintf("Success: %d commits normalized", len(result.Commits)), true, progressChan) } else { model.SendFatalError("converting commits", "no commits were normalized, please check the URL/Path", progressErrorChan) } } - return normalized, errs + return &HistoryBuildResult{ + Commits: result.Commits, + SkippedCommits: skippedCommits, + }, errs } // ProcessGithubRepo fetches file history from GitHub and builds the commit // changelog. Set opts.KeepComparable to preserve revisions even when the // legacy libopenapi diff is empty. -func ProcessGithubRepo(username, repo, filePath string, +func ProcessGithubRepoDetailed(username, repo, filePath string, progressChan chan *model.ProgressUpdate, errorChan chan model.ProgressError, opts HistoryOptions, breakingConfig *whatChangedModel.BreakingRulesConfig, -) ([]*model.Commit, []error) { +) (*HistoryBuildResult, []error) { if username == "" || repo == "" || filePath == "" { err := errors.New("please supply valid github username/repo and filepath") model.SendProgressError("git", err.Error(), errorChan) @@ -125,7 +155,7 @@ func ProcessGithubRepo(username, repo, filePath string, model.SendProgressUpdate("git", fmt.Sprintf("fetched %d github revisions", len(revisions)), true, progressChan) - commitHistory, errs := convertGitHubRevisionsIntoModel(revisions, filePath, progressChan, errorChan, opts, breakingConfig) + commitHistory, errs := convertGitHubRevisionsIntoModelDetailed(revisions, filePath, progressChan, errorChan, opts, breakingConfig) if errs != nil { for _, err := range errs { model.SendProgressError("git", err.Error(), errorChan) @@ -134,3 +164,14 @@ func ProcessGithubRepo(username, repo, filePath string, } return commitHistory, nil } + +func ProcessGithubRepo(username, repo, filePath string, + progressChan chan *model.ProgressUpdate, errorChan chan model.ProgressError, + opts HistoryOptions, breakingConfig *whatChangedModel.BreakingRulesConfig, +) ([]*model.Commit, []error) { + result, errs := ProcessGithubRepoDetailed(username, repo, filePath, progressChan, errorChan, opts, breakingConfig) + if result == nil { + return nil, errs + } + return result.Commits, errs +} diff --git a/git/github_test.go b/git/github_test.go index 4064f89..686ddb6 100644 --- a/git/github_test.go +++ b/git/github_test.go @@ -166,3 +166,30 @@ func TestConvertGitHubRevisionsIntoModel_ReturnsBuildErrors(t *testing.T) { assert.NotNil(t, result) require.NotEmpty(t, errs) } + +func TestConvertGitHubRevisionsIntoModelDetailed_SkipsInvalidCommitAndUsesNearestPriorValid(t *testing.T) { + progressChan, errorChan := progressChans() + + older := []byte("openapi: 3.0.3\ninfo:\n title: test\n version: \"1.0.0\"\npaths: {}\n") + invalid := []byte("swagger: \"2.0\"\ninfo:\n title: broken\n version: \"1.1.0\"\npaths: {}\n") + newer := []byte("openapi: 3.0.3\ninfo:\n title: test\n version: \"1.2.0\"\npaths:\n /pets:\n get:\n responses:\n \"200\":\n description: ok\n") + + revisions := []*doctorgithub.FileRevision{ + makeDoctorRevision("ccc333", "new", time.Now(), newer), + makeDoctorRevision("bbb222", "bad", time.Now().Add(-time.Hour), invalid), + makeDoctorRevision("aaa111", "old", time.Now().Add(-2*time.Hour), older), + } + + result, errs := convertGitHubRevisionsIntoModelDetailed(revisions, "spec.yaml", progressChan, errorChan, HistoryOptions{ + KeepComparable: true, + }, nil) + + require.Empty(t, errs) + require.NotNil(t, result) + require.Len(t, result.SkippedCommits, 1) + assert.Equal(t, "bbb222", result.SkippedCommits[0]) + require.Len(t, result.Commits, 2) + assert.Equal(t, "ccc333", result.Commits[0].Hash) + assert.Equal(t, "aaa111", result.Commits[1].Hash) + assert.Equal(t, string(result.Commits[1].Data), string(result.Commits[0].OldData)) +} diff --git a/git/options.go b/git/options.go index 4d0fe52..28ce1b4 100644 --- a/git/options.go +++ b/git/options.go @@ -3,6 +3,8 @@ package git +import "github.com/pb33f/openapi-changes/model" + // HistoryOptions controls how git history is fetched, traversed, and compared. // BreakingConfig is intentionally kept as a separate parameter in function // signatures — it controls comparison semantics, not history traversal. @@ -17,3 +19,10 @@ type HistoryOptions struct { GlobalRevisions bool // ExtractHistoryFromFile only BaseCommit string } + +// HistoryBuildResult contains the comparable commit history produced by the +// git/GitHub normalization layer plus any skipped revisions. +type HistoryBuildResult struct { + Commits []*model.Commit + SkippedCommits []string +} diff --git a/git/read_local.go b/git/read_local.go index 3023071..57608a1 100644 --- a/git/read_local.go +++ b/git/read_local.go @@ -140,10 +140,10 @@ func ExtractHistoryFromFile(repoDirectory, filePath string, // PopulateHistory reads file data from git for each commit, then builds the // changelog. Set opts.KeepComparable to preserve revisions even when the legacy // libopenapi diff is empty (used by the doctor/changerator-based commands). -func PopulateHistory(commitHistory []*model.Commit, +func PopulateHistoryDetailed(commitHistory []*model.Commit, progressChan chan *model.ProgressUpdate, errorChan chan model.ProgressError, opts HistoryOptions, breakingConfig *whatChangedModel.BreakingRulesConfig, -) ([]*model.Commit, []error) { +) (*HistoryBuildResult, []error) { for c := range commitHistory { var err error commitHistory[c].Data, err = readFile(commitHistory[c].RepoDirectory, commitHistory[c].Hash, commitHistory[c].FilePath) @@ -160,16 +160,27 @@ func PopulateHistory(commitHistory []*model.Commit, commitHistory = commitHistory[0 : opts.Limit+1] } - cleaned, errs := buildCommitChangelog(commitHistory, progressChan, errorChan, opts, breakingConfig) + result, errs := buildCommitChangelogDetailed(commitHistory, progressChan, errorChan, opts, breakingConfig) if len(errs) > 0 { model.SendProgressError("git", fmt.Sprintf("%d error(s) found building commit change log", len(errs)), errorChan) - return cleaned, errs + return result, errs } model.SendProgressUpdate("populated", - fmt.Sprintf("%d commits processed and populated", len(cleaned)), true, progressChan) - return cleaned, nil + fmt.Sprintf("%d commits processed and populated", len(result.Commits)), true, progressChan) + return result, nil +} + +func PopulateHistory(commitHistory []*model.Commit, + progressChan chan *model.ProgressUpdate, errorChan chan model.ProgressError, + opts HistoryOptions, breakingConfig *whatChangedModel.BreakingRulesConfig, +) ([]*model.Commit, []error) { + result, errs := PopulateHistoryDetailed(commitHistory, progressChan, errorChan, opts, breakingConfig) + if result == nil { + return nil, errs + } + return result.Commits, errs } // BuildChangelog compares consecutive commits and populates each with parsed @@ -179,15 +190,22 @@ func BuildChangelog(commitHistory []*model.Commit, progressChan chan *model.ProgressUpdate, errorChan chan model.ProgressError, opts HistoryOptions, breakingConfig *whatChangedModel.BreakingRulesConfig, ) ([]*model.Commit, []error) { - return buildCommitChangelog(commitHistory, progressChan, errorChan, opts, breakingConfig) + result, errs := buildCommitChangelogDetailed(commitHistory, progressChan, errorChan, opts, breakingConfig) + if result == nil { + return nil, errs + } + return result.Commits, errs } -func buildCommitChangelog(commitHistory []*model.Commit, +func buildCommitChangelogDetailed(commitHistory []*model.Commit, progressChan chan *model.ProgressUpdate, errorChan chan model.ProgressError, opts HistoryOptions, breakingConfig *whatChangedModel.BreakingRulesConfig, -) ([]*model.Commit, []error) { +) (*HistoryBuildResult, []error) { var changeErrors []error var cleaned []*model.Commit + var skippedCommits []string + skippedSeen := make(map[string]struct{}) + comparableCount := 0 // apply breaking rules configuration before comparisons if breakingConfig != nil { @@ -239,109 +257,95 @@ func buildCommitChangelog(commitHistory []*model.Commit, } } + var previousComparable *model.Commit for c := len(commitHistory) - 1; c > -1; c-- { - var oldBits, newBits []byte - var oldRevision, newRevision string - if len(commitHistory) == c+1 { - newBits = commitHistory[c].Data - newRevision = commitHistory[c].Hash - - // Obtain data from the previous commit and fail gracefully, if git - // errors. This might happen when the file does not exist in the git - // history. - oldRevision = fmt.Sprintf("%s~1", commitHistory[c].Hash) - oldBits, _ = readFile(commitHistory[c].RepoDirectory, oldRevision, commitHistory[c].FilePath) - } else { - oldBits = commitHistory[c+1].Data - oldRevision = commitHistory[c+1].Hash - commitHistory[c].OldData = oldBits - newBits = commitHistory[c].Data - newRevision = commitHistory[c].Hash + commit := commitHistory[c] + newRevision := commit.Hash + newBits := commit.Data + newDocConfig, configErr := BuildRevisionDocumentConfiguration(revisionContext, newRevision, docConfig) + if configErr != nil { + model.SendFatalError("building models", fmt.Sprintf("unable to configure modified document '%s': %s", commit.FilePath, configErr.Error()), errorChan) + changeErrors = append(changeErrors, configErr) + return nil, changeErrors + } + newDoc, buildErr := libopenapi.NewDocumentWithConfiguration(newBits, newDocConfig) + if buildErr != nil { + warnSkippedCommit(commit, fmt.Sprintf("unable to parse modified document '%s': %s", commit.FilePath, buildErr.Error()), progressChan, &skippedCommits, skippedSeen) + continue } - var oldDoc, newDoc libopenapi.Document - - if len(oldBits) > 0 && len(newBits) > 0 { - oldDocConfig, configErr := BuildRevisionDocumentConfiguration(revisionContext, oldRevision, docConfig) - if configErr != nil { - model.SendFatalError("building models", fmt.Sprintf("unable to configure original document '%s': %s", commitHistory[c].FilePath, configErr.Error()), errorChan) - changeErrors = append(changeErrors, configErr) - return nil, changeErrors - } - oldDoc, err = libopenapi.NewDocumentWithConfiguration(oldBits, oldDocConfig) - - if err != nil { - model.SendFatalError("building models", fmt.Sprintf("unable to parse original document '%s': %s", commitHistory[c].FilePath, err.Error()), errorChan) - changeErrors = append(changeErrors, err) - return nil, changeErrors - } else { - model.SendProgressUpdate("building models", - fmt.Sprintf("Building original model for commit %s", commitHistory[c].Hash[0:6]), false, progressChan) - } - newDocConfig, configErr := BuildRevisionDocumentConfiguration(revisionContext, newRevision, docConfig) - if configErr != nil { - model.SendFatalError("building models", fmt.Sprintf("unable to configure modified document '%s': %s", commitHistory[c].FilePath, configErr.Error()), errorChan) - changeErrors = append(changeErrors, configErr) - return nil, changeErrors - } - newDoc, err = libopenapi.NewDocumentWithConfiguration(newBits, newDocConfig) - if err != nil { - model.SendProgressError("building models", fmt.Sprintf("unable to parse modified document '%s': %s", commitHistory[c].FilePath, err.Error()), errorChan) - changeErrors = append(changeErrors, err) - } - - if oldDoc != nil && newDoc != nil { - changes, errs := libopenapi.CompareDocuments(oldDoc, newDoc) - - if errs != nil { - model.SendProgressError("building models", fmt.Sprintf("Error thrown when comparing: %s", errs.Error()), errorChan) - changeErrors = append(changeErrors, errs) - } - commitHistory[c].Changes = changes - } else { - model.SendProgressError("building models", "No OpenAPI documents can be compared!", errorChan) - changeErrors = append(changeErrors, errors.New("no OpenAPI documents can be compared")) - } + commit.Document = newDoc + if revisionContext != nil && revisionContext.DocumentRewriter != nil { + commit.DocumentRewriters = []model.DocumentPathRewriter{revisionContext.DocumentRewriter} } - if len(oldBits) == 0 && len(newBits) > 0 { - if commitHistory[c].RepoDirectory != "" { + + if previousComparable == nil { + if c == len(commitHistory)-1 && commit.RepoDirectory != "" { model.SendProgressWarning("building models", fmt.Sprintf("Commit %s is the first version of '%s' — no prior version to compare against, skipping", - commitHistory[c].Hash, commitHistory[c].FilePath), progressChan) - } - newDocConfig, configErr := BuildRevisionDocumentConfiguration(revisionContext, newRevision, docConfig) - if configErr != nil { - model.SendFatalError("building models", fmt.Sprintf("unable to configure modified document '%s': %s", commitHistory[c].FilePath, configErr.Error()), errorChan) - changeErrors = append(changeErrors, configErr) - return nil, changeErrors - } - newDoc, err = libopenapi.NewDocumentWithConfiguration(newBits, newDocConfig) - if err != nil { - model.SendFatalError("building models", fmt.Sprintf("unable to create OpenAPI modified document: %s", err.Error()), errorChan) - changeErrors = append(changeErrors, err) - return nil, changeErrors + commit.Hash, commit.FilePath), progressChan) } + cleaned = append(cleaned, commit) + previousComparable = commit + continue } - if newDoc != nil { - commitHistory[c].Document = newDoc - } - if oldDoc != nil { - commitHistory[c].OldDocument = oldDoc - } - if revisionContext != nil && revisionContext.DocumentRewriter != nil { - commitHistory[c].DocumentRewriters = []model.DocumentPathRewriter{revisionContext.DocumentRewriter} + + commit.OldData = previousComparable.Data + commit.OldDocument = previousComparable.Document + changes, compareErr := libopenapi.CompareDocuments(previousComparable.Document, newDoc) + if compareErr != nil { + commit.OldData = nil + commit.OldDocument = nil + warnSkippedCommit(commit, fmt.Sprintf("error comparing against prior comparable commit %s: %s", previousComparable.Hash, compareErr.Error()), progressChan, &skippedCommits, skippedSeen) + continue } - // Preserve the oldest entry as a sentinel when there is no prior version. - // The legacy commands keep only revisions with libopenapi changes, while - // the new doctor-based commands keep every revision with comparable docs. - if c == len(commitHistory)-1 || commitHistory[c].Changes != nil || (opts.KeepComparable && oldDoc != nil && newDoc != nil) { - cleaned = append(cleaned, commitHistory[c]) + + comparableCount++ + commit.Changes = changes + + // Preserve the oldest comparable entry as a sentinel when there is no + // prior version. The legacy commands keep only revisions with libopenapi + // changes, while the new doctor-based commands keep every comparable + // revision with documents. + if commit.Changes != nil || opts.KeepComparable { + cleaned = append(cleaned, commit) } + previousComparable = commit } for i, j := 0, len(cleaned)-1; i < j; i, j = i+1, j-1 { cleaned[i], cleaned[j] = cleaned[j], cleaned[i] } - return cleaned, changeErrors + reverseStrings(skippedCommits) + if comparableCount == 0 && len(skippedCommits) > 0 { + changeErrors = append(changeErrors, fmt.Errorf("all %d candidate commits failed to build or compare", len(skippedCommits))) + } + return &HistoryBuildResult{ + Commits: cleaned, + SkippedCommits: skippedCommits, + }, changeErrors +} + +func warnSkippedCommit(commit *model.Commit, reason string, progressChan chan *model.ProgressUpdate, skippedCommits *[]string, seen map[string]struct{}) { + if commit == nil { + return + } + if commit.Hash != "" { + if _, ok := seen[commit.Hash]; !ok { + seen[commit.Hash] = struct{}{} + *skippedCommits = append(*skippedCommits, commit.Hash) + } + } + message := reason + if commit.Hash != "" { + message = fmt.Sprintf("Skipping commit %s: %s", commit.Hash, reason) + } + model.SendProgressWarning("building models", message, progressChan) +} + +func reverseStrings(values []string) { + for i, j := 0, len(values)-1; i < j; i, j = i+1, j-1 { + values[i], values[j] = values[j], values[i] + } } func ExtractPathAndFile(location string) (string, string) { diff --git a/git/read_local_test.go b/git/read_local_test.go index ee7d258..4e57a4e 100644 --- a/git/read_local_test.go +++ b/git/read_local_test.go @@ -207,6 +207,32 @@ func TestPopulateHistory_UsesRevisionScopedSiblingRefs(t *testing.T) { require.NotEmpty(t, cleaned[0].DocumentRewriters) } +func TestPopulateHistoryDetailed_SkipsInvalidCommitAndUsesNearestPriorValid(t *testing.T) { + fixture := testutil.CreateInvalidHistoryGitSpecRepo(t) + progressChan := make(chan *model.ProgressUpdate, 64) + errorChan := make(chan model.ProgressError, 64) + + history, errs := ExtractHistoryFromFile(fixture.RepoDir, fixture.FileName, progressChan, errorChan, HistoryOptions{Limit: 0, LimitTime: -1}) + require.Empty(t, errs) + require.Len(t, history, 3) + + result, errs := PopulateHistoryDetailed(history, progressChan, errorChan, HistoryOptions{ + KeepComparable: true, + }, nil) + require.Empty(t, errs) + require.NotNil(t, result) + require.Len(t, result.SkippedCommits, 1) + assert.Equal(t, fixture.InvalidHash, result.SkippedCommits[0]) + require.Len(t, result.Commits, 2) + assert.Equal(t, fixture.NewestHash, result.Commits[0].Hash) + assert.Equal(t, fixture.OldestHash, result.Commits[1].Hash) + require.NotNil(t, result.Commits[0].OldDocument) + require.NotNil(t, result.Commits[0].Document) + assert.Equal(t, string(result.Commits[1].Data), string(result.Commits[0].OldData)) + assert.NotNil(t, result.Commits[1].Document) + assert.Nil(t, result.Commits[1].OldDocument) +} + func runGit(t *testing.T, dir string, args ...string) { t.Helper() diff --git a/internal/testutil/gitrepo.go b/internal/testutil/gitrepo.go index 1782035..db31a3a 100644 --- a/internal/testutil/gitrepo.go +++ b/internal/testutil/gitrepo.go @@ -4,6 +4,7 @@ package testutil import ( + "bytes" "os" "os/exec" "path/filepath" @@ -108,3 +109,87 @@ paths: return repoDir, "spec.yaml" } + +type InvalidHistoryRepo struct { + RepoDir string + FileName string + OldestHash string + InvalidHash string + NewestHash string + ExpectedChange string +} + +func CreateInvalidHistoryGitSpecRepo(t testing.TB) InvalidHistoryRepo { + t.Helper() + + repoDir := t.TempDir() + + RunGit(t, repoDir, "init") + RunGit(t, repoDir, "config", "user.name", "Test User") + RunGit(t, repoDir, "config", "user.email", "test@example.com") + + fileName := "openapi.yaml" + specPath := filepath.Join(repoDir, fileName) + + first := `openapi: 3.0.3 +info: + title: Test API + version: "1.0.0" +paths: {} +` + invalid := `swagger: "2.0" +info: + title: Broken API + version: "1.1.0" +paths: {} +` + third := `openapi: 3.0.3 +info: + title: Test API + version: "1.2.0" +paths: + /pets: + get: + responses: + "200": + description: ok +` + + require.NoError(t, os.WriteFile(specPath, []byte(first), 0o644)) + RunGit(t, repoDir, "add", fileName) + RunGit(t, repoDir, "commit", "-m", "valid first") + oldestHash := gitOutput(t, repoDir, "rev-parse", "--short", "HEAD") + + require.NoError(t, os.WriteFile(specPath, []byte(invalid), 0o644)) + RunGit(t, repoDir, "add", fileName) + RunGit(t, repoDir, "commit", "-m", "invalid middle") + invalidHash := gitOutput(t, repoDir, "rev-parse", "--short", "HEAD") + + require.NoError(t, os.WriteFile(specPath, []byte(third), 0o644)) + RunGit(t, repoDir, "add", fileName) + RunGit(t, repoDir, "commit", "-m", "valid last") + newestHash := gitOutput(t, repoDir, "rev-parse", "--short", "HEAD") + + return InvalidHistoryRepo{ + RepoDir: repoDir, + FileName: fileName, + OldestHash: oldestHash, + InvalidHash: invalidHash, + NewestHash: newestHash, + ExpectedChange: "$.paths['/pets']", + } +} + +func gitOutput(t testing.TB, dir string, args ...string) string { + t.Helper() + + cmd := exec.Command("git", args...) + cmd.Dir = dir + var stdout bytes.Buffer + var stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + err := cmd.Run() + require.NoError(t, err, "git %v failed: %s", args, stderr.String()) + return string(bytes.TrimSpace(stdout.Bytes())) +} diff --git a/model/report.go b/model/report.go index 0876146..3cd6bc7 100644 --- a/model/report.go +++ b/model/report.go @@ -83,10 +83,16 @@ type FlatReport struct { Commit *Commit `gorm:"foreignKey:ID" json:"commitDetails,omitempty"` } +type HistoricalReportMetaData struct { + Partial bool `json:"partial,omitempty"` + SkippedCommits []string `json:"skippedCommits,omitempty"` +} + type FlatHistoricalReport struct { - GitRepoPath string `json:"gitRepoPath"` - GitFilePath string `json:"gitFilePath"` - Filename string `json:"filename"` - DateGenerated string `json:"dateGenerated,omitempty"` - Reports []*FlatReport `json:"reports" ` + GitRepoPath string `json:"gitRepoPath"` + GitFilePath string `json:"gitFilePath"` + Filename string `json:"filename"` + DateGenerated string `json:"dateGenerated,omitempty"` + MetaData *HistoricalReportMetaData `json:"metaData,omitempty"` + Reports []*FlatReport `json:"reports" ` } From da5101da22dafcddebfe216c58089f929dc92a33 Mon Sep 17 00:00:00 2001 From: quobix Date: Tue, 14 Apr 2026 08:01:37 -0400 Subject: [PATCH 2/2] Address #227 fully. --- cmd/report.go | 51 ++++++------------ cmd/report_test.go | 91 +++++++++++++++++++++++++++++++ cmd/summary_test.go | 19 +++++++ cmd/test_helpers_test.go | 11 ++++ git/read_local.go | 69 ++++++++++++++++++++++-- git/read_local_test.go | 100 +++++++++++++++++++++++++++++++++++ go.mod | 2 +- go.sum | 2 + internal/testutil/gitrepo.go | 57 ++++++++++++++++++++ 9 files changed, 361 insertions(+), 41 deletions(-) diff --git a/cmd/report.go b/cmd/report.go index 5b78b12..dabb914 100644 --- a/cmd/report.go +++ b/cmd/report.go @@ -32,8 +32,9 @@ func changerateCommit(commit *model.Commit, breakingConfig *whatChangedModel.Bre } type historicalFlattenResult struct { - Reports []*model.FlatReport - SkippedCommits []string + Reports []*model.FlatReport + SkippedCommits []string + SuccessfulComparisons int } func changerateAndFlattenHistory(commits []*model.Commit, breakingConfig *whatChangedModel.BreakingRulesConfig) (*historicalFlattenResult, error) { @@ -72,8 +73,9 @@ func changerateAndFlattenHistory(commits []*model.Commit, breakingConfig *whatCh fmt.Fprintf(os.Stderr, "warning: %d commits failed to render report data\n", len(renderErrors)) } return &historicalFlattenResult{ - Reports: reports, - SkippedCommits: skippedCommits, + Reports: reports, + SkippedCommits: skippedCommits, + SuccessfulComparisons: processedComparables, }, nil } @@ -102,35 +104,7 @@ func runGitHistoryReport(gitPath, filePath string, opts summaryOpts, breakingCon if err != nil { return nil, err } - if loaded == nil { - return nil, nil - } - if len(loaded.Commits) == 0 && len(loaded.SkippedCommits) == 0 { - return nil, nil - } - history, err := changerateAndFlattenHistory(loaded.Commits, breakingConfig) - if err != nil { - return nil, err - } - skippedCommits := mergeSkippedCommitHashes(loaded.SkippedCommits, history.SkippedCommits) - if len(history.Reports) == 0 && len(skippedCommits) > 0 { - return nil, fmt.Errorf("all %d candidate commits were skipped or failed to render", len(skippedCommits)) - } - - report := &model.FlatHistoricalReport{ - GitRepoPath: gitPath, - GitFilePath: filePath, - Filename: path.Base(filePath), - DateGenerated: time.Now().Format(time.RFC3339), - Reports: history.Reports, - } - if len(skippedCommits) > 0 { - report.MetaData = &model.HistoricalReportMetaData{ - Partial: true, - SkippedCommits: skippedCommits, - } - } - return report, nil + return buildHistoricalReport(gitPath, filePath, loaded, breakingConfig) } func runGithubHistoryReport(rawURL string, opts summaryOpts, breakingConfig *whatChangedModel.BreakingRulesConfig) (*model.FlatHistoricalReport, error) { @@ -147,23 +121,30 @@ func runGithubHistoryReport(rawURL string, opts summaryOpts, breakingConfig *wha if err != nil { return nil, err } + return buildHistoricalReport(fmt.Sprintf("%s/%s", user, repo), filePath, loaded, breakingConfig) +} + +func buildHistoricalReport(repoPath, filePath string, loaded *loadedHistoryResult, + breakingConfig *whatChangedModel.BreakingRulesConfig, +) (*model.FlatHistoricalReport, error) { if loaded == nil { return nil, nil } if len(loaded.Commits) == 0 && len(loaded.SkippedCommits) == 0 { return nil, nil } + history, err := changerateAndFlattenHistory(loaded.Commits, breakingConfig) if err != nil { return nil, err } skippedCommits := mergeSkippedCommitHashes(loaded.SkippedCommits, history.SkippedCommits) - if len(history.Reports) == 0 && len(skippedCommits) > 0 { + if len(history.Reports) == 0 && history.SuccessfulComparisons == 0 && len(skippedCommits) > 0 { return nil, fmt.Errorf("all %d candidate commits were skipped or failed to render", len(skippedCommits)) } report := &model.FlatHistoricalReport{ - GitRepoPath: fmt.Sprintf("%s/%s", user, repo), + GitRepoPath: repoPath, GitFilePath: filePath, Filename: path.Base(filePath), DateGenerated: time.Now().Format(time.RFC3339), diff --git a/cmd/report_test.go b/cmd/report_test.go index 1bdf61b..95e241d 100644 --- a/cmd/report_test.go +++ b/cmd/report_test.go @@ -324,6 +324,42 @@ func TestRunGitHistoryReport_BasePathUsesRevisionScopedSiblingRefs(t *testing.T) assert.Contains(t, content, `"path":"$.paths['/thing'].get.responses['200'].content['application/json'].schema"`) } +func TestRunGitHistoryReport_LimitOneComparesAgainstParent(t *testing.T) { + repoDir := createGitSpecRepoForFile(t, "openapi.yaml") + + report, err := runGitHistoryReport(repoDir, "openapi.yaml", summaryOpts{ + base: repoDir, + noColor: true, + limit: 1, + limitTime: -1, + }, nil) + + require.NoError(t, err) + require.NotNil(t, report) + require.Len(t, report.Reports, 1) + assert.Equal(t, gitOutputInDir(t, repoDir, "rev-parse", "--short", "HEAD"), report.Reports[0].Commit.Hash) + assert.NotEmpty(t, report.Reports[0].Changes) +} + +func TestRunGitHistoryReport_BaseCommitUsesExcludedParentAsBaseline(t *testing.T) { + repoDir := createGitSpecRepoForFile(t, "openapi.yaml") + baseCommit := gitOutputInDir(t, repoDir, "rev-parse", "--short", "HEAD~2") + + report, err := runGitHistoryReport(repoDir, "openapi.yaml", summaryOpts{ + base: repoDir, + baseCommit: baseCommit, + noColor: true, + limitTime: -1, + }, nil) + + require.NoError(t, err) + require.NotNil(t, report) + require.Len(t, report.Reports, 2) + assert.Equal(t, gitOutputInDir(t, repoDir, "rev-parse", "--short", "HEAD"), report.Reports[0].Commit.Hash) + assert.Equal(t, gitOutputInDir(t, repoDir, "rev-parse", "--short", "HEAD~1"), report.Reports[1].Commit.Hash) + assert.NotEmpty(t, report.Reports[1].Changes) +} + func TestRunGitHistoryReport_PartialHistoryIncludesMetaData(t *testing.T) { fixture := testutil.CreateInvalidHistoryGitSpecRepo(t) @@ -343,6 +379,23 @@ func TestRunGitHistoryReport_PartialHistoryIncludesMetaData(t *testing.T) { assert.Contains(t, string(encoded), fixture.InvalidHash) } +func TestRunGitHistoryReport_PartialHistoryWithoutChangesReturnsEmptyPartialReport(t *testing.T) { + fixture := testutil.CreateRevertedHistoryGitSpecRepo(t) + + report, err := runGitHistoryReport(fixture.RepoDir, fixture.FileName, summaryOpts{ + base: fixture.RepoDir, + noColor: true, + limitTime: -1, + }, nil) + + require.NoError(t, err) + require.NotNil(t, report) + assert.Empty(t, report.Reports) + require.NotNil(t, report.MetaData) + assert.True(t, report.MetaData.Partial) + assert.Equal(t, []string{fixture.InvalidHash}, report.MetaData.SkippedCommits) +} + func TestRunGitHistoryReport_AllInvalidHistoryFails(t *testing.T) { repoDir := t.TempDir() runGitInDir(t, repoDir, "init") @@ -412,6 +465,44 @@ paths: assert.Equal(t, []string{"bbb222"}, report.MetaData.SkippedCommits) } +func TestRunGithubHistoryReport_PartialHistoryWithoutChangesReturnsEmptyPartialReport(t *testing.T) { + originalProcess := processGithubRepoDetailed + t.Cleanup(func() { + processGithubRepoDetailed = originalProcess + }) + + processGithubRepoDetailed = func(username, repo, filePath string, + progressChan chan *model.ProgressUpdate, errorChan chan model.ProgressError, + opts git.HistoryOptions, breakingConfig *whatChangedModel.BreakingRulesConfig, + ) (*git.HistoryBuildResult, []error) { + return &git.HistoryBuildResult{ + Commits: []*model.Commit{ + mustMakeDoctorOnlyCommitFromSpecs(t, "ccc333", `openapi: 3.0.3 +info: + title: test + version: "1.0.0" +paths: {} +`, `openapi: 3.0.3 +info: + title: test + version: "1.0.0" +paths: {} +`), + }, + SkippedCommits: []string{"bbb222"}, + }, nil + } + + report, err := runGithubHistoryReport("https://github.com/oai/openapi-specification/blob/main/examples/v3.0/petstore.yaml", summaryOpts{}, nil) + + require.NoError(t, err) + require.NotNil(t, report) + assert.Empty(t, report.Reports) + require.NotNil(t, report.MetaData) + assert.True(t, report.MetaData.Partial) + assert.Equal(t, []string{"bbb222"}, report.MetaData.SkippedCommits) +} + func TestReportCommand_GitRefUsesLeftRightMode(t *testing.T) { wd, err := os.Getwd() require.NoError(t, err) diff --git a/cmd/summary_test.go b/cmd/summary_test.go index 75acf3b..55b7094 100644 --- a/cmd/summary_test.go +++ b/cmd/summary_test.go @@ -957,3 +957,22 @@ func TestNewSummaryCommand_NoComparableHistoryPrintsPriorVersionMessage(t *testi assert.Contains(t, output, "The file has no prior version to compare against") } + +func TestSummaryCommand_LimitOneStillComparesAgainstParent(t *testing.T) { + repoDir := createGitSpecRepoForFile(t, "openapi.yaml") + + cmd := testRootCmd(GetSummaryCommand(), + "--no-logo", "--no-color", + "--limit", "1", + "--base", repoDir, + repoDir, "openapi.yaml", + ) + output := captureStdout(t, func() { + require.NoError(t, cmd.Execute()) + }) + + assert.NotContains(t, output, noPriorVersionMessage) + assert.NotContains(t, output, noChangesFoundMessage) + assert.Contains(t, output, "Document Element") + assert.Contains(t, output, "paths") +} diff --git a/cmd/test_helpers_test.go b/cmd/test_helpers_test.go index 77359b3..92c4dab 100644 --- a/cmd/test_helpers_test.go +++ b/cmd/test_helpers_test.go @@ -8,6 +8,7 @@ import ( "os" "os/exec" "path/filepath" + "strings" "testing" "time" @@ -318,3 +319,13 @@ func runGitInDir(t *testing.T, dir string, args ...string) { out, err := cmd.CombinedOutput() require.NoError(t, err, "git %v failed: %s", args, string(out)) } + +func gitOutputInDir(t *testing.T, dir string, args ...string) string { + t.Helper() + + cmd := exec.Command("git", args...) + cmd.Dir = dir + out, err := cmd.CombinedOutput() + require.NoError(t, err, "git %v failed: %s", args, string(out)) + return strings.TrimSpace(string(out)) +} diff --git a/git/read_local.go b/git/read_local.go index 57608a1..601da2e 100644 --- a/git/read_local.go +++ b/git/read_local.go @@ -280,12 +280,30 @@ func buildCommitChangelogDetailed(commitHistory []*model.Commit, } if previousComparable == nil { - if c == len(commitHistory)-1 && commit.RepoDirectory != "" { - model.SendProgressWarning("building models", - fmt.Sprintf("Commit %s is the first version of '%s' — no prior version to compare against, skipping", - commit.Hash, commit.FilePath), progressChan) + baseline, baselineErr := resolvePriorComparableBaseline(commit, newDoc, revisionContext, docConfig) + if baselineErr != nil { + model.SendFatalError("building models", fmt.Sprintf("unable to configure original document '%s': %s", commit.FilePath, baselineErr.Error()), errorChan) + changeErrors = append(changeErrors, baselineErr) + return nil, changeErrors + } + if baseline == nil { + if c == len(commitHistory)-1 && commit.RepoDirectory != "" { + model.SendProgressWarning("building models", + fmt.Sprintf("Commit %s is the first version of '%s' — no prior version to compare against, skipping", + commit.Hash, commit.FilePath), progressChan) + } + cleaned = append(cleaned, commit) + previousComparable = commit + continue + } + + comparableCount++ + commit.OldData = baseline.Data + commit.OldDocument = baseline.Document + commit.Changes = baseline.Changes + if commit.Changes != nil || opts.KeepComparable { + cleaned = append(cleaned, commit) } - cleaned = append(cleaned, commit) previousComparable = commit continue } @@ -325,6 +343,47 @@ func buildCommitChangelogDetailed(commitHistory []*model.Commit, }, changeErrors } +type priorComparableBaseline struct { + Data []byte + Document libopenapi.Document + Changes *whatChangedModel.DocumentChanges +} + +func resolvePriorComparableBaseline(commit *model.Commit, newDoc libopenapi.Document, + revisionContext *RevisionDocumentContext, docConfig *datamodel.DocumentConfiguration, +) (*priorComparableBaseline, error) { + if commit == nil || commit.RepoDirectory == "" || newDoc == nil { + return nil, nil + } + + for revision := fmt.Sprintf("%s~1", commit.Hash); ; revision = fmt.Sprintf("%s~1", revision) { + oldBits, err := readFile(commit.RepoDirectory, revision, commit.FilePath) + if err != nil { + return nil, nil + } + + oldDocConfig, err := BuildRevisionDocumentConfiguration(revisionContext, revision, docConfig) + if err != nil { + return nil, err + } + oldDoc, err := libopenapi.NewDocumentWithConfiguration(oldBits, oldDocConfig) + if err != nil { + continue + } + + changes, err := libopenapi.CompareDocuments(oldDoc, newDoc) + if err != nil { + continue + } + + return &priorComparableBaseline{ + Data: oldBits, + Document: oldDoc, + Changes: changes, + }, nil + } +} + func warnSkippedCommit(commit *model.Commit, reason string, progressChan chan *model.ProgressUpdate, skippedCommits *[]string, seen map[string]struct{}) { if commit == nil { return diff --git a/git/read_local_test.go b/git/read_local_test.go index 4e57a4e..13fcde2 100644 --- a/git/read_local_test.go +++ b/git/read_local_test.go @@ -233,6 +233,96 @@ func TestPopulateHistoryDetailed_SkipsInvalidCommitAndUsesNearestPriorValid(t *t assert.Nil(t, result.Commits[1].OldDocument) } +func TestPopulateHistoryDetailed_LimitOneComparesAgainstParent(t *testing.T) { + repoDir := t.TempDir() + runGit(t, repoDir, "init") + runGit(t, repoDir, "config", "user.name", "Test User") + runGit(t, repoDir, "config", "user.email", "test@example.com") + + fileName := "openapi.yaml" + specPath := filepath.Join(repoDir, fileName) + + require.NoError(t, os.WriteFile(specPath, []byte("openapi: 3.0.3\ninfo:\n title: first\n version: '1.0'\npaths: {}\n"), 0o644)) + runGit(t, repoDir, "add", fileName) + runGit(t, repoDir, "commit", "-m", "first") + + require.NoError(t, os.WriteFile(specPath, []byte("openapi: 3.0.3\ninfo:\n title: second\n version: '1.1'\npaths:\n /pets:\n get:\n responses:\n \"200\":\n description: ok\n"), 0o644)) + runGit(t, repoDir, "add", fileName) + runGit(t, repoDir, "commit", "-m", "second") + + require.NoError(t, os.WriteFile(specPath, []byte("openapi: 3.0.3\ninfo:\n title: third\n version: '1.2'\npaths:\n /pets:\n get:\n responses:\n \"200\":\n description: updated ok\n"), 0o644)) + runGit(t, repoDir, "add", fileName) + runGit(t, repoDir, "commit", "-m", "third") + + progressChan := make(chan *model.ProgressUpdate, 64) + errorChan := make(chan model.ProgressError, 64) + + history, errs := ExtractHistoryFromFile(repoDir, fileName, progressChan, errorChan, HistoryOptions{Limit: 1, LimitTime: -1}) + require.Empty(t, errs) + require.Len(t, history, 1) + + result, errs := PopulateHistoryDetailed(history, progressChan, errorChan, HistoryOptions{ + KeepComparable: true, + }, nil) + require.Empty(t, errs) + require.NotNil(t, result) + require.Len(t, result.Commits, 1) + + commit := result.Commits[0] + assert.Equal(t, gitOutput(t, repoDir, "rev-parse", "--short", "HEAD"), commit.Hash) + require.NotNil(t, commit.Document) + require.NotNil(t, commit.OldDocument) + require.NotNil(t, commit.Changes) + assert.Contains(t, string(commit.OldData), "title: second") +} + +func TestPopulateHistoryDetailed_BaseCommitComparesAgainstExcludedParent(t *testing.T) { + repoDir := t.TempDir() + runGit(t, repoDir, "init") + runGit(t, repoDir, "config", "user.name", "Test User") + runGit(t, repoDir, "config", "user.email", "test@example.com") + + fileName := "openapi.yaml" + specPath := filepath.Join(repoDir, fileName) + + require.NoError(t, os.WriteFile(specPath, []byte("openapi: 3.0.3\ninfo:\n title: first\n version: '1.0'\npaths: {}\n"), 0o644)) + runGit(t, repoDir, "add", fileName) + runGit(t, repoDir, "commit", "-m", "first") + + require.NoError(t, os.WriteFile(specPath, []byte("openapi: 3.0.3\ninfo:\n title: second\n version: '1.1'\npaths:\n /pets:\n get:\n responses:\n \"200\":\n description: ok\n"), 0o644)) + runGit(t, repoDir, "add", fileName) + runGit(t, repoDir, "commit", "-m", "second") + + require.NoError(t, os.WriteFile(specPath, []byte("openapi: 3.0.3\ninfo:\n title: third\n version: '1.2'\npaths:\n /pets:\n get:\n responses:\n \"200\":\n description: updated ok\n"), 0o644)) + runGit(t, repoDir, "add", fileName) + runGit(t, repoDir, "commit", "-m", "third") + + progressChan := make(chan *model.ProgressUpdate, 64) + errorChan := make(chan model.ProgressError, 64) + baseCommit := gitOutput(t, repoDir, "rev-parse", "--short", "HEAD~2") + + history, errs := ExtractHistoryFromFile(repoDir, fileName, progressChan, errorChan, HistoryOptions{ + BaseCommit: baseCommit, + LimitTime: -1, + }) + require.Empty(t, errs) + require.Len(t, history, 2) + + result, errs := PopulateHistoryDetailed(history, progressChan, errorChan, HistoryOptions{ + KeepComparable: true, + }, nil) + require.Empty(t, errs) + require.NotNil(t, result) + require.Len(t, result.Commits, 2) + + oldestLoaded := result.Commits[1] + assert.Equal(t, gitOutput(t, repoDir, "rev-parse", "--short", "HEAD~1"), oldestLoaded.Hash) + require.NotNil(t, oldestLoaded.Document) + require.NotNil(t, oldestLoaded.OldDocument) + require.NotNil(t, oldestLoaded.Changes) + assert.Contains(t, string(oldestLoaded.OldData), "title: first") +} + func runGit(t *testing.T, dir string, args ...string) { t.Helper() @@ -249,3 +339,13 @@ func mustReadTestFile(t *testing.T, path string) []byte { require.NoError(t, err, fmt.Sprintf("read test file %s", path)) return bits } + +func gitOutput(t *testing.T, dir string, args ...string) string { + t.Helper() + + cmd := exec.Command("git", args...) + cmd.Dir = dir + out, err := cmd.CombinedOutput() + require.NoError(t, err, "git %v failed: %s", args, string(out)) + return strings.TrimSpace(string(out)) +} diff --git a/go.mod b/go.mod index 38e1ad6..b5dfd27 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/google/uuid v1.6.0 github.com/mattn/go-runewidth v0.0.23 github.com/muesli/termenv v0.16.0 - github.com/pb33f/doctor v0.0.51 + github.com/pb33f/doctor v0.0.52 github.com/pb33f/libopenapi v0.36.1 github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 github.com/spf13/cobra v1.10.2 diff --git a/go.sum b/go.sum index 11b79d4..942336b 100644 --- a/go.sum +++ b/go.sum @@ -96,6 +96,8 @@ github.com/pb33f/doctor v0.0.50 h1:9DF7Ff+VyShWvyesF6ab2/g7KuxB5DqfsKsgPEKDgPg= github.com/pb33f/doctor v0.0.50/go.mod h1:a/X9otAs4JJEgRuCCNldwesU80ykIrsTN/l7fZLvamo= github.com/pb33f/doctor v0.0.51 h1:q2qqPFnmSvo5mFSRWmWM5fvrYaIA29yOT/iVWDkPMa0= github.com/pb33f/doctor v0.0.51/go.mod h1:B06Wq2Z9E8ERbvHFsrF03Js/JCfjxpL3OV9fF3c/O9w= +github.com/pb33f/doctor v0.0.52 h1:FXD8Po/xL6sTP5tyThp9+zEITjB12RYjEHljnouOY+Q= +github.com/pb33f/doctor v0.0.52/go.mod h1:B06Wq2Z9E8ERbvHFsrF03Js/JCfjxpL3OV9fF3c/O9w= github.com/pb33f/jsonpath v0.8.2 h1:Ou4C7zjYClBm97dfZjDCjdZGusJoynv/vrtiEKNfj2Y= github.com/pb33f/jsonpath v0.8.2/go.mod h1:zBV5LJW4OQOPatmQE2QdKpGQJvhDTlE5IEj6ASaRNTo= github.com/pb33f/libopenapi v0.36.0 h1:kQ73f8H0iXOj9naLxwYbKmgOBBXonJIsQbIztYjQ2lc= diff --git a/internal/testutil/gitrepo.go b/internal/testutil/gitrepo.go index db31a3a..41fe9fd 100644 --- a/internal/testutil/gitrepo.go +++ b/internal/testutil/gitrepo.go @@ -180,6 +180,63 @@ paths: } } +type RevertedHistoryRepo struct { + RepoDir string + FileName string + OldestHash string + InvalidHash string + NewestHash string +} + +func CreateRevertedHistoryGitSpecRepo(t testing.TB) RevertedHistoryRepo { + t.Helper() + + repoDir := t.TempDir() + + RunGit(t, repoDir, "init") + RunGit(t, repoDir, "config", "user.name", "Test User") + RunGit(t, repoDir, "config", "user.email", "test@example.com") + + fileName := "openapi.yaml" + specPath := filepath.Join(repoDir, fileName) + + valid := `openapi: 3.0.3 +info: + title: Test API + version: "1.0.0" +paths: {} +` + invalid := `swagger: "2.0" +info: + title: Broken API + version: "1.1.0" +paths: {} +` + + require.NoError(t, os.WriteFile(specPath, []byte(valid), 0o644)) + RunGit(t, repoDir, "add", fileName) + RunGit(t, repoDir, "commit", "-m", "valid first") + oldestHash := gitOutput(t, repoDir, "rev-parse", "--short", "HEAD") + + require.NoError(t, os.WriteFile(specPath, []byte(invalid), 0o644)) + RunGit(t, repoDir, "add", fileName) + RunGit(t, repoDir, "commit", "-m", "invalid middle") + invalidHash := gitOutput(t, repoDir, "rev-parse", "--short", "HEAD") + + require.NoError(t, os.WriteFile(specPath, []byte(valid), 0o644)) + RunGit(t, repoDir, "add", fileName) + RunGit(t, repoDir, "commit", "-m", "valid revert") + newestHash := gitOutput(t, repoDir, "rev-parse", "--short", "HEAD") + + return RevertedHistoryRepo{ + RepoDir: repoDir, + FileName: fileName, + OldestHash: oldestHash, + InvalidHash: invalidHash, + NewestHash: newestHash, + } +} + func gitOutput(t testing.TB, dir string, args ...string) string { t.Helper()