On Tue, Jan 16, 2024 at 03:37:24PM -0500, Taylor Blau wrote: > Hi Gábor, > > On Sun, Jan 14, 2024 at 12:41:34AM +0100, SZEDER Gábor wrote: > > > In any case, here's the patch on top (with a lightly modified version of > > > the test you wrote in <20230830200218.GA5147@xxxxxxxxxx>: > > > > I certainly hope that I'm just misunderstanding, and you don't > > actually imply that this one test in its current form would qualify as > > thorough testing... :) > > I hear what you're saying, though I think that the interesting behavior > that would be most likely to regress is the transition between different > Bloom filter settings/hash-version across split commit-graph layers. > > We have extensive tests on either "side" of this transition for both v1 > and v2 Bloom filters, so I'm not sure what we'd want to add there. Like > I said, the transition is the primary (previously-)untested area of this > code that I would want to ensure is covered to protect against > regressions. > > I think that the most recent round of this series accomplishes that > goal. It's great that we finally have test cases for different Bloom filter settings in different commit-graph layers, including a test case that merges those layers, but that test case doesn't check that the resulting merged commit-graph file contains the right settings. And there is still no test case that merges layers with different Bloom filter versions. I think adding these would be the bare minimum... and would need more for due diligence.
