| Message ID | 20230304212220.qkzc2joco5xj7d4s@lucy.dinwoodie.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | test-lib: allow storing counts with test harnesses | expand |
On Sat, Mar 04, 2023 at 09:22:20PM +0000, Adam Dinwoodie wrote: > Currently, test result files are only stored in test-results/*.counts if > $HARNESS_ACTIVE is not set. This dates from 8ef1abe550 (test-lib: Don't > write test-results when HARNESS_ACTIVE, 2010-08-11), where the > assumption was that if someone were using a test harness like prove, > that would track results and the count files wouldn't be required. > However, as of 49da404070 (test-lib: show missing prereq summary, > 2021-11-20), those files also store the list of git test prerequisites > that were missing during the test run, which isn't something that a > generic test harness like prove can provide. > > To allow folk using test harnesses to access the lists of missing > prerequisites, add a --counts argument to test-lib that will keep these > counts files even if a test harness is in use. This means that a > subsequent call of, say, `make -C t aggregate-results` will report > useful information. Your goal seems reasonable. I have to wonder if it is even worth requiring "--counts" here, though. Even 8ef1abe550 claims that the I/O from writing the results files is minimal. And certainly I run under prove with "--root=/some/ram/disk", and I haven't noticed any difference with and without my usual "--verbose-log", which writes a lot more data into test-results/. So would it be worth it to just revert 8ef1abe550, and always store the meta-files? That's one less option to support, and one less surprise when some other feature is built around them. Or is there some reason that we really want to have a mode where nothing is written into t/? From reading 8ef1abe550 it sounded like this was mostly a hygiene / optimization thing, and not some special mode we cared about supporting. > t/test-lib.sh | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) The patch itself looks correct, if we want to go with a --counts option. -Peff
Jeff King <peff@peff.net> writes: > So would it be worth it to just revert 8ef1abe550, and always store the > meta-files? That's one less option to support, and one less surprise > when some other feature is built around them. > > Or is there some reason that we really want to have a mode where nothing > is written into t/? From reading 8ef1abe550 it sounded like this was > mostly a hygiene / optimization thing, and not some special mode we > cared about supporting. Interesting thought. It would simplify our lives to have fewer conditionally-moving parts. >> t/test-lib.sh | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) > > The patch itself looks correct, if we want to go with a --counts option. Yes.
On Sat, Mar 4, 2023 at 2:16 PM Adam Dinwoodie <adam@dinwoodie.org> wrote: > > Currently, test result files are only stored in test-results/*.counts if > $HARNESS_ACTIVE is not set. This dates from 8ef1abe550 (test-lib: Don't > write test-results when HARNESS_ACTIVE, 2010-08-11), where the > assumption was that if someone were using a test harness like prove, > that would track results and the count files wouldn't be required. > However, as of 49da404070 (test-lib: show missing prereq summary, > 2021-11-20), those files also store the list of git test prerequisites > that were missing during the test run, which isn't something that a > generic test harness like prove can provide. > > To allow folk using test harnesses to access the lists of missing > prerequisites, add a --counts argument to test-lib that will keep these > counts files even if a test harness is in use. This means that a > subsequent call of, say, `make -C t aggregate-results` will report > useful information. > > It might be preferable to do make a wider-ranging change, including Replace "do make" with either "do" or "make"? > storing the missing prerequisites separately from the count files, so > the results can be reported regardless of whether the success/failure > counts are wanted, but that would be more disruptive and more work for > relatively little gain. > > Signed-off-by: Adam Dinwoodie <adam@dinwoodie.org> > --- > > I submitted this as an RFC back in December, and received no comments, > so I'm submitting this as an actual patch now. My key concern was the > final paragraph above -- embedding using the "count" files for something > other than counts -- but I've mostly convinced myself that refactoring > this code to separate that out is unlikely to actually cause significant > pain. Actual code change looks fine to me as well, though I see Peff has pointed out we might not even need to make it an option.
diff --git a/t/test-lib.sh b/t/test-lib.sh index 6db377f68b..bbd9ee0e34 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -157,6 +157,8 @@ parse_option () { local opt="$1" case "$opt" in + -c|--c|--co|--cou|--coun|--count|--counts) + record_counts=t ;; -d|--d|--de|--deb|--debu|--debug) debug=t ;; -i|--i|--im|--imm|--imme|--immed|--immedi|--immedia|--immediat|--immediate) @@ -1282,7 +1284,7 @@ test_done () { finalize_test_output - if test -z "$HARNESS_ACTIVE" + if test -z "$HARNESS_ACTIVE" || test -n "$record_counts" then mkdir -p "$TEST_RESULTS_DIR"
Currently, test result files are only stored in test-results/*.counts if $HARNESS_ACTIVE is not set. This dates from 8ef1abe550 (test-lib: Don't write test-results when HARNESS_ACTIVE, 2010-08-11), where the assumption was that if someone were using a test harness like prove, that would track results and the count files wouldn't be required. However, as of 49da404070 (test-lib: show missing prereq summary, 2021-11-20), those files also store the list of git test prerequisites that were missing during the test run, which isn't something that a generic test harness like prove can provide. To allow folk using test harnesses to access the lists of missing prerequisites, add a --counts argument to test-lib that will keep these counts files even if a test harness is in use. This means that a subsequent call of, say, `make -C t aggregate-results` will report useful information. It might be preferable to do make a wider-ranging change, including storing the missing prerequisites separately from the count files, so the results can be reported regardless of whether the success/failure counts are wanted, but that would be more disruptive and more work for relatively little gain. Signed-off-by: Adam Dinwoodie <adam@dinwoodie.org> --- I submitted this as an RFC back in December, and received no comments, so I'm submitting this as an actual patch now. My key concern was the final paragraph above -- embedding using the "count" files for something other than counts -- but I've mostly convinced myself that refactoring this code to separate that out is unlikely to actually cause significant pain. t/test-lib.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)