Address #227 fully.

Author: daveshanley

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),

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)

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")
+}

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))
+}

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