Re: [PATCH v11 4/7] tests: Add overlay tests

[David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>]

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

On Tue, Nov 29, 2016 at 01:11:43PM +0200, Pantelis Antoniou wrote:
> Hi David,
> 
> > On Nov 29, 2016, at 05:08 , David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > 
> > On Mon, Nov 28, 2016 at 06:05:38PM +0200, Pantelis Antoniou wrote:
> >> Add a number of tests for dynamic objects/overlays.
> >> 
> >> Re-use the original test by moving the contents to a .dtsi include
> >> 
> >> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> >> ---
> >> tests/overlay_overlay_dtc.dts     | 76 +----------------------------------
> >> tests/overlay_overlay_dtc.dtsi    | 83 +++++++++++++++++++++++++++++++++++++++
> >> tests/overlay_overlay_new_dtc.dts | 11 ++++++
> >> tests/overlay_overlay_simple.dts  | 12 ++++++
> >> tests/run_tests.sh                | 41 +++++++++++++++++++
> >> 5 files changed, 148 insertions(+), 75 deletions(-)
> >> create mode 100644 tests/overlay_overlay_dtc.dtsi
> >> create mode 100644 tests/overlay_overlay_new_dtc.dts
> >> create mode 100644 tests/overlay_overlay_simple.dts
> >> 
> >> diff --git a/tests/overlay_overlay_dtc.dts b/tests/overlay_overlay_dtc.dts
> >> index 30d2362..ca943ea 100644
> >> --- a/tests/overlay_overlay_dtc.dts
> >> +++ b/tests/overlay_overlay_dtc.dts
> >> @@ -8,78 +8,4 @@
> >> /dts-v1/;
> >> /plugin/;
> >> 
> >> -/ {
> >> -	/* Test that we can change an int by another */
> >> -	fragment@0 {
> >> -		target = <&test>;
> >> -
> >> -		__overlay__ {
> >> -			test-int-property = <43>;
> >> -		};
> >> -	};
> >> -
> >> -	/* Test that we can replace a string by a longer one */
> >> -	fragment@1 {
> >> -		target = <&test>;
> >> -
> >> -		__overlay__ {
> >> -			test-str-property = "foobar";
> >> -		};
> >> -	};
> >> -
> >> -	/* Test that we add a new property */
> >> -	fragment@2 {
> >> -		target = <&test>;
> >> -
> >> -		__overlay__ {
> >> -			test-str-property-2 = "foobar2";
> >> -		};
> >> -	};
> >> -
> >> -	/* Test that we add a new node (by phandle) */
> >> -	fragment@3 {
> >> -		target = <&test>;
> >> -
> >> -		__overlay__ {
> >> -			new-node {
> >> -				new-property;
> >> -			};
> >> -		};
> >> -	};
> >> -
> >> -	fragment@5 {
> >> -		target = <&test>;
> >> -
> >> -		__overlay__ {
> >> -			local: new-local-node {
> >> -				new-property;
> >> -			};
> >> -		};
> >> -	};
> >> -
> >> -	fragment@6 {
> >> -		target = <&test>;
> >> -
> >> -		__overlay__ {
> >> -			test-phandle = <&test>, <&local>;
> >> -		};
> >> -	};
> >> -
> >> -	fragment@7 {
> >> -		target = <&test>;
> >> -
> >> -		__overlay__ {
> >> -			test-several-phandle = <&local>, <&local>;
> >> -		};
> >> -	};
> >> -
> >> -	fragment@8 {
> >> -		target = <&test>;
> >> -
> >> -		__overlay__ {
> >> -			sub-test-node {
> >> -				new-sub-test-property;
> >> -			};
> >> -		};
> >> -	};
> >> -};
> >> +/include/ "overlay_overlay_dtc.dtsi"
> > 
> > Don't duplicate this, just replace it with the new style.  This only
> > existed as essentially documentation for the libfdt overlay
> > application stuff.  Since the new dtc won't support the old tag
> > format, there's no point having a test for it.
> 
> The parser now handles both tag formats just fine. I could remove support
> for it if you’re willing to tackle the flak.

Oh, sorry, I missed that.

I'd suggest having that controlled by the same "backwards compat"
option to control magic number and other things.  Although changing
parser behaviour based on flags can get fiddly.

> 
> >> diff --git a/tests/overlay_overlay_dtc.dtsi b/tests/overlay_overlay_dtc.dtsi
> >> new file mode 100644
> >> index 0000000..8ea8d5d
> >> --- /dev/null
> >> +++ b/tests/overlay_overlay_dtc.dtsi
> >> @@ -0,0 +1,83 @@
> >> +/*
> >> + * Copyright (c) 2016 NextThing Co
> >> + * Copyright (c) 2016 Free Electrons
> >> + * Copyright (c) 2016 Konsulko Inc.
> >> + *
> >> + * SPDX-License-Identifier:	GPL-2.0+
> >> + */
> >> +
> >> +/ {
> >> +	/* Test that we can change an int by another */
> >> +	fragment@0 {
> >> +		target = <&test>;
> >> +
> >> +		__overlay__ {
> >> +			test-int-property = <43>;
> >> +		};
> >> +	};
> >> +
> >> +	/* Test that we can replace a string by a longer one */
> >> +	fragment@1 {
> >> +		target = <&test>;
> >> +
> >> +		__overlay__ {
> >> +			test-str-property = "foobar";
> >> +		};
> >> +	};
> >> +
> >> +	/* Test that we add a new property */
> >> +	fragment@2 {
> >> +		target = <&test>;
> >> +
> >> +		__overlay__ {
> >> +			test-str-property-2 = "foobar2";
> >> +		};
> >> +	};
> >> +
> >> +	/* Test that we add a new node (by phandle) */
> >> +	fragment@3 {
> >> +		target = <&test>;
> >> +
> >> +		__overlay__ {
> >> +			new-node {
> >> +				new-property;
> >> +			};
> >> +		};
> >> +	};
> >> +
> >> +	fragment@5 {
> >> +		target = <&test>;
> >> +
> >> +		__overlay__ {
> >> +			local: new-local-node {
> >> +				new-property;
> >> +			};
> >> +		};
> >> +	};
> >> +
> >> +	fragment@6 {
> >> +		target = <&test>;
> >> +
> >> +		__overlay__ {
> >> +			test-phandle = <&test>, <&local>;
> >> +		};
> >> +	};
> >> +
> >> +	fragment@7 {
> >> +		target = <&test>;
> >> +
> >> +		__overlay__ {
> >> +			test-several-phandle = <&local>, <&local>;
> >> +		};
> >> +	};
> >> +
> >> +	fragment@8 {
> >> +		target = <&test>;
> >> +
> >> +		__overlay__ {
> >> +			sub-test-node {
> >> +				new-sub-test-property;
> >> +			};
> >> +		};
> >> +	};
> >> +};
> >> diff --git a/tests/overlay_overlay_new_dtc.dts b/tests/overlay_overlay_new_dtc.dts
> >> new file mode 100644
> >> index 0000000..14d3f54
> >> --- /dev/null
> >> +++ b/tests/overlay_overlay_new_dtc.dts
> >> @@ -0,0 +1,11 @@
> >> +/*
> >> + * Copyright (c) 2016 NextThing Co
> >> + * Copyright (c) 2016 Free Electrons
> >> + * Copyright (c) 2016 Konsulko Inc.
> >> + *
> >> + * SPDX-License-Identifier:	GPL-2.0+
> >> + */
> >> +
> >> +/dts-v1/ /plugin/;
> >> +
> >> +/include/ "overlay_overlay_dtc.dtsi"
> >> diff --git a/tests/overlay_overlay_simple.dts b/tests/overlay_overlay_simple.dts
> >> new file mode 100644
> >> index 0000000..8657e1e
> >> --- /dev/null
> >> +++ b/tests/overlay_overlay_simple.dts
> >> @@ -0,0 +1,12 @@
> >> +/*
> >> + * Copyright (c) 2016 Konsulko Inc.
> >> + *
> >> + * SPDX-License-Identifier:	GPL-2.0+
> >> + */
> >> +
> >> +/dts-v1/;
> >> +/plugin/;
> >> +
> >> +&test {
> >> +	test-int-property = <43>;
> >> +};
> >> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> >> index e4139dd..74af0ff 100755
> >> --- a/tests/run_tests.sh
> >> +++ b/tests/run_tests.sh
> >> @@ -181,6 +181,47 @@ overlay_tests () {
> >>         run_dtc_test -@ -I dts -O dtb -o overlay_base_with_symbols.test.dtb overlay_base.dts
> >>         run_dtc_test -@ -I dts -O dtb -o overlay_overlay_with_symbols.test.dtb overlay_overlay_dtc.dts
> >>         run_test overlay overlay_base_with_symbols.test.dtb overlay_overlay_with_symbols.test.dtb
> >> +
> >> +        # new /plugin/ format
> >> +        run_dtc_test -@ -I dts -O dtb -o overlay_overlay_new_with_symbols.test.dtb overlay_overlay_new_dtc.dts
> >> +	run_test check_path overlay_overlay_new_with_symbols.test.dtb exists "/__symbols__"
> >> +	run_test check_path overlay_overlay_new_with_symbols.test.dtb exists "/__fixups__"
> >> +	run_test check_path overlay_overlay_new_with_symbols.test.dtb exists "/__local_fixups__"
> > 
> > Looks like you're mixing tabs and spaces here.  I don't really mind
> > which, but keep it consistent at least at the same indentation level.
> > 
> 
> Oh, sorry, I use tabs but this sections has spaces… Will fix.

Well, it's more tha the first line in the block seems to be using
spaces, then the rest using tabs.

> 
> >> +        # test new magic option
> >> +        run_dtc_test -M@ -I dts -O dtb -o overlay_overlay_with_symbols_new_magic.test.dtb overlay_overlay_dtc.dts
> >> +	run_test check_path overlay_overlay_with_symbols_new_magic.test.dtb exists "/__symbols__"
> >> +	run_test check_path overlay_overlay_with_symbols_new_magic.test.dtb exists "/__fixups__"
> >> +	run_test check_path overlay_overlay_with_symbols_new_magic.test.dtb exists "/__local_fixups__"
> >> +
> >> +        # test plugin source to dtb and back
> >> +        run_dtc_test -@ -I dtb -O dts -o overlay_overlay_dtc.test.dts overlay_overlay_with_symbols.test.dtb
> >> +        run_dtc_test -@ -I dts -O dtb -o overlay_overlay_with_symbols.test.test.dtb overlay_overlay_dtc.test.dts
> >> +        run_test dtbs_equal_ordered overlay_overlay_with_symbols.test.dtb overlay_overlay_with_symbols.test.test.dtb
> >> +
> >> +	# test plugin source to dtb and back (with new magic)
> >> +        run_dtc_test -@ -I dtb -O dts -o overlay_overlay_dtc_new_magic.test.dts overlay_overlay_with_symbols_new_magic.test.dtb
> >> +        run_dtc_test -@ -I dts -O dtb -o overlay_overlay_with_symbols_new_magic.test.test.dtb overlay_overlay_dtc_new_magic.test.dts
> >> +        run_test dtbs_equal_ordered overlay_overlay_with_symbols_new_magic.test.dtb overlay_overlay_with_symbols_new_magic.test.test.dtb
> >> +
> >> +        # test plugin auto-generation without using -@
> >> +        run_dtc_test -I dts -O dtb -o overlay_overlay_new_with_symbols_auto.test.dtb overlay_overlay_dtc.dts
> >> +	run_test check_path overlay_overlay_new_with_symbols_auto.test.dtb exists "/__symbols__"
> >> +	run_test check_path overlay_overlay_new_with_symbols_auto.test.dtb exists "/__fixups__"
> >> +	run_test check_path overlay_overlay_new_with_symbols_auto.test.dtb exists "/__local_fixups__"
> >> +
> >> +        # Test suppression of fixups
> >> +        run_dtc_test -F -@ -I dts -O dtb -o overlay_base_with_symbols_no_fixups.test.dtb overlay_base.dts
> >> +	run_test check_path overlay_base_with_symbols_no_fixups.test.dtb exists "/__symbols__"
> >> +	run_test check_path overlay_base_with_symbols_no_fixups.test.dtb not-exists "/__fixups__"
> >> +	run_test check_path overlay_base_with_symbols_no_fixups.test.dtb not-exists "/__local_fixups__"
> >> +
> >> +        # Test generation of aliases insted of symbols
> >> +        run_dtc_test -A -I dts -O dtb -o overlay_overlay_with_aliases.dtb overlay_overlay_dtc.dts
> >> +	run_test check_path overlay_overlay_with_aliases.dtb exists "/aliases"
> >> +	run_test check_path overlay_overlay_with_aliases.dtb exists "/__symbols__"
> >> +	run_test check_path overlay_overlay_with_aliases.dtb exists "/__fixups__"
> >> +	run_test check_path overlay_overlay_with_aliases.dtb exists "/__local_fixups__"
> >>     fi
> >> 
> >>     # Bad fixup tests
> > 
> 
> Regards
> 
> — Pantelis
> 

-- 
David Gibson			| 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: 819 bytes --]