| Message ID | 85a4ca164a9f665016d4aad0f29cbef6f62f36b0.1602616786.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Make test selection easier by specifying description substrings instead of just numeric counters | expand |
On Tue, Oct 13, 2020 at 07:19:46PM +0000, Elijah Newren via GitGitGadget wrote: > From: Elijah Newren <newren@gmail.com> > > When using the --run flag to run just two or three tests from a test > file which contains several dozen tests, having every skipped test print > out dozens of lines of output for the test code for that skipped test > adds up to hundreds or thousands of lines of irrelevant output that make > it very hard to fish out the relevant results you were looking for. > Simplify the output for skipped tests down to just showing the one-line > descriptions. This last sentence is inaccurate in this version, isn't it? Other than that, I think this is a good change (I admit I never noticed the irrelevant output because it is only shown with "-v", and that is already full of irrelevant bits. But I have trouble imagining how it would be useful). Earlier discussion mentioned shortening the "ok 2 # skip" line, but I think removing this fd-3 version is the only sane direction. The other one gets parsed by TAP tools like prove, and may be shown. E.g., "prove --directive t0003-attributes.sh" shows the description of the skipped tests (likewise, "-v" shows all tests but highlights the skipped ones). -Peff
On Wed, Oct 14, 2020 at 9:53 AM Jeff King <peff@peff.net> wrote: > > On Tue, Oct 13, 2020 at 07:19:46PM +0000, Elijah Newren via GitGitGadget wrote: > > > From: Elijah Newren <newren@gmail.com> > > > > When using the --run flag to run just two or three tests from a test > > file which contains several dozen tests, having every skipped test print > > out dozens of lines of output for the test code for that skipped test > > adds up to hundreds or thousands of lines of irrelevant output that make > > it very hard to fish out the relevant results you were looking for. > > Simplify the output for skipped tests down to just showing the one-line > > descriptions. > > This last sentence is inaccurate in this version, isn't it? Maybe I could make it clearer, but I think that it is accurate[1]. If this wording seems confusing, though, I could simplify the commit message by reducing the sentence to "Simplify the output for skipped tests." [1] The original code showed the one-line description (with a "skipping test:" prefix) AND the full test code AND then repeated the one-line description on an "ok" line. My v1 version of this patch ripped out the long middle portion -- the full test code -- and thus only showed the other bits, namely showing the one-line descriptions twice (once with the "skipping test:" prefix, and the second time with an "ok <number> # " prefix). This v2 version of the patch shows the one-line description only once with the "ok <number> #" prefix. So, I'd say the commit message is still accurate: the code only shows the one-line description in both v1 and v2, though there is a question of how many times it is shown. > Other than that, I think this is a good change (I admit I never noticed > the irrelevant output because it is only shown with "-v", and that is > already full of irrelevant bits. But I have trouble imagining how it > would be useful). > > Earlier discussion mentioned shortening the "ok 2 # skip" line, but I > think removing this fd-3 version is the only sane direction. The other > one gets parsed by TAP tools like prove, and may be shown. E.g., "prove > --directive t0003-attributes.sh" shows the description of the skipped > tests (likewise, "-v" shows all tests but highlights the skipped ones). Make sense, thanks.
On Wed, Oct 14, 2020 at 10:39:01AM -0700, Elijah Newren wrote: > On Wed, Oct 14, 2020 at 9:53 AM Jeff King <peff@peff.net> wrote: > > > > On Tue, Oct 13, 2020 at 07:19:46PM +0000, Elijah Newren via GitGitGadget wrote: > > > > > From: Elijah Newren <newren@gmail.com> > > > > > > When using the --run flag to run just two or three tests from a test > > > file which contains several dozen tests, having every skipped test print > > > out dozens of lines of output for the test code for that skipped test > > > adds up to hundreds or thousands of lines of irrelevant output that make > > > it very hard to fish out the relevant results you were looking for. > > > Simplify the output for skipped tests down to just showing the one-line > > > descriptions. > > > > This last sentence is inaccurate in this version, isn't it? > > Maybe I could make it clearer, but I think that it is accurate[1]. If > this wording seems confusing, though, I could simplify the commit > message by reducing the sentence to "Simplify the output for skipped > tests." Yeah, I wondered if you might have been thinking that. It makes sense in the context of the other discussion, but the single-line TAP output is not even mentioned here. And it might be worth doing so, because the real reason it is OK to delete this line entirely is that it is redundant with that line. -Peff
On Wed, Oct 14, 2020 at 10:55 AM Jeff King <peff@peff.net> wrote: > > On Wed, Oct 14, 2020 at 10:39:01AM -0700, Elijah Newren wrote: > > > On Wed, Oct 14, 2020 at 9:53 AM Jeff King <peff@peff.net> wrote: > > > > > > On Tue, Oct 13, 2020 at 07:19:46PM +0000, Elijah Newren via GitGitGadget wrote: > > > > > > > From: Elijah Newren <newren@gmail.com> > > > > > > > > When using the --run flag to run just two or three tests from a test > > > > file which contains several dozen tests, having every skipped test print > > > > out dozens of lines of output for the test code for that skipped test > > > > adds up to hundreds or thousands of lines of irrelevant output that make > > > > it very hard to fish out the relevant results you were looking for. > > > > Simplify the output for skipped tests down to just showing the one-line > > > > descriptions. > > > > > > This last sentence is inaccurate in this version, isn't it? > > > > Maybe I could make it clearer, but I think that it is accurate[1]. If > > this wording seems confusing, though, I could simplify the commit > > message by reducing the sentence to "Simplify the output for skipped > > tests." > > Yeah, I wondered if you might have been thinking that. It makes sense in > the context of the other discussion, but the single-line TAP output is > not even mentioned here. And it might be worth doing so, because the > real reason it is OK to delete this line entirely is that it is > redundant with that line. Makes sense; I'll update it to mention that, once the discussion on how we want to handle regexes/globs/subshells for patch 1/3 is resolved.
diff --git a/t/test-lib.sh b/t/test-lib.sh index 2aca398e1e..fe81d9c733 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -1059,7 +1059,6 @@ test_skip () { " <skipped message=\"$message\" />" fi - say_color skip >&3 "skipping test: $@" say_color skip "ok $test_count # skip $1 ($skipped_reason)" : true ;;