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

[David Gibson <david@gibson.dropbear.id.au>]

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

On Fri, Oct 30, 2026 at 10:40:11AM +0100, Herve Codina wrote:
> 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.

Sort of.  Using TAP for this would, IMO, negate one of the big
advantages of moving to meson tests - being able to easily run
specific test cases.  Indeed, I'm not overly fond of TAP in general -
I'd be more likely to try converting to exeter[0].

[0] https://www.youtube.com/watch?v=4yKwN9u33TI

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

Uh.. how so?

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

I don't see how TAP really makes an incremental conversion any easier.

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

I'm aware,.

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

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

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