Re: [PATCH 4/4] meson: parse TAP output generated by our tests

[Karthik Nayak <karthik.188@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 6112 bytes --]

Patrick Steinhardt <ps@pks.im> writes:

> By default, Meson only knows to pay respect to the exit code of tests to
> judge whether or not it ran successfully. This can be changed though by
> specifying the "protocol" parameter. Next to the default "exitcode"
> protocol, Meson also supports the "tap" output that our tests already
> know to generate.
>
> Unfortunately, the "tap" protocol was incompatible with `meson test
> --interactive` and caused a hang. We have upstreamed a fix [1] though,
> so with the recent release of Meson 1.8 that fix is finally out and we
> can start using the "tap" protocol when running with a recent-enough
> version of this build tool.
>
> With this change in place, Meson now properly detects how many subtests
> ran and whether test suites have been skipped:
>
>     ```
>     $ meson test t002*
>     ninja: Entering directory `/home/pks/Development/git/build'
>      1/10 t0024-crlf-archive                  OK              0.17s   2 subtests passed
>      2/10 t0022-crlf-rename                   OK              0.18s   2 subtests passed
>      3/10 t0029-core-unsetenvvars             SKIP            0.15s
>      4/10 t0023-crlf-am                       OK              0.18s   2 subtests passed
>      5/10 t0025-crlf-renormalize              OK              0.21s   3 subtests passed
>      6/10 t0026-eol-config                    OK              0.25s   5 subtests passed
>      7/10 t0020-crlf                          OK              0.81s   36 subtests passed
>      8/10 t0028-working-tree-encoding         OK              0.85s   22 subtests passed
>      9/10 t0021-conversion                    OK              3.45s   38 subtests passed
>     10/10 t0027-auto-crlf                     OK             26.35s   2600 subtests passed
>
>     Ok:                9
>     Fail:              0
>     Skipped:           1
>     ```
>
> [1]: https://github.com/mesonbuild/meson/pull/13980
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  meson.build | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/meson.build b/meson.build
> index 94bd525dd7b..cd8df189d79 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2031,6 +2031,14 @@ if get_option('tests')
>      'timeout': 0,
>    }
>
> +  # The TAP protocol was already understood by previous versions of Meson, but
> +  # it was incompatible with the `meson test --interactive` flag.
> +  if meson.version().version_compare('>=1.8.0')
> +    test_kwargs += {
> +      'protocol': 'tap',
> +    }
> +  endif
> +
>

The change itself looks good. But I do have a question about this:

  $ meson -version
  1.8.99

  $ meson test --interactive t3206-range-diff
  ninja: Entering directory `/home/karthik/code/git/build'
  [1/28] Generating GIT-VERSION-FILE with a custom command (wrapped by
meson to set env)
  1/1 t3206-range-diff RUNNING
  >>> MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1
MESON_TEST_ITERATION=1 MALLOC_PERTURB_=119
GIT_BUILD_DIR=/home/karthik/code/git/build
ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1
UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1
/nix/store/xg75pc4yyfd5n2fimhb98ps910q5lm5n-bash-5.2p37/bin/sh
t3206-range-diff.sh
  ok 1 - setup
  ok 2 - simple A..B A..C (unmodified)
  ok 3 - simple B...C (unmodified)
  ok 4 - simple A B C (unmodified)
  ok 5 - simple A..B A..C (unmodified) with --abbrev
  ok 6 - A^! and A^-<n> (unmodified)
  ok 7 - A^{/..} is not mistaken for a range
  ok 8 - trivial reordering
  ok 9 - removed a commit
  ok 10 - added a commit
  ok 11 - new base, A B C
  ok 12 - new base, B...C
  ok 13 - changed commit
  ok 14 - changed commit with --no-patch diff option
  ok 15 - changed commit with --stat diff option
  ok 16 - changed commit with sm config
  ok 17 - renamed file
  ok 18 - file with mode only change
  ok 19 - file added and later removed
  ok 20 - no commits on one side
  ok 21 - changed message
  ok 22 - dual-coloring
  ok 23 - format-patch --range-diff=topic
  ok 24 - format-patch --range-diff=main..topic
  ok 25 - --range-diff implies --cover-letter for multi-patch series
  ok 26 - explicit --no-cover-letter defeats implied --cover-letter
  ok 27 - format-patch --range-diff as commentary
  ok 28 - format-patch --range-diff reroll-count with a non-integer
  ok 29 - format-patch --range-diff reroll-count with a integer
  ok 30 - format-patch --range-diff with v0
  ok 31 - range-diff overrides diff.noprefix internally
  ok 32 - basic with modified format.pretty with suffix
  ok 33 - basic with modified format.pretty without "commit "
  ok 34 - range-diff compares notes by default
  ok 35 - range-diff with --no-notes
  ok 36 - range-diff with multiple --notes
  ok 37 - range-diff with --notes=custom does not show default notes
  ok 38 - format-patch --range-diff does not compare notes by default
  ok 39 - format-patch --notes=custom --range-diff only compares custom notes
  ok 40 - format-patch --range-diff with --no-notes
  ok 41 - format-patch --range-diff with --notes
  ok 42 - format-patch --range-diff with format.notes config
  ok 43 - format-patch --range-diff with multiple notes
  ok 44 - --left-only/--right-only
  ok 45 - ranges with pathspecs
  ok 46 - submodule changes are shown irrespective of diff.submodule
  ok 47 - --diff-merges
  # passed all 47 test(s)
  1..47
  1/1 t3206-range-diff IGNORED         1.76s


  Ok:                0
  Fail:              0
  Ignored:           1


  $ meson test t3206-range-diff
  ninja: Entering directory `/home/karthik/code/git/build'
  [1/28] Generating GIT-VERSION-FILE with a custom command (wrapped by
meson to set env)
  1/1 t3206-range-diff        OK              1.70s   47 subtests passed

  Ok:                1
  Fail:              0

  Full log written to
  /home/karthik/code/git/build/meson-logs/testlog.txt

Shouldn't the '--interactive' flag also produce 'Ok: 1'. Instead it is
printing out 'Ignored: 1'. This is while I was testing on your series.
Seems to be fine on master.

>    subdir('t')
>  endif
>
>
> --
> 2.49.0.1045.g170613ef41.dirty

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]