diff --git a/modules/git/diff_compare.go b/modules/git/diff_compare.go new file mode 100644 index 0000000000..0eba8cb541 --- /dev/null +++ b/modules/git/diff_compare.go @@ -0,0 +1,56 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later +package git + +import ( + "bytes" + "context" + "fmt" + "os" + + "forgejo.org/modules/log" + "forgejo.org/modules/util" +) + +// CheckIfDiffDiffers returns if the diff of the newCommitID and +// oldCommitID with the merge base of the base branch has changed. +// +// Informally it checks if the following two diffs are exactly the same in their +// contents, thus ignoring different commit IDs, headers and messages: +// 1. git diff --merge-base baseReference newCommitID +// 2. git diff --merge-base baseReference oldCommitID +func (repo *Repository) CheckIfDiffDiffers(base, oldCommitID, newCommitID string, env []string) (hasChanged bool, err error) { + cmd := NewCommand(repo.Ctx, "diff", "--name-only", "-z").AddDynamicArguments(newCommitID, oldCommitID, base) + stdoutReader, stdoutWriter, err := os.Pipe() + if err != nil { + return false, fmt.Errorf("unable to open pipe for to run diff: %w", err) + } + + stderr := new(bytes.Buffer) + if err := cmd.Run(&RunOpts{ + Dir: repo.Path, + Stdout: stdoutWriter, + Stderr: stderr, + PipelineFunc: func(ctx context.Context, cancel context.CancelFunc) error { + _ = stdoutWriter.Close() + defer func() { + _ = stdoutReader.Close() + }() + return util.IsEmptyReader(stdoutReader) + }, + }); err != nil { + if err == util.ErrNotEmpty { + return true, nil + } + err = ConcatenateError(err, stderr.String()) + + log.Error("Unable to run git diff on %s %s %s in %q: Error: %v", + newCommitID, oldCommitID, base, + repo.Path, + err) + + return false, fmt.Errorf("Unable to run git diff --name-only -z %s %s %s: %w", newCommitID, oldCommitID, base, err) + } + + return false, nil +} diff --git a/modules/git/diff_compare_test.go b/modules/git/diff_compare_test.go new file mode 100644 index 0000000000..433497b5c4 --- /dev/null +++ b/modules/git/diff_compare_test.go @@ -0,0 +1,421 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later +package git + +import ( + "bytes" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCheckIfDiffDiffers(t *testing.T) { + tmpDir := t.TempDir() + + err := InitRepository(t.Context(), tmpDir, false, Sha1ObjectFormat.Name()) + require.NoError(t, err) + + gitRepo, err := openRepositoryWithDefaultContext(tmpDir) + require.NoError(t, err) + defer gitRepo.Close() + + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "main").Run(&RunOpts{Dir: tmpDir})) + + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "README"), []byte("aaa"), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "README").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "initial commit").Run(&RunOpts{Dir: tmpDir})) + + t.Run("Simple fast-forward", func(t *testing.T) { + // Check that A--B--C, where A is the base branch. + + t.Run("Different diff", func(t *testing.T) { + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "a-1", "main").Run(&RunOpts{Dir: tmpDir})) + + // B commit + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "README"), []byte("bbb"), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "README").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Changes to the README").Run(&RunOpts{Dir: tmpDir})) + + // C commit + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "a-2").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "README"), []byte("ccc"), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "README").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Changes to the README").Run(&RunOpts{Dir: tmpDir})) + + changed, err := gitRepo.CheckIfDiffDiffers("main", "a-1", "a-2", nil) + require.NoError(t, err) + assert.True(t, changed) + }) + + t.Run("Same diff", func(t *testing.T) { + // Because C is a empty commit, the diff does not differ relative to the + // base branch. + + // B commit + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "a-3", "main").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "README"), []byte("bbb"), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "README").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Changes to the README").Run(&RunOpts{Dir: tmpDir})) + + // C commit + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "a-4").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "--allow-empty", "-m", "No changes to the README").Run(&RunOpts{Dir: tmpDir})) + + changed, err := gitRepo.CheckIfDiffDiffers("main", "a-3", "a-4", nil) + require.NoError(t, err) + assert.False(t, changed) + }) + }) + + t.Run("Merge-base is base reference", func(t *testing.T) { + // B + // / + // A + // \ + // C + t.Run("Different diff", func(t *testing.T) { + // B commit + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "b-1", "main").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "README"), []byte("bbb"), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "README").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Changed to the README").Run(&RunOpts{Dir: tmpDir})) + + // C commit + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "b-2", "main").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "README"), []byte("ccc"), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "README").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Changed to the README").Run(&RunOpts{Dir: tmpDir})) + + changed, err := gitRepo.CheckIfDiffDiffers("main", "b-1", "b-2", nil) + require.NoError(t, err) + assert.True(t, changed) + }) + + t.Run("Same diff", func(t *testing.T) { + // B commit + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "b-3", "main").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "README"), []byte("bbb"), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "README").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Changed to the README").Run(&RunOpts{Dir: tmpDir})) + + // C commit + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "b-4", "main").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "README"), []byte("bbb"), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "README").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Changed to the README").Run(&RunOpts{Dir: tmpDir})) + + changed, err := gitRepo.CheckIfDiffDiffers("main", "b-3", "b-4", nil) + require.NoError(t, err) + assert.False(t, changed) + }) + }) + + t.Run("Merge-base is different", func(t *testing.T) { + // B + // / + // A--D + // \ + // C + // Where D is the base reference. + + // D commit, where A is `main`. + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "main-D", "main").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "FUNFACT"), []byte("Smithy was the runner up to be Forgejo's name"), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "FUNFACT").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Forgejo history").Run(&RunOpts{Dir: tmpDir})) + + t.Run("Different diff", func(t *testing.T) { + // B commit + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "c-1", "main").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "README"), []byte("bbb"), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "README").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Changes to the README").Run(&RunOpts{Dir: tmpDir})) + + // C commit + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "c-2", "main-D").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "FUNFACT"), []byte("bbb"), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "FUNFACT").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Changes to the funfact").Run(&RunOpts{Dir: tmpDir})) + + changed, err := gitRepo.CheckIfDiffDiffers("main-D", "c-1", "c-2", nil) + require.NoError(t, err) + assert.True(t, changed) + }) + + t.Run("Same diff", func(t *testing.T) { + // B commit + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "c-3", "main").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "README"), []byte("bbb"), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "README").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Changes to the README").Run(&RunOpts{Dir: tmpDir})) + + // C commit + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "c-4", "main-D").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "README"), []byte("bbb"), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "README").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Changes to the README").Run(&RunOpts{Dir: tmpDir})) + + changed, err := gitRepo.CheckIfDiffDiffers("main-D", "c-3", "c-4", nil) + require.NoError(t, err) + assert.False(t, changed) + }) + }) + + t.Run("Merge commit", func(t *testing.T) { + // B + // / + // A - D + // \ + // C + // + // From B, it merges D where E is the merge commit : + // B---E + // / / + // A---D + // \ + // C + + t.Run("Different diff", func(t *testing.T) { + // B commit + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "d-1", "main").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "README"), []byte("bbb"), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "README").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Changes to the README").Run(&RunOpts{Dir: tmpDir})) + // E commit + require.NoError(t, NewCommand(t.Context(), "merge", "--no-ff", "main-D").Run(&RunOpts{Dir: tmpDir})) + + // C commit + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "d-2", "main-D").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "FUNFACT"), []byte("bbb"), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "FUNFACT").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Changes to the funfact").Run(&RunOpts{Dir: tmpDir})) + + changed, err := gitRepo.CheckIfDiffDiffers("main-D", "d-1", "d-2", nil) + require.NoError(t, err) + assert.True(t, changed) + }) + + t.Run("Same diff", func(t *testing.T) { + // B commit + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "d-3", "main").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "README"), []byte("bbb"), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "README").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Changes to the README").Run(&RunOpts{Dir: tmpDir})) + // Merges D. + require.NoError(t, NewCommand(t.Context(), "merge", "--no-ff", "main-D").Run(&RunOpts{Dir: tmpDir})) + + // C commit + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "d-4", "main-D").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "README"), []byte("bbb"), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "README").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Changes to the README").Run(&RunOpts{Dir: tmpDir})) + + changed, err := gitRepo.CheckIfDiffDiffers("main-D", "d-3", "d-4", nil) + require.NoError(t, err) + assert.False(t, changed) + }) + }) + + t.Run("Non-typical rebase", func(t *testing.T) { + // B + // / + // A--D + // \ + // C + // Where D is the base reference. + // B was rebased onto D, which produced C. + // B and D made the same change to same file. + + // D commit. + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "main-D-2", "main").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "FUNFACT"), []byte("Smithy was the runner up to be Forgejo's name"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "README"), []byte("ccc"), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "FUNFACT", "README").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Forgejo history").Run(&RunOpts{Dir: tmpDir})) + + // B commit + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "e-1", "main").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "CONTACT"), []byte("@example.com"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "README"), []byte("ccc"), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "README", "CONTACT").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Changes to the contact and README").Run(&RunOpts{Dir: tmpDir})) + + // C commit + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "e-2").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "rebase", "main-D-2").Run(&RunOpts{Dir: tmpDir})) + + // The diff changed, because it no longers shows the change made to `README`. + changed, err := gitRepo.CheckIfDiffDiffers("main-D-2", "e-1", "e-2", nil) + require.NoError(t, err) + assert.False(t, changed) // This should be true. + }) + + t.Run("Directory", func(t *testing.T) { + // B + // / + // A + // \ + // C + t.Run("Same directory", func(t *testing.T) { + t.Run("Different diff", func(t *testing.T) { + // B commit + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "f-1", "main").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "docs"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "docs", "README"), []byte("bbb"), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "docs/README").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Changed to the README").Run(&RunOpts{Dir: tmpDir})) + + // C commit + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "f-2", "main").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "docs"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "docs", "README"), []byte("ccc"), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "docs/README").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Changed to the README").Run(&RunOpts{Dir: tmpDir})) + + changed, err := gitRepo.CheckIfDiffDiffers("main", "f-1", "f-2", nil) + require.NoError(t, err) + assert.True(t, changed) + }) + + t.Run("Same diff", func(t *testing.T) { + // B commit + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "f-3", "main").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "docs"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "docs", "README"), []byte("bbb"), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "docs/README").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Changed to the README").Run(&RunOpts{Dir: tmpDir})) + + // C commit + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "f-4", "main").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "docs"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "docs", "README"), []byte("bbb"), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "docs/README").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Changes to the README").Run(&RunOpts{Dir: tmpDir})) + + changed, err := gitRepo.CheckIfDiffDiffers("main", "f-3", "f-4", nil) + require.NoError(t, err) + assert.False(t, changed) + }) + }) + + t.Run("Different directory", func(t *testing.T) { + // B commit + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "f-5", "main").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "docs"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "docs", "README"), []byte("bbb"), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "docs/README").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Changes to the README").Run(&RunOpts{Dir: tmpDir})) + + // C commit + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "f-6", "main").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "documentation"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "documentation", "README"), []byte("bbb"), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "documentation/README").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Changes to the README").Run(&RunOpts{Dir: tmpDir})) + + changed, err := gitRepo.CheckIfDiffDiffers("main", "f-5", "f-6", nil) + require.NoError(t, err) + assert.True(t, changed) + }) + + t.Run("Directory and file", func(t *testing.T) { + // B commit + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "f-7", "main").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "docs"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "docs", "README"), []byte("bbb"), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "docs/README").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Changes to the README").Run(&RunOpts{Dir: tmpDir})) + + // C commit + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "f-8", "main").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "README"), []byte("bbb"), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "README").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Changes to the README").Run(&RunOpts{Dir: tmpDir})) + + changed, err := gitRepo.CheckIfDiffDiffers("main", "f-7", "f-8", nil) + require.NoError(t, err) + assert.True(t, changed) + }) + }) + + t.Run("Rebase", func(t *testing.T) { + // B + // / + // A--D + // \ + // C + // Where D is the base reference. + // B was rebased onto D, which produced C. + // B and D made different (non conflicting) changes to same file. + + // A commit + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "main-3", "main").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "REBASE"), bytes.Repeat([]byte{'b', 'b', 'b', '\n'}, 100), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "REBASE").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Rebasing is fun").Run(&RunOpts{Dir: tmpDir})) + + // B commit + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "g-1", "main-3").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "REBASE"), append(bytes.Repeat([]byte{'b', 'b', 'b', '\n'}, 100), 'a', 'a', 'a'), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "REBASE").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Rebasing is fun").Run(&RunOpts{Dir: tmpDir})) + + // D commit + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "g-2", "main-3").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "REBASE"), append([]byte{'a', 'a', 'a'}, bytes.Repeat([]byte{'b', 'b', 'b', '\n'}, 99)...), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "REBASE").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Rebasing is fun").Run(&RunOpts{Dir: tmpDir})) + + // C commit + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "g-3", "g-1").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "rebase", "g-2").Run(&RunOpts{Dir: tmpDir})) + + changed, err := gitRepo.CheckIfDiffDiffers("g-2", "g-1", "g-3", nil) + require.NoError(t, err) + assert.True(t, changed) + }) + + t.Run("Rebasing change not shown", func(t *testing.T) { + // B + // / + // A--D + // \ + // C + // Where D is the base reference. + // B was rebased onto D, which produced C. + // B and D made different (non conflicting) changes to same file. + + // A commit + require.NoError(t, NewCommand(t.Context(), "switch", "--orphan", "main-4").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "A"), 0o700)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "A", "a"), bytes.Repeat([]byte{'A', 'A', 'A', '\n'}, 100), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "A/a").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Just wondering").Run(&RunOpts{Dir: tmpDir})) + + // B commit + // Changes last line. + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "h-1").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "A", "a"), append(bytes.Repeat([]byte{'A', 'A', 'A', '\n'}, 99), 'B', 'B', 'B', '\n'), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "A/a").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Just wondering").Run(&RunOpts{Dir: tmpDir})) + + // D commit + // Changes first line. + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "h-2", "main-4").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "A", "a"), append([]byte{'B', 'B', 'B', '\n'}, bytes.Repeat([]byte{'A', 'A', 'A', '\n'}, 99)...), 0o600)) + require.NoError(t, NewCommand(t.Context(), "add", "A/a").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "commit", "-m", "Just wondering").Run(&RunOpts{Dir: tmpDir})) + + // C commit + require.NoError(t, NewCommand(t.Context(), "switch", "-c", "h-3").Run(&RunOpts{Dir: tmpDir})) + require.NoError(t, NewCommand(t.Context(), "rebase", "h-2").Run(&RunOpts{Dir: tmpDir})) + + changed, err := gitRepo.CheckIfDiffDiffers("h-2", "h-1", "h-3", nil) + require.NoError(t, err) + + assert.False(t, changed) + }) +} diff --git a/services/pull/patch.go b/services/pull/patch.go index 37a0f818e9..89581c916b 100644 --- a/services/pull/patch.go +++ b/services/pull/patch.go @@ -106,12 +106,15 @@ func (t *testPatchContext) LoadHeadRevision(ctx context.Context, pr *issues_mode } // getTestPatchCtx constructs a new testpatch context for the given pull request. -func getTestPatchCtx(ctx context.Context, pr *issues_model.PullRequest) (*testPatchContext, error) { +// If `onBare` is true, then the context will use the base repository that does +// not contain a working tree. Otherwise a temprorary repository is created that +// contains a working tree. +func getTestPatchCtx(ctx context.Context, pr *issues_model.PullRequest, onBare bool) (*testPatchContext, error) { testPatchCtx := &testPatchContext{ close: func() {}, } - if git.SupportGitMergeTree { + if onBare { if err := pr.LoadBaseRepo(ctx); err != nil { return testPatchCtx, fmt.Errorf("LoadBaseRepo: %w", err) } @@ -157,7 +160,7 @@ func getTestPatchCtx(ctx context.Context, pr *issues_model.PullRequest) (*testPa } func testPatch(ctx context.Context, pr *issues_model.PullRequest) (*testPatchContext, error) { - testPatchCtx, err := getTestPatchCtx(ctx, pr) + testPatchCtx, err := getTestPatchCtx(ctx, pr, git.SupportGitMergeTree) if err != nil { return testPatchCtx, fmt.Errorf("getTestPatchCtx: %w", err) } diff --git a/services/pull/pull.go b/services/pull/pull.go index 26210f7156..c1fe6c6b55 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -4,11 +4,9 @@ package pull import ( - "bytes" "context" "fmt" "io" - "os" "regexp" "strings" "time" @@ -30,7 +28,6 @@ import ( repo_module "forgejo.org/modules/repository" "forgejo.org/modules/setting" "forgejo.org/modules/sync" - "forgejo.org/modules/util" gitea_context "forgejo.org/services/context" issue_service "forgejo.org/services/issue" notify_service "forgejo.org/services/notify" @@ -384,98 +381,51 @@ func TestPullRequest(ctx context.Context, doer *user_model.User, repoID, olderTh // Update commit divergence. func ValidatePullRequest(ctx context.Context, pr *issues_model.PullRequest, newCommitID, oldCommitID string, doer *user_model.User) { objectFormat := git.ObjectFormatFromName(pr.BaseRepo.ObjectFormatName) - if newCommitID != "" && newCommitID != objectFormat.EmptyObjectID().String() { - changed, err := checkIfPRContentChanged(ctx, pr, oldCommitID, newCommitID) + if newCommitID == "" || newCommitID == objectFormat.EmptyObjectID().String() { + return + } + + testPatchCtx, err := getTestPatchCtx(ctx, pr, true) + defer testPatchCtx.close() + if err != nil { + log.Error("testPatchCtx: %v", err) + return + } + + changed, err := testPatchCtx.gitRepo.CheckIfDiffDiffers(testPatchCtx.baseRev, oldCommitID, newCommitID, testPatchCtx.env) + if err != nil { + log.Error("CheckIfDiffDiffers: %v", err) + } + if changed { + if err := issues_model.MarkReviewsAsStale(ctx, pr.IssueID); err != nil { + log.Error("MarkReviewsAsStale: %v", err) + } + + pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pr.BaseRepoID, pr.BaseBranch) if err != nil { - log.Error("checkIfPRContentChanged: %v", err) + log.Error("GetFirstMatchProtectedBranchRule: %v", err) } - if changed { - if err := issues_model.MarkReviewsAsStale(ctx, pr.IssueID); err != nil { - log.Error("MarkReviewsAsStale: %v", err) + if pb != nil && pb.DismissStaleApprovals { + if err := DismissApprovalReviews(ctx, doer, pr); err != nil { + log.Error("DismissApprovalReviews: %v", err) } + } + } + if err := issues_model.MarkReviewsAsNotStale(ctx, pr.IssueID, newCommitID); err != nil { + log.Error("MarkReviewsAsNotStale: %v", err) + } - pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pr.BaseRepoID, pr.BaseBranch) - if err != nil { - log.Error("GetFirstMatchProtectedBranchRule: %v", err) - } - if pb != nil && pb.DismissStaleApprovals { - if err := DismissApprovalReviews(ctx, doer, pr); err != nil { - log.Error("DismissApprovalReviews: %v", err) - } - } - } - if err := issues_model.MarkReviewsAsNotStale(ctx, pr.IssueID, newCommitID); err != nil { - log.Error("MarkReviewsAsNotStale: %v", err) - } - divergence, err := GetDiverging(ctx, pr) + divergence, err := git.GetDivergingCommits(ctx, testPatchCtx.gitRepo.Path, testPatchCtx.baseRev, testPatchCtx.headRev, testPatchCtx.env) + if err != nil { + log.Error("GetDivergingCommits: %v", err) + } else { + err = pr.UpdateCommitDivergence(ctx, divergence.Ahead, divergence.Behind) if err != nil { - log.Error("GetDiverging: %v", err) - } else { - err = pr.UpdateCommitDivergence(ctx, divergence.Ahead, divergence.Behind) - if err != nil { - log.Error("UpdateCommitDivergence: %v", err) - } + log.Error("UpdateCommitDivergence: %v", err) } } } -// checkIfPRContentChanged checks if diff to target branch has changed by push -// A commit can be considered to leave the PR untouched if the patch/diff with its merge base is unchanged -func checkIfPRContentChanged(ctx context.Context, pr *issues_model.PullRequest, oldCommitID, newCommitID string) (hasChanged bool, err error) { - prCtx, cancel, err := createTemporaryRepoForPR(ctx, pr) - if err != nil { - log.Error("CreateTemporaryRepoForPR %-v: %v", pr, err) - return false, err - } - defer cancel() - - tmpRepo, err := git.OpenRepository(ctx, prCtx.tmpBasePath) - if err != nil { - return false, fmt.Errorf("OpenRepository: %w", err) - } - defer tmpRepo.Close() - - // Find the merge-base - _, base, err := tmpRepo.GetMergeBase("", "base", "tracking") - if err != nil { - return false, fmt.Errorf("GetMergeBase: %w", err) - } - - cmd := git.NewCommand(ctx, "diff", "--name-only", "-z").AddDynamicArguments(newCommitID, oldCommitID, base) - stdoutReader, stdoutWriter, err := os.Pipe() - if err != nil { - return false, fmt.Errorf("unable to open pipe for to run diff: %w", err) - } - - stderr := new(bytes.Buffer) - if err := cmd.Run(&git.RunOpts{ - Dir: prCtx.tmpBasePath, - Stdout: stdoutWriter, - Stderr: stderr, - PipelineFunc: func(ctx context.Context, cancel context.CancelFunc) error { - _ = stdoutWriter.Close() - defer func() { - _ = stdoutReader.Close() - }() - return util.IsEmptyReader(stdoutReader) - }, - }); err != nil { - if err == util.ErrNotEmpty { - return true, nil - } - err = git.ConcatenateError(err, stderr.String()) - - log.Error("Unable to run diff on %s %s %s in tempRepo for PR[%d]%s/%s...%s/%s: Error: %v", - newCommitID, oldCommitID, base, - pr.ID, pr.BaseRepo.FullName(), pr.BaseBranch, pr.HeadRepo.FullName(), pr.HeadBranch, - err) - - return false, fmt.Errorf("Unable to run git diff --name-only -z %s %s %s: %w", newCommitID, oldCommitID, base, err) - } - - return false, nil -} - // PushToBaseRepo pushes commits from branches of head repository to // corresponding branches of base repository. // FIXME: Only push branches that are actually updates? diff --git a/services/pull/review.go b/services/pull/review.go index 7d232d6d79..b0ab700fa6 100644 --- a/services/pull/review.go +++ b/services/pull/review.go @@ -298,10 +298,16 @@ func SubmitReview(ctx context.Context, doer *user_model.User, gitRepo *git.Repos if headCommitID == commitID { stale = false } else { - stale, err = checkIfPRContentChanged(ctx, pr, commitID, headCommitID) + testPatchCtx, err := getTestPatchCtx(ctx, pr, true) + defer testPatchCtx.close() if err != nil { return nil, nil, err } + + stale, err = testPatchCtx.gitRepo.CheckIfDiffDiffers(testPatchCtx.baseRev, commitID, headCommitID, testPatchCtx.env) + if err != nil { + return nil, nil, fmt.Errorf("CheckIfDiffDiffers: %w", err) + } } } diff --git a/tests/integration/pull_review_test.go b/tests/integration/pull_review_test.go index 4af0a1100e..67acfe6f00 100644 --- a/tests/integration/pull_review_test.go +++ b/tests/integration/pull_review_test.go @@ -784,47 +784,66 @@ func TestPullRequestStaleReview(t *testing.T) { ) defer f() - // Clone it. - dstPath := t.TempDir() - r := fmt.Sprintf("%suser2/%s.git", u.String(), repo.Name) - cloneURL, _ := url.Parse(r) - cloneURL.User = url.UserPassword("user2", userPassword) - require.NoError(t, git.CloneWithArgs(t.Context(), nil, cloneURL.String(), dstPath, git.CloneRepoOptions{})) + clone := func(t *testing.T, clone string) string { + t.Helper() - // Create first commit. - require.NoError(t, os.WriteFile(path.Join(dstPath, "README.md"), []byte("## test content"), 0o600)) - require.NoError(t, git.AddChanges(dstPath, true)) - require.NoError(t, git.CommitChanges(dstPath, git.CommitChangesOptions{ - Committer: &git.Signature{ - Email: "user2@example.com", - Name: "user2", - When: time.Now(), - }, - Author: &git.Signature{ - Email: "user2@example.com", - Name: "user2", - When: time.Now(), - }, - Message: "Add README.", - })) - stdout := &bytes.Buffer{} - require.NoError(t, git.NewCommand(t.Context(), "rev-parse", "HEAD").Run(&git.RunOpts{Dir: dstPath, Stdout: stdout})) - firstCommitID := strings.TrimSpace(stdout.String()) + dstPath := t.TempDir() + cloneURL, _ := url.Parse(clone) + cloneURL.User = url.UserPassword("user2", userPassword) + require.NoError(t, git.CloneWithArgs(t.Context(), nil, cloneURL.String(), dstPath, git.CloneRepoOptions{})) - // Create agit PR. - require.NoError(t, git.NewCommand(t.Context(), "push", "origin", "HEAD:refs/for/main", "-o", "topic=agit-pr").Run(&git.RunOpts{Dir: dstPath})) + return dstPath + } - pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{Index: 1, BaseRepoID: repo.ID}) + firstCommit := func(t *testing.T, dstPath string) string { + t.Helper() - req := NewRequest(t, "GET", "/"+repo.FullName()+"/pulls/1/files/reviews/new_comment") - resp := session.MakeRequest(t, req, http.StatusOK) - doc := NewHTMLParser(t, resp.Body) + require.NoError(t, os.WriteFile(path.Join(dstPath, "README.md"), []byte("## test content"), 0o600)) + require.NoError(t, git.AddChanges(dstPath, true)) + require.NoError(t, git.CommitChanges(dstPath, git.CommitChangesOptions{ + Committer: &git.Signature{ + Email: "user2@example.com", + Name: "user2", + When: time.Now(), + }, + Author: &git.Signature{ + Email: "user2@example.com", + Name: "user2", + When: time.Now(), + }, + Message: "Add README.", + })) + stdout := &bytes.Buffer{} + require.NoError(t, git.NewCommand(t.Context(), "rev-parse", "HEAD").Run(&git.RunOpts{Dir: dstPath, Stdout: stdout})) - t.Run("Mark review as stale", func(t *testing.T) { - defer tests.PrintCurrentTest(t)() + return strings.TrimSpace(stdout.String()) + } - // Create a approved review against against this commit. - req = NewRequestWithValues(t, "POST", "/"+repo.FullName()+"/pulls/1/files/reviews/comments", map[string]string{ + secondCommit := func(t *testing.T, dstPath string) { + require.NoError(t, os.WriteFile(path.Join(dstPath, "README.md"), []byte("## I prefer this heading"), 0o600)) + require.NoError(t, git.AddChanges(dstPath, true)) + require.NoError(t, git.CommitChanges(dstPath, git.CommitChangesOptions{ + Committer: &git.Signature{ + Email: "user2@example.com", + Name: "user2", + When: time.Now(), + }, + Author: &git.Signature{ + Email: "user2@example.com", + Name: "user2", + When: time.Now(), + }, + Message: "Add README.", + })) + } + + firstReview := func(t *testing.T, firstCommitID string, index int64) { + t.Helper() + + resp := session.MakeRequest(t, NewRequest(t, "GET", fmt.Sprintf("/%s/pulls/%d/files/reviews/new_comment", repo.FullName(), index)), http.StatusOK) + doc := NewHTMLParser(t, resp.Body) + + req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/pulls/%d/files/reviews/comments", repo.FullName(), index), map[string]string{ "_csrf": doc.GetCSRF(), "origin": doc.GetInputValueByName("origin"), "latest_commit_id": firstCommitID, @@ -846,77 +865,166 @@ func TestPullRequestStaleReview(t *testing.T) { "type": "comment", }) session.MakeRequest(t, req, http.StatusOK) + } - // Review is not stale. - review := unittest.AssertExistsAndLoadBean(t, &issues_model.Review{IssueID: pr.IssueID}) - assert.False(t, review.Stale) - - // Create second commit - require.NoError(t, os.WriteFile(path.Join(dstPath, "README.md"), []byte("## I prefer this heading"), 0o600)) - require.NoError(t, git.AddChanges(dstPath, true)) - require.NoError(t, git.CommitChanges(dstPath, git.CommitChangesOptions{ - Committer: &git.Signature{ - Email: "user2@example.com", - Name: "user2", - When: time.Now(), - }, - Author: &git.Signature{ - Email: "user2@example.com", - Name: "user2", - When: time.Now(), - }, - Message: "Add README.", - })) - - // Push to agit PR. - require.NoError(t, git.NewCommand(t.Context(), "push", "origin", "HEAD:refs/for/main", "-o", "topic=agit-pr").Run(&git.RunOpts{Dir: dstPath})) - - // Review is stale. - review = unittest.AssertExistsAndLoadBean(t, &issues_model.Review{IssueID: pr.IssueID}) - assert.True(t, review.Stale) - }) - - t.Run("Create stale review", func(t *testing.T) { - defer tests.PrintCurrentTest(t)() - + staleReview := func(t *testing.T, firstCommitID string, index int64) { // Review based on the first commit, which is a stale review because the // PR's head is at the seconnd commit. - req := NewRequestWithValues(t, "POST", "/"+repo.FullName()+"/pulls/1/files/reviews/submit", map[string]string{ - "_csrf": doc.GetCSRF(), + req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/pulls/%d/files/reviews/submit", repo.FullName(), index), map[string]string{ + "_csrf": GetCSRF(t, session, fmt.Sprintf("/%s/pulls/%d/files/reviews/new_comment", repo.FullName(), index)), "commit_id": firstCommitID, "content": "looks good", "type": "approve", }) session.MakeRequest(t, req, http.StatusOK) + } - // There does not exist a review that is not stale, because all reviews - // are based on the first commit and the PR's head is at the second commit. - unittest.AssertExistsIf(t, false, &issues_model.Review{IssueID: pr.IssueID}, "stale = false") + t.Run("Across repositories", func(t *testing.T) { + testRepoFork(t, session, "user2", repo.Name, "org3", "forked-repo") + + // Clone it. + dstPath := clone(t, fmt.Sprintf("%sorg3/forked-repo.git", u.String())) + + // Create first commit. + firstCommitID := firstCommit(t, dstPath) + + // Create PR across repositories. + require.NoError(t, git.NewCommand(t.Context(), "push", "origin", "main").Run(&git.RunOpts{Dir: dstPath})) + session.MakeRequest(t, NewRequestWithValues(t, "POST", repo.FullName()+"/compare/main...org3/forked-repo:main", map[string]string{ + "_csrf": GetCSRF(t, session, repo.FullName()+"/compare/main...org3/forked-repo:main"), + "title": "pull request", + }), http.StatusOK) + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{Index: 1, BaseRepoID: repo.ID}) + + t.Run("Mark review as stale", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + // Create first review + firstReview(t, firstCommitID, pr.Index) + + // Review is not stale. + review := unittest.AssertExistsAndLoadBean(t, &issues_model.Review{IssueID: pr.IssueID}) + assert.False(t, review.Stale) + + // Create second commit + secondCommit(t, dstPath) + + // Push to PR. + require.NoError(t, git.NewCommand(t.Context(), "push", "origin", "main").Run(&git.RunOpts{Dir: dstPath})) + + // Review is stale. + assert.Eventually(t, func() bool { + return unittest.AssertExistsAndLoadBean(t, &issues_model.Review{IssueID: pr.IssueID}).Stale == true + }, time.Second*10, time.Microsecond*100) + }) + + t.Run("Create stale review", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + // Review based on the first commit, which is a stale review because the + // PR's head is at the seconnd commit. + staleReview(t, firstCommitID, pr.Index) + + // There does not exist a review that is not stale, because all reviews + // are based on the first commit and the PR's head is at the second commit. + unittest.AssertExistsIf(t, false, &issues_model.Review{IssueID: pr.IssueID}, "stale = false") + }) + + t.Run("Mark unstale", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + // Force push the PR to the first commit. + require.NoError(t, git.NewCommand(t.Context(), "reset", "--hard", "HEAD~1").Run(&git.RunOpts{Dir: dstPath})) + require.NoError(t, git.NewCommand(t.Context(), "push", "--force-with-lease", "origin", "main").Run(&git.RunOpts{Dir: dstPath})) + + // There does not exist a review that is stale, because all reviews + // are based on the first commit and thus all reviews are no longer marked + // as stale. + assert.Eventually(t, func() bool { + return !unittest.BeanExists(t, &issues_model.Review{IssueID: pr.IssueID}, "stale = true") + }, time.Second*10, time.Microsecond*100) + }) + + t.Run("Diff did not change", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + // Create a empty commit and push it to the PR. + require.NoError(t, git.NewCommand(t.Context(), "commit", "--allow-empty", "-m", "Empty commit").Run(&git.RunOpts{Dir: dstPath})) + require.NoError(t, git.NewCommand(t.Context(), "push", "origin", "main").Run(&git.RunOpts{Dir: dstPath})) + + // There does not exist a review that is stale, because the diff did not + // change. + unittest.AssertExistsIf(t, false, &issues_model.Review{IssueID: pr.IssueID}, "stale = true") + }) }) - t.Run("Mark unstale", func(t *testing.T) { - defer tests.PrintCurrentTest(t)() + t.Run("AGit", func(t *testing.T) { + dstPath := clone(t, fmt.Sprintf("%suser2/%s.git", u.String(), repo.Name)) - // Force push the PR to the first commit. - require.NoError(t, git.NewCommand(t.Context(), "reset", "--hard", "HEAD~1").Run(&git.RunOpts{Dir: dstPath})) - require.NoError(t, git.NewCommand(t.Context(), "push", "origin", "HEAD:refs/for/main", "-o", "topic=agit-pr", "-o", "force-push").Run(&git.RunOpts{Dir: dstPath})) + // Create first commit. + firstCommitID := firstCommit(t, dstPath) - // There does not exist a review that is stale, because all reviews - // are based on the first commit and thus all reviews are no longer marked - // as stale. - unittest.AssertExistsIf(t, false, &issues_model.Review{IssueID: pr.IssueID}, "stale = true") - }) - - t.Run("Diff did not change", func(t *testing.T) { - defer tests.PrintCurrentTest(t)() - - // Create a empty commit and push it to the PR. - require.NoError(t, git.NewCommand(t.Context(), "commit", "--allow-empty", "-m", "Empty commit").Run(&git.RunOpts{Dir: dstPath})) + // Create agit PR. require.NoError(t, git.NewCommand(t.Context(), "push", "origin", "HEAD:refs/for/main", "-o", "topic=agit-pr").Run(&git.RunOpts{Dir: dstPath})) - // There does not exist a review that is stale, because the diff did not - // change. - unittest.AssertExistsIf(t, false, &issues_model.Review{IssueID: pr.IssueID}, "stale = true") + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{Index: 2, BaseRepoID: repo.ID}) + + t.Run("Mark review as stale", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + firstReview(t, firstCommitID, pr.Index) + + // Review is not stale. + review := unittest.AssertExistsAndLoadBean(t, &issues_model.Review{IssueID: pr.IssueID}) + assert.False(t, review.Stale) + + // Create second commit + secondCommit(t, dstPath) + + // Push to agit PR. + require.NoError(t, git.NewCommand(t.Context(), "push", "origin", "HEAD:refs/for/main", "-o", "topic=agit-pr").Run(&git.RunOpts{Dir: dstPath})) + + // Review is stale. + review = unittest.AssertExistsAndLoadBean(t, &issues_model.Review{IssueID: pr.IssueID}) + assert.True(t, review.Stale) + }) + + t.Run("Create stale review", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + // Review based on the first commit, which is a stale review because the + // PR's head is at the seconnd commit. + staleReview(t, firstCommitID, pr.Index) + + // There does not exist a review that is not stale, because all reviews + // are based on the first commit and the PR's head is at the second commit. + unittest.AssertExistsIf(t, false, &issues_model.Review{IssueID: pr.IssueID}, "stale = false") + }) + + t.Run("Mark unstale", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + // Force push the PR to the first commit. + require.NoError(t, git.NewCommand(t.Context(), "reset", "--hard", "HEAD~1").Run(&git.RunOpts{Dir: dstPath})) + require.NoError(t, git.NewCommand(t.Context(), "push", "origin", "HEAD:refs/for/main", "-o", "topic=agit-pr", "-o", "force-push").Run(&git.RunOpts{Dir: dstPath})) + + // There does not exist a review that is stale, because all reviews + // are based on the first commit and thus all reviews are no longer marked + // as stale. + unittest.AssertExistsIf(t, false, &issues_model.Review{IssueID: pr.IssueID}, "stale = true") + }) + + t.Run("Diff did not change", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + // Create a empty commit and push it to the PR. + require.NoError(t, git.NewCommand(t.Context(), "commit", "--allow-empty", "-m", "Empty commit").Run(&git.RunOpts{Dir: dstPath})) + require.NoError(t, git.NewCommand(t.Context(), "push", "origin", "HEAD:refs/for/main", "-o", "topic=agit-pr").Run(&git.RunOpts{Dir: dstPath})) + + // There does not exist a review that is stale, because the diff did not + // change. + unittest.AssertExistsIf(t, false, &issues_model.Review{IssueID: pr.IssueID}, "stale = true") + }) }) }) }