Ævar Arnfjörð Bjarmason wrote: > > On Mon, Jun 14 2021, Felipe Contreras wrote: > > > Jeff King wrote: > >> or even making things worse (as this patch did). > > > > I think breaking the test suite is objectively worse than having a few > > extra files in the output directory, but to each his own. > > We've got both in-tree and out-tree things that rely on e.g. the > *.counts in that directory to have a 1=1 mapping with "real" > tests. E.g. "make aggregate-results". Yeah, that's not good, but I wouldn't call it "worse". > >> I don't know if anybody still uses it these days, though. I suspect it's > >> outlived its usefulness, in that we would typically not have any > >> valgrind errors at all (so coalescing them is not that interesting). > >> > >> Possibly folks investigating leak-checking via valgrind could find it > >> useful, but even there I think LSan is a much better path forward. > > > > Yeah, but even if they do run this tool, they can set > > TEST_OUTPUT_DIRECTORY manually. > > > > The needs of the few should not otweight needs of the many. > > Do we need to bring Spock into this?:) Hopefully not. > I think the following alternate/on-top patch (which you should feel free > to squash in with my SOB if you agree) solves the issue both of you are > noting. > > It will barf if you run the tests under e.g. --tee with that environment > variable, but that's intentional. It's better to loudly error if we > don't have the expected test-results than to silently write in the wrong > place. I'm not sure how that patch was supposed to work: 'err' is not empty, it contains: mkdir: cannot create directory '/dev/null': File exists /home/felipec/dev/git.git/git/t/test-lib.sh: line 1133: /dev/null.counts: Permission denied not ok 5 - pretend we have a fully passing test suite Does this assume the rest of the framework will be updated to properly handle TEST_NO_RESULTS_OUTPUT? Cheers. -- Felipe Contreras
