Hi Patrick, On Wed, 12 Jun 2024, Patrick Steinhardt wrote: > On Tue, Jun 11, 2024 at 03:09:15PM +0000, Johannes Schindelin via GitGitGadget wrote: > > From: Derrick Stolee <derrickstolee@xxxxxxxxxx> > > > > This fixes a bug that was introduced by 368d19b0b7 (commit-graph: > > refactor compute_topological_levels(), 2023-03-20): Previously, the > > progress indicator was updated from `i + 1` where `i` is the loop > > variable of the enclosing `for` loop. After this patch, the update used > > `info->progress_cnt + 1` instead, however, unlike `i`, the > > `progress_cnt` attribute was not incremented. Let's increment it. > > > > [...] > > The fix looks obviously correct. Do we also want to amend tests? We have > e.g. "t6500-gc.sh", "gc --no-quiet", where we already grep for the > progress report without verifying numbers. The output there is: > > Computing commit graph topological levels: 25% (1/4), done. > Computing commit graph generation numbers: 25% (1/4), done. > > , which clearly demonstrates the bug for both callsites of the buggy > function. > > The following change would thus detect such regressions in the future: > > diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh > index 43d40175f8..1b5909d1b7 100755 > --- a/t/t6500-gc.sh > +++ b/t/t6500-gc.sh > @@ -158,7 +158,7 @@ test_expect_success TTY 'with TTY: gc --no-quiet' ' > git -c gc.writeCommitGraph=true gc --no-quiet >stdout 2>stderr && > test_must_be_empty stdout && > test_grep "Enumerating objects" stderr && > - test_grep "Computing commit graph generation numbers" stderr > + test_grep "Computing commit graph generation numbers: 100% (4/4), done." stderr > ' > > test_expect_success 'gc --quiet' ' Good idea! Thank you, Johannes
