diff mbox series

[v2,3/3] test-lib: reduce verbosity of skipped tests

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

Commit Message

Elijah Newren Oct. 13, 2020, 7:19 p.m. UTC
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.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 t/test-lib.sh | 1 -
 1 file changed, 1 deletion(-)

Comments

Jeff King Oct. 14, 2020, 4:53 p.m. UTC | #1
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
Elijah Newren Oct. 14, 2020, 5:39 p.m. UTC | #2
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.
Jeff King Oct. 14, 2020, 5:55 p.m. UTC | #3
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
Elijah Newren Oct. 14, 2020, 5:57 p.m. UTC | #4
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 mbox series

Patch

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
 		;;