Re: [PATCH] test-lib: allow storing counts with test harnesses
- From
Elijah Newren <newren@gmail.com>
- Date
- Apr 1, 2023, 18:56 UTC
- Message-ID
- <CABPp-BGBYUHeYtsyM-gYvr0CsKGAmJ1OKqcmnHiKYy0ps6NrCg@mail.gmail.com>
- In-Reply-To
- <20230304212220.qkzc2joco5xj7d4s@lucy.dinwoodie.org>
On Sat, Mar 4, 2023 at 2:16 PM Adam Dinwoodie <adam@dinwoodie.org> wrote:
Show 18 quoted lines
> > 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"?
Show 14 quoted lines
> 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.