On Thu, Apr 18, 2019 at 03:49:53PM -0400, Jeff King wrote:
> > This block after "if (bitmap_git)" is not exercised by the (non-performance)
> > test suite, so the rest of the code above is not tested, either. Could a test
> > of this "prune" capability be added to the regression tests around the bitmaps?
>
> I have somewhat mixed feelings here. We can add a test with a trivial
> bitmap to get code coverage here. But as with many optimizations
> (bitmaps, but also your commit graph work), we get a much more thorough
> correctness test by re-running the whole test suite (though we do not
> yet have one for running with bitmaps on), and a better test that the
> optimization is kicking in and working via the t/perf tests.
>
> I dunno. I guess it does not hurt to at least to at least make sure this
> code is running in the normal suite. I don't think that will find the
> more interesting regressions, but at least saves us the from the most
> egregious ones ("oops, the whole thing segfaults because somebody
> changed the API" kinds of things).
So here's a test. This goes on top of jk/prune-optim (which is also
already in master).
-- >8 --
Subject: [PATCH] t5304: add a test for pruning with bitmaps
Commit fde67d6896 (prune: use bitmaps for reachability traversal,
2019-02-13) uses bitmaps for pruning when they're available, but only
covers this functionality in the t/perf tests. This makes a kind of
sense, since the point is that the behaviour is indistinguishable before
and after the patch, just faster.
But since the bitmap code path is not exercised at all in the regular
test suite, it leaves us open to a regression where the behavior does in
fact change. The most thorough way to test that would be running the
whole suite with bitmaps enabled. But we don't yet have a way to do
that, and anyway it's expensive to do so. Let's at least add a basic
test that exercises this path and make sure we prune an object we should
(and not one that we shouldn't).
That would hopefully catch the most obvious breakages early.
Signed-off-by: Jeff King <peff@xxxxxxxx>
---
t/t5304-prune.sh | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh
index 1eeb828a15..df60f18fb8 100755
--- a/t/t5304-prune.sh
+++ b/t/t5304-prune.sh
@@ -341,4 +341,12 @@ test_expect_success 'prune: handle expire option correctly' '
git prune --no-expire
'
+test_expect_success 'trivial prune with bitmaps enabled' '
+ git repack -adb &&
+ blob=$(echo bitmap-unreachable-blob | git hash-object -w --stdin) &&
+ git prune --expire=now &&
+ git cat-file -e HEAD &&
+ test_must_fail git cat-file -e $blob
+'
+
test_done
--
2.21.0.1090.g9d17dac192
