On Mon, Aug 22, 2022 at 12:09:55PM -0400, Derrick Stolee wrote: > On 8/19/2022 5:30 PM, Taylor Blau wrote: > > > +test_expect_success 'preferred pack change with existing MIDX bitmap' ' > > + rm -fr repo && > > Does a previous test not delete 'repo' when necessary? > > Or, do previous tests re-use 'repo' and now we are in a region > where we can safely clear that directory? Should we use a > different name? > > > + git init repo && > > + test_when_finished "rm -fr repo" && > > nit: test_when_finished should be the first line of the test. The "rm-then-init-then-test_when_finished" is an (unfortunate) pattern extended throughout t5326, mostly that some tests don't clean up "repo" after deleting and recreating it. But it's easy enough to just use a separate repository, and avoid removing it altogether. Thanks for the suggestion! > > + # Generate a new MIDX which changes the preferred pack to a pack > > + # contained in the existing MIDX, such that not all objects from > > + # p2 that appear in the MIDX had their copy selected from p2. > > + git multi-pack-index write --bitmap \ > > + --preferred-pack="pack-$p2.pack" && > > + test_path_is_file $midx && > > + test_path_is_file $midx-$(midx_checksum $objdir).bitmap && > > + > > + test_must_fail git clone --no-local . clone2 > > This section is demonstrating the bug. Perhaps we should have > comments indicating that this is not desired behavior, but is > being demonstrated so the bug can be fixed by a later change? Yeah, good call. Thanks again! Thanks, Taylor