Hi, On 17.10.2020 15:34, Junio C Hamano wrote: > Samuel Čavoj <samuel@xxxxxxxxx> writes: > > >> Now that we know that the root cause of the bug you fixed was > >> because rebase rebase with the default merge strategy for two-head > >> merges use separate codepaths from and all other rebases, I wonder > >> if it is prudent to also test the same cases this series adds > >> without giving "-s resolve". That would exercise the other codepath > > > > I will leave that for someone else to tackle eventually. > > We know that other codepath has been working even before this fix, > but tests are not about showing off what we fixed, but are about > making sure similar breakage won't be introduced by mistake in the > future. Leaving it "for someone", when we know what the problem is > and how to solve it, is asking for the "evantually" not materialize > forever. I agree with that, don't take me wrong, but in general, people have other things to do, than implement test cases only marginally related to the inital patch they submitted. Anyway, as it didn't take long in this case, I added them as patch 3/3 in v4. > > > As the number of very similar test is slowly growing, do you think it is > > worth copying (or making more generic) the test_rebase_gpg_sign for this > > situation as well? We currently have 4 almost identical tests (counting > > the new one you suggested for v4). Just a thought, as it is simpler to > > just add it at this point. Thanks for the feedback. > > That is a tough question. Often, a generic test helper makes it too > easy to do a full matrix of tests and encourages us to overdo it, > which we probably would want to avoid. I think what I've suggested > so far is a bare minimum combination for code coverage. > Alright, I will leave them as is. Regards, Samuel
