Re: [PATCH v3 1/4] tests: Check the message when dtc fails

[Herve Codina <herve.codina@bootlin.com>]

Hi,

On Fri, 30 Oct 2026 14:36:22 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

...

> > > @@ -319,7 +322,8 @@ libfdt_overlay_tests () {
> > >  	run_dtc_test -I dts -O dtb -o $tree.test.dtb "$SRCDIR/$tree.dts"
> > >  	run_test overlay_bad_fixup overlay_base_manual_symbols.test.dtb $tree.test.dtb
> > >      done
> > > -    run_sh_test "$SRCDIR/dtc-fatal.sh" -I dts -O dtb -o /dev/null "$SRCDIR/fixup-ref-to-path.dts"
> > > +    run_wrap_error_msg_test "Can't generate fixup for reference to path" \
> > > +	$VALGRIND $DTC -I dts -O dtb -o /dev/null "$SRCDIR/fixup-ref-to-path.dts"  
> > 
> > I don't think that the $VALGRIND here is a good idea. Also this is not consistent
> > with other dtc test calls. Those tests calls (other than using a script) use
> > run_dtc_test(). run_dtc_test() use $VALGRIND internally.  
> 
> So, this is reminding me why I think dtc-fatal.sh existed in the first
> place:
>   1. We want to wrap all dtc invocations in $VALGRIND
>   
>   2. We don't want to use valgrind on other commands we invoke via
>      wrap_error
> 
>   3. We want to avoid individual test invocations from dealing with
>      $VALGRIND
> 
> Hence, two different wrappers: run_wrap_error_test for general
> commands, dtc-fatal.sh for errors from dtc specifically.  Why one is a
> function and one a separate script I don't recall, although I have a
> nasty feeling there might have been a particular reason for having the
> script, though I've forgotten what it might have been.
> 
> I can think of one more modern reason to prefer scripts over shell
> functions:  the current integration of run_tests.sh into meson is
> pretty ugly.  Rather than using meson to manage testcases (which it
> can do reasonably well), we just invoke each "set" of tests from
> run-tests.sh as a single meson testcase.  That means more levels of
> logs to figure out exactly what went wrong for a meson test failure.
> 
> At some point it might be nice to eliminate run_tests.sh and make the
> testsuite meson native.  For that, shell function wrappers are
> obviously not ideal.

Have you consider switching current tests to TAP (Test Anything Protocol)?

TAP is supported by meson test.

If at some point the plan is to move current tests to a a better meson
integration, I suggest to consider TAP. Indeed, this will allow a finer
grain in test results compared to a meson basic executable test.

We can move tests from current tests to TAP by parts to ease this moving
task.

run_test.sh can provide its output in TAP format.

Cleaner test harnesses are create step by step by extracting parts from
run_test.sh until run_test.sh is empty.

Also all new tests should be TAP tests.

Libraries exist to help building TAP tests. One lib to chose for C test
program (if they are run out of a test shell script) and an other for
Shell test scripts should be enough.

> 
> The split between patches 1 & 2 here isn't ideal, btw.  The VALGRIND
> behaviour change is correct, but not logicall connected to the
> run_wrap_error_test / dtc-fatal.sh consolidation.  Meanwhile removing
> dtc-fatal.sh _does_ belong with the patch that makes it possible
> (given that's a single fairly small patch).
> 
> So, where next.
> 
> I think keep dtc-fatal.sh for the time being, but extend it with the
> -m option to check for specific messages.  Consider moving
> run_wrap_error_test into its own script (which dtc-fatal.sh could
> call).

Ok.

Best regards,
Hervé