[RFC PATCH 10/77] tests: Add basic metadata tests

Herve Codina posted 77 patches 3 weeks, 5 days ago
[RFC PATCH 10/77] tests: Add basic metadata tests
Posted by Herve Codina 3 weeks, 5 days ago
This first test is related to local phandle references (FDT_REF_LOCAL
dtb tag).

The test pattern used is
  - Generate a dts (xxx.dts.dts) from an input dts
  - Check this generated dts against expected contents
  - Generate a dtb (xxx.dtb) from the same input dts
  - Check this generated dtb against expected contents
  - Generate a dts (xxx.dtb.dts) from the generated dtb
  - Check this generated dts against expected contents
  - Generate a dtb (xxx.dtb.dts.dtb) from the generated dts
  - Check this generated dtb, expect the same contents as for xxx.dtb

Even if only one meta-data feature is currently tested in this tests
introduction, use a loop in order to ease future addition consisting in
adding new input dts as soon as new meta-data feature are supported.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 tests/meson.build                      |  3 +-
 tests/metadata_reflocal.dtb.dts.expect | 23 ++++++++++
 tests/metadata_reflocal.dtb.expect     | 20 +++++++++
 tests/metadata_reflocal.dts            | 27 ++++++++++++
 tests/metadata_reflocal.dts.dts.expect | 23 ++++++++++
 tests/run_tests.sh                     | 58 +++++++++++++++++++++++++-
 6 files changed, 152 insertions(+), 2 deletions(-)
 create mode 100644 tests/metadata_reflocal.dtb.dts.expect
 create mode 100644 tests/metadata_reflocal.dtb.expect
 create mode 100644 tests/metadata_reflocal.dts
 create mode 100644 tests/metadata_reflocal.dts.dts.expect

diff --git a/tests/meson.build b/tests/meson.build
index 37bfd47..e81a2e1 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -140,7 +140,8 @@ run_test_types = [
   'fdtget',
   'fdtput',
   'fdtdump',
-  'fdtoverlay'
+  'fdtoverlay',
+  'metadata'
 ]
 run_test_deps = [
   dtc_tools, dumptrees_dtb, tests_exe
diff --git a/tests/metadata_reflocal.dtb.dts.expect b/tests/metadata_reflocal.dtb.dts.expect
new file mode 100644
index 0000000..076c17a
--- /dev/null
+++ b/tests/metadata_reflocal.dtb.dts.expect
@@ -0,0 +1,23 @@
+/dts-v1/;
+
+/ {
+
+	node-a {
+
+		subnode-a {
+			phandle = <0x01>;
+		};
+	};
+
+	node-b {
+		phandle = <0x02>;
+	};
+
+	node-c {
+	};
+
+	node-d {
+		ref-subnode-a = <&{/node-a/subnode-a}>;
+		ref-node-b = <0x123 0x456 &{/node-b} 0x789>;
+	};
+};
diff --git a/tests/metadata_reflocal.dtb.expect b/tests/metadata_reflocal.dtb.expect
new file mode 100644
index 0000000..33b3896
--- /dev/null
+++ b/tests/metadata_reflocal.dtb.expect
@@ -0,0 +1,20 @@
+/dts-v1/;
+
+/ {
+    node-a {
+        subnode-a {
+            phandle = <0x00000001>;
+        };
+    };
+    node-b {
+        phandle = <0x00000002>;
+    };
+    node-c {
+    };
+    node-d {
+        ref-subnode-a = <0x00000001>;
+        // [FDT_REF_LOCAL] ref-subnode-a[0]
+        ref-node-b = <0x00000123 0x00000456 0x00000002 0x00000789>;
+        // [FDT_REF_LOCAL] ref-node-b[8]
+    };
+};
diff --git a/tests/metadata_reflocal.dts b/tests/metadata_reflocal.dts
new file mode 100644
index 0000000..f04d24f
--- /dev/null
+++ b/tests/metadata_reflocal.dts
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause)
+/*
+ * Copyright (C) 2026 Bootlin
+ */
+
+/dts-v1/;
+
+/ {
+	node-a {
+		subnode_a: subnode-a {
+		};
+	};
+
+	node_b: node-b {
+	};
+
+	node-c {
+	};
+
+	node_d: node-d {
+		ref-subnode-a = <&subnode_a>;
+	};
+};
+
+&node_d {
+	ref-node-b = <0x123 0x456 &node_b 0x789>;
+};
diff --git a/tests/metadata_reflocal.dts.dts.expect b/tests/metadata_reflocal.dts.dts.expect
new file mode 100644
index 0000000..b72b545
--- /dev/null
+++ b/tests/metadata_reflocal.dts.dts.expect
@@ -0,0 +1,23 @@
+/dts-v1/;
+
+/ {
+
+	node-a {
+
+		subnode_a: subnode-a {
+			phandle = <0x01>;
+		};
+	};
+
+	node_b: node-b {
+		phandle = <0x02>;
+	};
+
+	node-c {
+	};
+
+	node_d: node-d {
+		ref-subnode-a = <&subnode_a>;
+		ref-node-b = <0x123 0x456 &node_b 0x789>;
+	};
+};
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index f07092b..7a8bdbc 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -1090,6 +1090,59 @@ fdtoverlay_tests() {
     run_wrap_test test "$bd" = "$pd"
 }
 
+# $1: f1 file
+# $2: f2 file
+check_diff () {
+    printf "diff $1 $2:	"
+    local f1="$1"
+    local f2="$2"
+    (
+        if diff $f1 $f2 >/dev/null; then
+            PASS
+        else
+            if [ -z "$QUIET_TEST" ]; then
+                echo "DIFF :-:"
+                diff -u $f1 $f2
+            fi
+            FAIL "Results differ from expected"
+        fi
+    )
+}
+
+# $1: dtb file
+# $2: out file
+wrap_fdtdump () {
+    printf "wrap_fdtdump $1:	"
+    local dtb="$1"
+    local out="$2"
+    (
+        if $FDTDUMP ${dtb} 2>/dev/null >${out}; then
+            PASS
+        else
+            FAIL
+        fi
+    )
+}
+
+metadata_tests() {
+	for dt in metadata_reflocal; do
+		run_dtc_test -I dts -O dts -o $dt.dts.dts "$SRCDIR/$dt.dts"
+		base_run_test check_diff $dt.dts.dts "$SRCDIR/$dt.dts.dts.expect"
+		run_dtc_test -I dts -O dtb -o $dt.dtb "$SRCDIR/$dt.dts"
+		base_run_test wrap_fdtdump $dt.dtb $dt.dtb.out
+		# Remove unneeded comments
+		sed -i '/^\/\/ /d' $dt.dtb.out
+		base_run_test check_diff $dt.dtb.out "$SRCDIR/$dt.dtb.expect"
+		run_dtc_test -I dtb -O dts -o $dt.dtb.dts $dt.dtb
+		base_run_test check_diff $dt.dtb.dts "$SRCDIR/$dt.dtb.dts.expect"
+		run_dtc_test -I dts -O dtb -o $dt.dtb.dts.dtb $dt.dtb.dts
+		base_run_test wrap_fdtdump $dt.dtb.dts.dtb $dt.dtb.dts.dtb.out
+		# Remove unneeded comments
+		sed -i '/^\/\/ /d' $dt.dtb.dts.dtb.out
+		base_run_test check_diff $dt.dtb.dts.dtb.out "$SRCDIR/$dt.dtb.expect"
+	done
+}
+
 pylibfdt_tests () {
     run_dtc_test -I dts -O dtb -o test_props.dtb "$SRCDIR/test_props.dts"
     TMP=/tmp/tests.stderr.$$
@@ -1129,7 +1182,7 @@ while getopts "vt:me" ARG ; do
 done
 
 if [ -z "$TESTSETS" ]; then
-    TESTSETS="libfdt utilfdt dtc dtbs_equal fdtget fdtput fdtdump fdtoverlay"
+    TESTSETS="libfdt utilfdt dtc dtbs_equal fdtget fdtput fdtdump fdtoverlay metadata"
 
     # Test pylibfdt if the libfdt Python module is available.
     if ! $no_python; then
@@ -1169,6 +1222,9 @@ for set in $TESTSETS; do
         "fdtoverlay")
 	    fdtoverlay_tests
 	    ;;
+	"metadata")
+	    metadata_tests
+	    ;;
     esac
 done
 
-- 
2.52.0
Re: [RFC PATCH 10/77] tests: Add basic metadata tests
Posted by David Gibson 3 weeks, 3 days ago
On Mon, Jan 12, 2026 at 03:19:00PM +0100, Herve Codina wrote:
> This first test is related to local phandle references (FDT_REF_LOCAL
> dtb tag).
> 
> The test pattern used is
>   - Generate a dts (xxx.dts.dts) from an input dts
>   - Check this generated dts against expected contents
>   - Generate a dtb (xxx.dtb) from the same input dts
>   - Check this generated dtb against expected contents
>   - Generate a dts (xxx.dtb.dts) from the generated dtb
>   - Check this generated dts against expected contents
>   - Generate a dtb (xxx.dtb.dts.dtb) from the generated dts
>   - Check this generated dtb, expect the same contents as for xxx.dtb
> 
> Even if only one meta-data feature is currently tested in this tests
> introduction, use a loop in order to ease future addition consisting in
> adding new input dts as soon as new meta-data feature are supported.

As a rule of tumb, I prefer tests to be introduced in the same patch
that introduces the feature tested.  Usually, I don't care that much,
but in a giant series like this, it would really help review (the
tests help to document the feature being added without switching
between patches).

> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  tests/meson.build                      |  3 +-
>  tests/metadata_reflocal.dtb.dts.expect | 23 ++++++++++
>  tests/metadata_reflocal.dtb.expect     | 20 +++++++++
>  tests/metadata_reflocal.dts            | 27 ++++++++++++
>  tests/metadata_reflocal.dts.dts.expect | 23 ++++++++++
>  tests/run_tests.sh                     | 58 +++++++++++++++++++++++++-
>  6 files changed, 152 insertions(+), 2 deletions(-)
>  create mode 100644 tests/metadata_reflocal.dtb.dts.expect
>  create mode 100644 tests/metadata_reflocal.dtb.expect
>  create mode 100644 tests/metadata_reflocal.dts
>  create mode 100644 tests/metadata_reflocal.dts.dts.expect
> 
> diff --git a/tests/meson.build b/tests/meson.build
> index 37bfd47..e81a2e1 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -140,7 +140,8 @@ run_test_types = [
>    'fdtget',
>    'fdtput',
>    'fdtdump',
> -  'fdtoverlay'
> +  'fdtoverlay',
> +  'metadata'
>  ]
>  run_test_deps = [
>    dtc_tools, dumptrees_dtb, tests_exe
> diff --git a/tests/metadata_reflocal.dtb.dts.expect b/tests/metadata_reflocal.dtb.dts.expect
> new file mode 100644
> index 0000000..076c17a
> --- /dev/null
> +++ b/tests/metadata_reflocal.dtb.dts.expect
> @@ -0,0 +1,23 @@
> +/dts-v1/;
> +
> +/ {
> +
> +	node-a {
> +
> +		subnode-a {
> +			phandle = <0x01>;
> +		};
> +	};
> +
> +	node-b {
> +		phandle = <0x02>;
> +	};
> +
> +	node-c {
> +	};
> +
> +	node-d {
> +		ref-subnode-a = <&{/node-a/subnode-a}>;
> +		ref-node-b = <0x123 0x456 &{/node-b} 0x789>;
> +	};
> +};
> diff --git a/tests/metadata_reflocal.dtb.expect b/tests/metadata_reflocal.dtb.expect
> new file mode 100644
> index 0000000..33b3896
> --- /dev/null
> +++ b/tests/metadata_reflocal.dtb.expect
> @@ -0,0 +1,20 @@
> +/dts-v1/;
> +
> +/ {
> +    node-a {
> +        subnode-a {
> +            phandle = <0x00000001>;
> +        };
> +    };
> +    node-b {
> +        phandle = <0x00000002>;
> +    };
> +    node-c {
> +    };
> +    node-d {
> +        ref-subnode-a = <0x00000001>;
> +        // [FDT_REF_LOCAL] ref-subnode-a[0]
> +        ref-node-b = <0x00000123 0x00000456 0x00000002 0x00000789>;
> +        // [FDT_REF_LOCAL] ref-node-b[8]
> +    };
> +};
> diff --git a/tests/metadata_reflocal.dts b/tests/metadata_reflocal.dts
> new file mode 100644
> index 0000000..f04d24f
> --- /dev/null
> +++ b/tests/metadata_reflocal.dts
> @@ -0,0 +1,27 @@
> +// SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause)
> +/*
> + * Copyright (C) 2026 Bootlin
> + */
> +
> +/dts-v1/;
> +
> +/ {
> +	node-a {
> +		subnode_a: subnode-a {
> +		};
> +	};
> +
> +	node_b: node-b {
> +	};
> +
> +	node-c {
> +	};
> +
> +	node_d: node-d {
> +		ref-subnode-a = <&subnode_a>;
> +	};
> +};
> +
> +&node_d {
> +	ref-node-b = <0x123 0x456 &node_b 0x789>;
> +};
> diff --git a/tests/metadata_reflocal.dts.dts.expect b/tests/metadata_reflocal.dts.dts.expect
> new file mode 100644
> index 0000000..b72b545
> --- /dev/null
> +++ b/tests/metadata_reflocal.dts.dts.expect
> @@ -0,0 +1,23 @@
> +/dts-v1/;
> +
> +/ {
> +
> +	node-a {
> +
> +		subnode_a: subnode-a {
> +			phandle = <0x01>;
> +		};
> +	};
> +
> +	node_b: node-b {
> +		phandle = <0x02>;
> +	};
> +
> +	node-c {
> +	};
> +
> +	node_d: node-d {
> +		ref-subnode-a = <&subnode_a>;
> +		ref-node-b = <0x123 0x456 &node_b 0x789>;
> +	};
> +};
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index f07092b..7a8bdbc 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -1090,6 +1090,59 @@ fdtoverlay_tests() {
>      run_wrap_test test "$bd" = "$pd"
>  }
>  
> +# $1: f1 file
> +# $2: f2 file
> +check_diff () {
> +    printf "diff $1 $2:	"
> +    local f1="$1"
> +    local f2="$2"
> +    (
> +        if diff $f1 $f2 >/dev/null; then
> +            PASS
> +        else
> +            if [ -z "$QUIET_TEST" ]; then
> +                echo "DIFF :-:"
> +                diff -u $f1 $f2
> +            fi
> +            FAIL "Results differ from expected"
> +        fi
> +    )
> +}
> +
> +# $1: dtb file
> +# $2: out file
> +wrap_fdtdump () {
> +    printf "wrap_fdtdump $1:	"
> +    local dtb="$1"
> +    local out="$2"
> +    (
> +        if $FDTDUMP ${dtb} 2>/dev/null >${out}; then
> +            PASS
> +        else
> +            FAIL
> +        fi
> +    )
> +}
> +
> +metadata_tests() {
> +	for dt in metadata_reflocal; do
> +		run_dtc_test -I dts -O dts -o $dt.dts.dts "$SRCDIR/$dt.dts"
> +		base_run_test check_diff $dt.dts.dts "$SRCDIR/$dt.dts.dts.expect"
> +		run_dtc_test -I dts -O dtb -o $dt.dtb "$SRCDIR/$dt.dts"
> +		base_run_test wrap_fdtdump $dt.dtb $dt.dtb.out
> +		# Remove unneeded comments
> +		sed -i '/^\/\/ /d' $dt.dtb.out
> +		base_run_test check_diff $dt.dtb.out "$SRCDIR/$dt.dtb.expect"
> +		run_dtc_test -I dtb -O dts -o $dt.dtb.dts $dt.dtb
> +		base_run_test check_diff $dt.dtb.dts "$SRCDIR/$dt.dtb.dts.expect"
> +		run_dtc_test -I dts -O dtb -o $dt.dtb.dts.dtb $dt.dtb.dts
> +		base_run_test wrap_fdtdump $dt.dtb.dts.dtb $dt.dtb.dts.dtb.out
> +		# Remove unneeded comments
> +		sed -i '/^\/\/ /d' $dt.dtb.dts.dtb.out
> +		base_run_test check_diff $dt.dtb.dts.dtb.out "$SRCDIR/$dt.dtb.expect"
> +	done
> +}
> +
>  pylibfdt_tests () {
>      run_dtc_test -I dts -O dtb -o test_props.dtb "$SRCDIR/test_props.dts"
>      TMP=/tmp/tests.stderr.$$
> @@ -1129,7 +1182,7 @@ while getopts "vt:me" ARG ; do
>  done
>  
>  if [ -z "$TESTSETS" ]; then
> -    TESTSETS="libfdt utilfdt dtc dtbs_equal fdtget fdtput fdtdump fdtoverlay"
> +    TESTSETS="libfdt utilfdt dtc dtbs_equal fdtget fdtput fdtdump fdtoverlay metadata"
>  
>      # Test pylibfdt if the libfdt Python module is available.
>      if ! $no_python; then
> @@ -1169,6 +1222,9 @@ for set in $TESTSETS; do
>          "fdtoverlay")
>  	    fdtoverlay_tests
>  	    ;;
> +	"metadata")
> +	    metadata_tests
> +	    ;;
>      esac
>  done
>  
> -- 
> 2.52.0
> 
> 

-- 
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
Re: [RFC PATCH 10/77] tests: Add basic metadata tests
Posted by Herve Codina 3 weeks, 1 day ago
On Thu, 15 Jan 2026 11:50:56 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Mon, Jan 12, 2026 at 03:19:00PM +0100, Herve Codina wrote:
> > This first test is related to local phandle references (FDT_REF_LOCAL
> > dtb tag).
> > 
> > The test pattern used is
> >   - Generate a dts (xxx.dts.dts) from an input dts
> >   - Check this generated dts against expected contents
> >   - Generate a dtb (xxx.dtb) from the same input dts
> >   - Check this generated dtb against expected contents
> >   - Generate a dts (xxx.dtb.dts) from the generated dtb
> >   - Check this generated dts against expected contents
> >   - Generate a dtb (xxx.dtb.dts.dtb) from the generated dts
> >   - Check this generated dtb, expect the same contents as for xxx.dtb
> > 
> > Even if only one meta-data feature is currently tested in this tests
> > introduction, use a loop in order to ease future addition consisting in
> > adding new input dts as soon as new meta-data feature are supported.  
> 
> As a rule of tumb, I prefer tests to be introduced in the same patch
> that introduces the feature tested.  Usually, I don't care that much,
> but in a giant series like this, it would really help review (the
> tests help to document the feature being added without switching
> between patches).

Hum ok but it is worth noting that a feature needs several patches to be
fully supported.

In order to ease the review (maybe I was wrong), I chose to have distinct
patches for modification related to new dts keywords and for modification
related to new dtb tag and tried to have patches as small as possible.

The last patch of a new feature was a patch adding test.

I am open to remove patches that just add tests. In that case tests will be
added in the last patch related to a new feature. You still need to switch
between patches in that case.

Further more, during iteration, the last patch of a new feature could be
modified just because the test part is present in this patch even if other
part of the patch itself is not impacted.

I mean:
  - patch 1: related to dts keyword
  - patch 2: related to dtb + test

Update patch 1 will imply an update to patch 2.
I am not sure that this will ease the review.


To avoid that, I can merge all patches related to feature into only one
patch. This patch can be quite huge and I am not sure that it will be easy
to review it.

Once again, I am open to merging patches. Let me know what do you prefer
in terms of patch granularity.

Best regards,
Hervé
Re: [RFC PATCH 10/77] tests: Add basic metadata tests
Posted by David Gibson 2 weeks, 6 days ago
On Fri, Jan 16, 2026 at 02:36:00PM +0100, Herve Codina wrote:
> On Thu, 15 Jan 2026 11:50:56 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Mon, Jan 12, 2026 at 03:19:00PM +0100, Herve Codina wrote:
> > > This first test is related to local phandle references (FDT_REF_LOCAL
> > > dtb tag).
> > > 
> > > The test pattern used is
> > >   - Generate a dts (xxx.dts.dts) from an input dts
> > >   - Check this generated dts against expected contents
> > >   - Generate a dtb (xxx.dtb) from the same input dts
> > >   - Check this generated dtb against expected contents
> > >   - Generate a dts (xxx.dtb.dts) from the generated dtb
> > >   - Check this generated dts against expected contents
> > >   - Generate a dtb (xxx.dtb.dts.dtb) from the generated dts
> > >   - Check this generated dtb, expect the same contents as for xxx.dtb
> > > 
> > > Even if only one meta-data feature is currently tested in this tests
> > > introduction, use a loop in order to ease future addition consisting in
> > > adding new input dts as soon as new meta-data feature are supported.  
> > 
> > As a rule of tumb, I prefer tests to be introduced in the same patch
> > that introduces the feature tested.  Usually, I don't care that much,
> > but in a giant series like this, it would really help review (the
> > tests help to document the feature being added without switching
> > between patches).
> 
> Hum ok but it is worth noting that a feature needs several patches to be
> fully supported.

Right, it's a general guideline, not a hard and fast rule.

In most cases I'd suggest putting the tests in with the patch that
adds the piece they need to work.  That does mean that if some of the
simpler tests can operate with only some of the patches, moving those
tests earlier.

Again, only a guideline, but it helps review because the tests act as
documentation for the functionality the patch is adding.

> In order to ease the review (maybe I was wrong), I chose to have distinct
> patches for modification related to new dts keywords and for modification
> related to new dtb tag and tried to have patches as small as
> possible.

Small is usually good.  However, if it's so small that the patch
doesn't express a complete idea so you have to keep referencing
surrounding patches to make sense of it, then it's no longer good.
Fwiw, think these patches are mostly well divided, but as above,
folding tests in with code changes is usually better because the tests
help show what the new code is expected to do.

[I will note that having the tests as a separate patch, usuallly
*before* the code changes is very useful during development.  But git
makes it pretty easy to re-order and fold things together when you're
ready to post]

> The last patch of a new feature was a patch adding test.
> 
> I am open to remove patches that just add tests. In that case tests will be
> added in the last patch related to a new feature. You still need to switch
> between patches in that case.

Yes, but not quite as much.  And even less if those tests which can be
moved earlier are moved earlier.

> Further more, during iteration, the last patch of a new feature could be
> modified just because the test part is present in this patch even if other
> part of the patch itself is not impacted.
> 
> I mean:
>   - patch 1: related to dts keyword
>   - patch 2: related to dtb + test
> 
> Update patch 1 will imply an update to patch 2.
> I am not sure that this will ease the review.

Point taken.  For tests that intrinsically require things from
multiple earlier patches, having them separate probably makes sense.
Often at least some of the tests can be more closely related to a
single patch though.  When possible, that's generally preferable.

> To avoid that, I can merge all patches related to feature into only one
> patch. This patch can be quite huge and I am not sure that it will be easy
> to review it.

No, definitely don't do that.

> Once again, I am open to merging patches. Let me know what do you prefer
> in terms of patch granularity.
> 
> 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