Jeff King <peff@xxxxxxxx> writes: > I don't mind the general direction, but... > >> +# Additional tests from places in contrib/ that are prepared to take >> +# "make -C $there test", but expects that the primary build is done >> +# already. >> +test-extra: all >> + $(MAKE) -C contrib/diff-highlight test >> + $(MAKE) -C contrib/mw-to-git test >> + $(MAKE) -C contrib/subtree test > > I'm not sure of the quality of tests in some of the contrib stuff. The > tests in diff-highlight worked for me when I added them, but it's not > like I ever run them regularly, or that they've been tested on a wide > variety of platforms. > > So I think this is as likely to cause somebody a headache due to a dumb > portability problem or random bitrot as it is to actually find a bug. I > guess test-extra wouldn't be run by default, but only via CI, so maybe > that limits the blast radius sufficiently. Yeah, that is the exact thought I had when I did it. Anybody who is not aware of test target other than 'test' will not be hurt, and we explicitly make the CI aware of 'test-all' to trigger it. But as long as somebody bothered to write the tests, exercising them to reveal bitrot-bugs either in the tested contrib stuff or the tests themselves to be fixed or removed would be a good thing to do. An updated version of the posted patch is in 'seen' that also covers credential/netrc; https://github.com/git/git/runs/4465323829 shows the logs from its jobs. It is not particularly interesting that most of the jobs are marked as failed, as t1092 was broken the same way in my local test. What I found interesting from my quick scan of randomly chosen jobs are that (1) nobody seemed to have failed test-extra, and (2) nobody had mediawiki installed to test mw-to-git. So I am tempted to do test-extra: all $(MAKE) -C contrib/credential/netrc test $(MAKE) -C contrib/diff-highlight test : $(MAKE) -C contrib/mw-to-git test $(MAKE) -C contrib/subtree test in the topic itself, while adding $(MAKE) -C contrib/scalar test before the subtree test (alphabetically) when it is merged to 'seen' with the js/scalar topic.
