| Message ID | c60c78c7-a029-48e8-840a-45dcc785a6e5@web.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | unit-tests: add and use TEST_RUN to simplify tests | expand |
On Sat, Jun 29, 2024 at 05:35:31PM +0200, René Scharfe wrote: > Provide the expected output of "test-tool example-tap" verbatim instead > of as a here-doc, to avoid distractions due to quoting, variables > containing quotes and indentation. I'm not really opposed to this patch, but I wondered... > test_expect_success 'TAP output from unit tests' ' > - cat >expect <<-EOF && > - ok 1 - passing test > - ok 2 - passing test and assertion return 1 If you could take the test input on stdin, like so: test_expect_success 'TAP output from unit tests' - <<-\EOT cat >expect <<-\EOF ok 1 - passing test ok 2 - passing test and assertion return 1 [...] # check "'a' == '\n'" failed at t/helper/test-example-tap.c:64 # left: 'a' # right: '\012' [...] EOF EOT would that be preferable to moving it to its own file? I kind of like keeping everything in the test scripts themselves so related changes can happen side-by-side, though I admit in this case it is intimately tied to the separate test-example-tap.c source anyway. But I do have such an "EOT" patch which I've been meaning to send out, since it makes many of these quoting annoyances go away (though of course it leaves the indentation). -Peff
Jeff King <peff@peff.net> writes: > On Sat, Jun 29, 2024 at 05:35:31PM +0200, René Scharfe wrote: > >> Provide the expected output of "test-tool example-tap" verbatim instead >> of as a here-doc, to avoid distractions due to quoting, variables >> containing quotes and indentation. > > I'm not really opposed to this patch, but I wondered... > >> test_expect_success 'TAP output from unit tests' ' >> - cat >expect <<-EOF && >> - ok 1 - passing test >> - ok 2 - passing test and assertion return 1 > > If you could take the test input on stdin, like so: > > test_expect_success 'TAP output from unit tests' - <<-\EOT > cat >expect <<-\EOF > ok 1 - passing test > ok 2 - passing test and assertion return 1 > [...] > # check "'a' == '\n'" failed at t/helper/test-example-tap.c:64 > # left: 'a' > # right: '\012' > [...] > EOF > EOT > > would that be preferable to moving it to its own file? I kind of like > keeping everything in the test scripts themselves so related changes can > happen side-by-side, though I admit in this case it is intimately tied > to the separate test-example-tap.c source anyway. Yeah, it does feel a bit of cop-out to separate the expectation out to an external file. I guess I was to blame for things like t4013 but there is a valid excuse there (it would be expected that similar tests would need to be added and one test per one expected result was a natural way to manage hundreds of tests). In this case, I think the fact that validating the test framework is an oddball use case is a sufficient excuse ;-). > But I do have such an "EOT" patch which I've been meaning to send out, > since it makes many of these quoting annoyances go away (though of > course it leaves the indentation). I am not sure about your "test body comes from the standard input" (not saying "I am not convinced it is a good idea" or even "I am convinced it is a bad idea"---I do not know what to think about it, not just yet). THe above illustration does make it easier to grok by keeping everything in one place. Thanks.
Am 01.07.24 um 05:20 schrieb Jeff King: > On Sat, Jun 29, 2024 at 05:35:31PM +0200, René Scharfe wrote: > >> Provide the expected output of "test-tool example-tap" verbatim instead >> of as a here-doc, to avoid distractions due to quoting, variables >> containing quotes and indentation. > > I'm not really opposed to this patch, but I wondered... > >> test_expect_success 'TAP output from unit tests' ' >> - cat >expect <<-EOF && >> - ok 1 - passing test >> - ok 2 - passing test and assertion return 1 > > If you could take the test input on stdin, like so: > > test_expect_success 'TAP output from unit tests' - <<-\EOT > cat >expect <<-\EOF > ok 1 - passing test > ok 2 - passing test and assertion return 1 > [...] > # check "'a' == '\n'" failed at t/helper/test-example-tap.c:64 > # left: 'a' > # right: '\012' > [...] > EOF > EOT > > would that be preferable to moving it to its own file? I kind of like > keeping everything in the test scripts themselves so related changes can > happen side-by-side, though I admit in this case it is intimately tied > to the separate test-example-tap.c source anyway. I can't think of an example where we keep test definitions in the same file as the code to be tested. It would be somewhat cool to empower the unit test framework to test itself, but I suspect that this nesting ability would be hard to achieve and not very useful otherwise. And would we be able to trust such a self-test? We could cheese it by putting the expected output into a special comment before (or after) the TEST invocations and letting the test script piece them together to build the expect file, something like: /* expect ok 1 - passing test */ test_res = TEST(check_res = check_int(1, ==, 1), "passing test"); That would be a bit annoying if we change something because some messages contain line numbers and the comments would affect those due to their existence alone. And there would be a ripple effect if we change the number of output lines of a test to the output of later tests. The only downside of keeping the expected output of t0080 separate that I can think of is that it might get confusing if we'd ever add more test_expect_success calls to it, but I can't imagine why we'd want to do that. > But I do have such an "EOT" patch which I've been meaning to send out, > since it makes many of these quoting annoyances go away (though of > course it leaves the indentation). Being able to pass the test code to test_expect_success as a here-doc or file to avoid nested shell quoting sounds useful in general. For t0080 we could achieve the same effect already by creating the expect file before calling test_expect_success. That has the downside of passing even when the disk is full and the files are created empty, but we can throw in a "test -s" to rule it out. René
On Mon, Jul 01, 2024 at 12:17:11PM -0700, Junio C Hamano wrote: > > But I do have such an "EOT" patch which I've been meaning to send out, > > since it makes many of these quoting annoyances go away (though of > > course it leaves the indentation). > > I am not sure about your "test body comes from the standard input" > (not saying "I am not convinced it is a good idea" or even "I am > convinced it is a bad idea"---I do not know what to think about it, > not just yet). THe above illustration does make it easier to grok > by keeping everything in one place. I just (re-)posted it in: https://lore.kernel.org/git/20240701220815.GA20293@coredump.intra.peff.net/ so you can see the improvement in some other real cases. -Peff
On Mon, Jul 01, 2024 at 09:51:31PM +0200, René Scharfe wrote: > > would that be preferable to moving it to its own file? I kind of like > > keeping everything in the test scripts themselves so related changes can > > happen side-by-side, though I admit in this case it is intimately tied > > to the separate test-example-tap.c source anyway. > > I can't think of an example where we keep test definitions in the same > file as the code to be tested. It would be somewhat cool to empower the > unit test framework to test itself, but I suspect that this nesting > ability would be hard to achieve and not very useful otherwise. And > would we be able to trust such a self-test? Yeah, I think this is really a special case, just because we're not testing specific items, but rather sanity-checking the TAP output itself. I think even in the unit tests, we're mostly carrying expected output next to the code (and the unit test harness is checking those and producing appropriate messages). It's only this "run a bunch of tests that might themselves fail and see if the output was sensible" that it's hard to do this for. I.e., t0080 is special and weird. > The only downside of keeping the expected output of t0080 separate that > I can think of is that it might get confusing if we'd ever add more > test_expect_success calls to it, but I can't imagine why we'd want to > do that. Yeah, I think it is mostly a lost cause here. We are far away from the source code whether it is a here-doc or a separate file. I guess I was responding more to the principle that external files are usually just distracting and annoying (another annoyance: they inhibit tab completion ;) ). But it is not that big a deal either way in this case. > Being able to pass the test code to test_expect_success as a here-doc or > file to avoid nested shell quoting sounds useful in general. For t0080 > we could achieve the same effect already by creating the expect file > before calling test_expect_success. That has the downside of passing > even when the disk is full and the files are created empty, but we can > throw in a "test -s" to rule it out. Yup, agreed with everything there. -Peff
Jeff King <peff@peff.net> writes: > On Mon, Jul 01, 2024 at 12:17:11PM -0700, Junio C Hamano wrote: > >> > But I do have such an "EOT" patch which I've been meaning to send out, >> > since it makes many of these quoting annoyances go away (though of >> > course it leaves the indentation). >> >> I am not sure about your "test body comes from the standard input" >> (not saying "I am not convinced it is a good idea" or even "I am >> convinced it is a bad idea"---I do not know what to think about it, >> not just yet). THe above illustration does make it easier to grok >> by keeping everything in one place. > > I just (re-)posted it in: > > https://lore.kernel.org/git/20240701220815.GA20293@coredump.intra.peff.net/ > > so you can see the improvement in some other real cases. ;-) The shells we care about (and that does not include the /bin/sh on ancient Solaris ☹) should be OK, but "IFS= read -r line" somehow makes me feel nervous. Maybe I am superstitious. Both steps look quite good. Will queue. Thanks.
On Mon, Jul 01, 2024 at 04:38:45PM -0700, Junio C Hamano wrote: > > I just (re-)posted it in: > > > > https://lore.kernel.org/git/20240701220815.GA20293@coredump.intra.peff.net/ > > > > so you can see the improvement in some other real cases. > > ;-) > > The shells we care about (and that does not include the /bin/sh on > ancient Solaris ☹) should be OK, but "IFS= read -r line" somehow > makes me feel nervous. Maybe I am superstitious. I don't know offhand of any case where it will fail. I'd prefer to start with it and see if it bites us, given that it saves us a process invocation per test (and those do add up in some cases). I also wondered if we might be able to save the syscall-per-byte overhead of "read" for some shells (since we know we are reading until EOF anyway). Using "read -N" with bash would let us do that, but obviously we'd still need to fall back to regular "read" for other shells. I don't think it's worth the complexity unless we can really show a measurable speedup (and I didn't seem to see one). -Peff
diff --git a/t/t0080-unit-test-output.sh b/t/t0080-unit-test-output.sh index 7bbb065d58..93f2defa19 100755 --- a/t/t0080-unit-test-output.sh +++ b/t/t0080-unit-test-output.sh @@ -6,54 +6,8 @@ TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh test_expect_success 'TAP output from unit tests' ' - cat >expect <<-EOF && - ok 1 - passing test - ok 2 - passing test and assertion return 1 - # check "1 == 2" failed at t/helper/test-example-tap.c:77 - # left: 1 - # right: 2 - not ok 3 - failing test - ok 4 - failing test and assertion return 0 - not ok 5 - passing TEST_TODO() # TODO - ok 6 - passing TEST_TODO() returns 1 - # todo check ${SQ}check(x)${SQ} succeeded at t/helper/test-example-tap.c:26 - not ok 7 - failing TEST_TODO() - ok 8 - failing TEST_TODO() returns 0 - # check "0" failed at t/helper/test-example-tap.c:31 - # skipping test - missing prerequisite - # skipping check ${SQ}1${SQ} at t/helper/test-example-tap.c:33 - ok 9 - test_skip() # SKIP - ok 10 - skipped test returns 1 - # skipping test - missing prerequisite - ok 11 - test_skip() inside TEST_TODO() # SKIP - ok 12 - test_skip() inside TEST_TODO() returns 1 - # check "0" failed at t/helper/test-example-tap.c:49 - not ok 13 - TEST_TODO() after failing check - ok 14 - TEST_TODO() after failing check returns 0 - # check "0" failed at t/helper/test-example-tap.c:57 - not ok 15 - failing check after TEST_TODO() - ok 16 - failing check after TEST_TODO() returns 0 - # check "!strcmp("\thello\\\\", "there\"\n")" failed at t/helper/test-example-tap.c:62 - # left: "\011hello\\\\" - # right: "there\"\012" - # check "!strcmp("NULL", NULL)" failed at t/helper/test-example-tap.c:63 - # left: "NULL" - # right: NULL - # check "${SQ}a${SQ} == ${SQ}\n${SQ}" failed at t/helper/test-example-tap.c:64 - # left: ${SQ}a${SQ} - # right: ${SQ}\012${SQ} - # check "${SQ}\\\\${SQ} == ${SQ}\\${SQ}${SQ}" failed at t/helper/test-example-tap.c:65 - # left: ${SQ}\\\\${SQ} - # right: ${SQ}\\${SQ}${SQ} - not ok 17 - messages from failing string and char comparison - # BUG: test has no checks at t/helper/test-example-tap.c:92 - not ok 18 - test with no checks - ok 19 - test with no checks returns 0 - 1..19 - EOF - ! test-tool example-tap >actual && - test_cmp expect actual + test_cmp "$TEST_DIRECTORY"/t0080/expect actual ' test_done diff --git a/t/t0080/expect b/t/t0080/expect new file mode 100644 index 0000000000..0cfa0dc6d8 --- /dev/null +++ b/t/t0080/expect @@ -0,0 +1,43 @@ +ok 1 - passing test +ok 2 - passing test and assertion return 1 +# check "1 == 2" failed at t/helper/test-example-tap.c:77 +# left: 1 +# right: 2 +not ok 3 - failing test +ok 4 - failing test and assertion return 0 +not ok 5 - passing TEST_TODO() # TODO +ok 6 - passing TEST_TODO() returns 1 +# todo check 'check(x)' succeeded at t/helper/test-example-tap.c:26 +not ok 7 - failing TEST_TODO() +ok 8 - failing TEST_TODO() returns 0 +# check "0" failed at t/helper/test-example-tap.c:31 +# skipping test - missing prerequisite +# skipping check '1' at t/helper/test-example-tap.c:33 +ok 9 - test_skip() # SKIP +ok 10 - skipped test returns 1 +# skipping test - missing prerequisite +ok 11 - test_skip() inside TEST_TODO() # SKIP +ok 12 - test_skip() inside TEST_TODO() returns 1 +# check "0" failed at t/helper/test-example-tap.c:49 +not ok 13 - TEST_TODO() after failing check +ok 14 - TEST_TODO() after failing check returns 0 +# check "0" failed at t/helper/test-example-tap.c:57 +not ok 15 - failing check after TEST_TODO() +ok 16 - failing check after TEST_TODO() returns 0 +# check "!strcmp("\thello\\", "there\"\n")" failed at t/helper/test-example-tap.c:62 +# left: "\011hello\\" +# right: "there\"\012" +# check "!strcmp("NULL", NULL)" failed at t/helper/test-example-tap.c:63 +# left: "NULL" +# right: NULL +# check "'a' == '\n'" failed at t/helper/test-example-tap.c:64 +# left: 'a' +# right: '\012' +# check "'\\' == '\''" failed at t/helper/test-example-tap.c:65 +# left: '\\' +# right: '\'' +not ok 17 - messages from failing string and char comparison +# BUG: test has no checks at t/helper/test-example-tap.c:92 +not ok 18 - test with no checks +ok 19 - test with no checks returns 0 +1..19
Provide the expected output of "test-tool example-tap" verbatim instead of as a here-doc, to avoid distractions due to quoting, variables containing quotes and indentation. Signed-off-by: René Scharfe <l.s.r@web.de> --- t/t0080-unit-test-output.sh | 48 +------------------------------------ t/t0080/expect | 43 +++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 47 deletions(-) create mode 100644 t/t0080/expect -- 2.45.2