| Message ID | bb2317972a8faf0358aaad1247fdfd2af2ea142d.1602545164.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] test-lib: allow selecting tests by substring/regex with --run | expand |
On Mon, Oct 12, 2020 at 11:26:04PM +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. > > Signed-off-by: Elijah Newren <newren@gmail.com> > --- > t/test-lib.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/t/test-lib.sh b/t/test-lib.sh > index 2aca398e1e..7602bbe9e9 100644 > --- a/t/test-lib.sh > +++ b/t/test-lib.sh > @@ -1059,7 +1059,7 @@ test_skip () { > " <skipped message=\"$message\" />" > fi > > - say_color skip >&3 "skipping test: $@" > + say_color skip >&3 "skipping test: $1" I would be comfortable going further than this and removing this line entirely. We need the "ok $test_count # skip" below since it's part of our TAP output, but now the output looks somewhat redundant. With this patch running a test that I'm working on with `--run=...`, I get output that looks like: skipping test: writing bitmaps via command-line can duplicate .keep objects ok 2 # skip writing bitmaps via command-line can duplicate .keep objects (--run) skipping test: writing bitmaps via config can duplicate .keep objects ok 3 # skip writing bitmaps via config can duplicate .keep objects (--run) Scanning over the same test description twice per skipped test makes the output difficult (but still much easier than before) to scan. What do you think about either of the following: skipping test: writing bitmaps via command-line can duplicate .keep objects ok 2 # skip (--run) or: ok 2 # skip writing bitmaps via command-line can duplicate .keep objects (--run) I have a slight preference towards the latter, since it keeps more of the information in the TAP line, and it cuts the total line count of output from skipped tests in half. For what it's worth, I'd also be fine with the patch as-is. Thanks, Taylor
On Tue, Oct 13, 2020 at 8:44 AM Taylor Blau <me@ttaylorr.com> wrote: > > On Mon, Oct 12, 2020 at 11:26:04PM +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. > > > > Signed-off-by: Elijah Newren <newren@gmail.com> > > --- > > t/test-lib.sh | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/t/test-lib.sh b/t/test-lib.sh > > index 2aca398e1e..7602bbe9e9 100644 > > --- a/t/test-lib.sh > > +++ b/t/test-lib.sh > > @@ -1059,7 +1059,7 @@ test_skip () { > > " <skipped message=\"$message\" />" > > fi > > > > - say_color skip >&3 "skipping test: $@" > > + say_color skip >&3 "skipping test: $1" > > I would be comfortable going further than this and removing this line > entirely. We need the "ok $test_count # skip" below since it's part of > our TAP output, but now the output looks somewhat redundant. > > With this patch running a test that I'm working on with `--run=...`, I > get output that looks like: > > skipping test: writing bitmaps via command-line can duplicate .keep objects > ok 2 # skip writing bitmaps via command-line can duplicate .keep objects (--run) > > skipping test: writing bitmaps via config can duplicate .keep objects > ok 3 # skip writing bitmaps via config can duplicate .keep objects (--run) > > Scanning over the same test description twice per skipped test makes the > output difficult (but still much easier than before) to scan. What do > you think about either of the following: > > skipping test: writing bitmaps via command-line can duplicate .keep objects > ok 2 # skip (--run) > > or: > > ok 2 # skip writing bitmaps via command-line can duplicate .keep objects (--run) > > I have a slight preference towards the latter, since it keeps more of > the information in the TAP line, and it cuts the total line count of > output from skipped tests in half. > > For what it's worth, I'd also be fine with the patch as-is. I was worried that shortening it as much as I did would run into objections for some obscure reason. But if no one objects, I think I'd also prefer your latter suggestion for shortening it even more. It looks like the dual output for skipped tests comes from commit 04ece59399 ("GIT_SKIP_TESTS: allow users to omit tests that are known to break", 2006-12-28) by Junio, so it'd be nice to hear his opinion on how much we shorten it.
Elijah Newren <newren@gmail.com> writes: >> skipping test: writing bitmaps via command-line can duplicate .keep objects >> ok 2 # skip (--run) >> >> or: >> >> ok 2 # skip writing bitmaps via command-line can duplicate .keep objects (--run) >> >> I have a slight preference towards the latter, since it keeps more of >> the information in the TAP line, and it cuts the total line count of >> output from skipped tests in half. >> >> For what it's worth, I'd also be fine with the patch as-is. > > I was worried that shortening it as much as I did would run into > objections for some obscure reason. But if no one objects, I think > I'd also prefer your latter suggestion for shortening it even more. > It looks like the dual output for skipped tests comes from commit > 04ece59399 ("GIT_SKIP_TESTS: allow users to omit tests that are known > to break", 2006-12-28) by Junio, so it'd be nice to hear his opinion > on how much we shorten it. I am afraid that the surrounding code has changed so heavily that expertise as the author of 04ece593 would not count that much in the context of today's code. Back in the late 2006 version of the code, text_expect_success asked test_skip to see if a test should be skipped, and when the latter says yes (which also gave "skipping test" to the verbose output and "skip $test_count: $1" to normal output), test_expect_success did not even call test_ok_ or test_failure_. So "ok 2 # skip ..." we see in the quoted part of your exchange above is probably a much more recent invention. As long as we can convince ourselves that writing similar things to fd #1 (for tap in "ok $count # skip ($why)" format) and fd #3 is redundant and not useful, I think removing the utterance to fd #3 in this codepath may be a reasonable thing to do. ... goes and scans for ">&3" in test-lib.sh and ... test-lib-functions.sh OK, I think we can safely lose the one we send for the "verbose" case here and that would turn our use of fd #3 more consistent with how it is used by other parts of the test framework. Thanks.
diff --git a/t/test-lib.sh b/t/test-lib.sh index 2aca398e1e..7602bbe9e9 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -1059,7 +1059,7 @@ test_skip () { " <skipped message=\"$message\" />" fi - say_color skip >&3 "skipping test: $@" + say_color skip >&3 "skipping test: $1" say_color skip "ok $test_count # skip $1 ($skipped_reason)" : true ;;