[PATCH v1 0/2] tools/tests: test harness fragment

[PATCH v1 0/2] tools/tests: test harness fragment

[dmukhin]

This is a follow-on mini-series based on the original feedback in [1].

Patch 1 addresses the remaining feedback from [1].
Patch 2 adds a new fragment for auto-generating test harness dependencies.

[1] https://lore.kernel.org/xen-devel/aLmZLm2_G48yfPWR@l14/
[2] CI: https://gitlab.com/xen-project/people/dmukhin/xen/-/pipelines/2195624771

Denis Mukhin (2):
  tests: fixup domid test harness dependencies
  tests: introduce Tests.mk fragment

 tools/Tests.mk             | 34 ++++++++++++++++++++++++++++++++++
 tools/tests/domid/Makefile | 28 +---------------------------
 2 files changed, 35 insertions(+), 27 deletions(-)
 create mode 100644 tools/Tests.mk

-- 
2.52.0

[PATCH v1 1/2] tests: fixup domid test harness dependencies

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com> 

There can be multiple test harnesses per one test target. Fix that by
iterating over all prerequisites in emit-harness-nested-rule().

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 tools/tests/domid/Makefile | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/tools/tests/domid/Makefile b/tools/tests/domid/Makefile
index 753129029ed9..1a2129d20655 100644
--- a/tools/tests/domid/Makefile
+++ b/tools/tests/domid/Makefile
@@ -14,16 +14,18 @@ $(shell sed -n \
     's/^[ \t]*# *include[ \t]*[<"]\([^">]*\)[">].*/\1/p' $(1) 2>/dev/null)
 endef
 
-# NB: $1 cannot be a list
+# $1 target
+# $2 list of test harnesses
 define emit-harness-nested-rule
-$(1): $(CURDIR)/harness.h
-	mkdir -p $$(@D);
-	ln -sf $$< $$@;
+$(1): $(2)
+	mkdir -p $$(@D); \
+	for i in $$<; do ln -sf $$$$i $$@; done
 
 endef
 
 define emit-harness-rules
-$(foreach x,$(2),$(call emit-harness-nested-rule,$(CURDIR)/generated/$(x)))
+$(foreach x,$(2),$(call \
+    emit-harness-nested-rule,$(CURDIR)/generated/$(x),$(CURDIR)/harness.h))
 $(1:.c=.o): $(addprefix $(CURDIR)/generated/,$(2))
 endef
 
-- 
2.52.0

Re: [PATCH v1 1/2] tests: fixup domid test harness dependencies

[Jan Beulich]

On 04.12.2025 13:37, dmukhin@xen.org wrote:
> From: Denis Mukhin <dmukhin@ford.com> 
> 
> There can be multiple test harnesses per one test target. Fix that by
> iterating over all prerequisites in emit-harness-nested-rule().

This reads as if previously there was no iterating, and you add some.
What you do it further parameterize the existing macro. That anomaly
actually looks to reflect itself ...

> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
>  tools/tests/domid/Makefile | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/tests/domid/Makefile b/tools/tests/domid/Makefile
> index 753129029ed9..1a2129d20655 100644
> --- a/tools/tests/domid/Makefile
> +++ b/tools/tests/domid/Makefile
> @@ -14,16 +14,18 @@ $(shell sed -n \
>      's/^[ \t]*# *include[ \t]*[<"]\([^">]*\)[">].*/\1/p' $(1) 2>/dev/null)
>  endef
>  
> -# NB: $1 cannot be a list
> +# $1 target
> +# $2 list of test harnesses

... in this comment. According to ...

>  define emit-harness-nested-rule
> -$(1): $(CURDIR)/harness.h
> -	mkdir -p $$(@D);
> -	ln -sf $$< $$@;
> +$(1): $(2)

... the use of the parameter, this is the list of dependencies.

And then, how does this make any difference at all when ...

> +	mkdir -p $$(@D); \
> +	for i in $$<; do ln -sf $$$$i $$@; done
>  
>  endef
>  
>  define emit-harness-rules
> -$(foreach x,$(2),$(call emit-harness-nested-rule,$(CURDIR)/generated/$(x)))
> +$(foreach x,$(2),$(call \
> +    emit-harness-nested-rule,$(CURDIR)/generated/$(x),$(CURDIR)/harness.h))

... you still hardcode the exact same file here?

As an aside, imo this would better be wrapped as

$(foreach x,$(2),$(call emit-harness-nested-rule, \
                        $(CURDIR)/generated/$(x),$(CURDIR)/harness.h))

or even

$(foreach x,$(2),$(call emit-harness-nested-rule, \
                        $(CURDIR)/generated/$(x), \
                        $(CURDIR)/harness.h))

Jan

Re: [PATCH v1 1/2] tests: fixup domid test harness dependencies

[Anthony PERARD]

On Thu, Dec 04, 2025 at 04:37:11AM -0800, dmukhin@xen.org wrote:
> diff --git a/tools/tests/domid/Makefile b/tools/tests/domid/Makefile
> index 753129029ed9..1a2129d20655 100644
> --- a/tools/tests/domid/Makefile
> +++ b/tools/tests/domid/Makefile
> @@ -14,16 +14,18 @@ $(shell sed -n \
>      's/^[ \t]*# *include[ \t]*[<"]\([^">]*\)[">].*/\1/p' $(1) 2>/dev/null)
>  endef
>  
> -# NB: $1 cannot be a list
> +# $1 target
> +# $2 list of test harnesses
>  define emit-harness-nested-rule
> -$(1): $(CURDIR)/harness.h
> -	mkdir -p $$(@D);
> -	ln -sf $$< $$@;
> +$(1): $(2)
> +	mkdir -p $$(@D); \
> +	for i in $$<; do ln -sf $$$$i $$@; done

This doesn't work.

First, on the first line, the introduction of the backslash mean that
error from `mkdir` are ignored, Make will execute both line in the same
shell instead of 2 separate shell. You would need to add `set -e` if
executing all lines of the recipe in the same shell is useful.

Second, $< only refer to the first prerequisite. So only a single
symlink is created, even if $(2) list multiple files.

Third, if we fix to loop through all the dependencies, the loop will
still only create a single symlink, the last iteration will overwrite
the previous one.

Thanks,

-- 
Anthony PERARD

[PATCH v1 2/2] tests: introduce Tests.mk fragment

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com> 

Add new make fragment for unit tests with auto-generated test harness
dependencies.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
 tools/Tests.mk             | 34 ++++++++++++++++++++++++++++++++++
 tools/tests/domid/Makefile | 30 +-----------------------------
 2 files changed, 35 insertions(+), 29 deletions(-)
 create mode 100644 tools/Tests.mk

diff --git a/tools/Tests.mk b/tools/Tests.mk
new file mode 100644
index 000000000000..fe941209db0f
--- /dev/null
+++ b/tools/Tests.mk
@@ -0,0 +1,34 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Unit test fragment.
+#
+# Copyright 2025 Ford Motor Company
+
+define list-c-headers
+$(shell sed -n \
+    's/^[ \t]*# *include[ \t]*[<"]\([^">]*\)[">].*/\1/p' $(1) 2>/dev/null)
+endef
+
+# $1 target
+# $2 list of test harnesses
+define emit-harness-nested-rule
+$(1): $(2)
+	mkdir -p $$(@D); \
+	for i in $$<; do ln -sf $$$$i $$@; done
+
+endef
+
+define emit-harness-rules
+$(foreach x,$(2),$(call \
+    emit-harness-nested-rule,$(CURDIR)/generated/$(x),$(CURDIR)/harness.h))
+$(1:.c=.o): $(addprefix $(CURDIR)/generated/,$(2))
+endef
+
+define emit-harness-deps
+$(if $(strip $(2)),$(call emit-harness-rules,$1,$2),)
+endef
+
+define vpath-with-harness-deps
+vpath $(1) $(2)
+$(call emit-harness-deps,$(1),$(call list-c-headers,$(2)$(1)))
+endef
diff --git a/tools/tests/domid/Makefile b/tools/tests/domid/Makefile
index 1a2129d20655..51a465ce6318 100644
--- a/tools/tests/domid/Makefile
+++ b/tools/tests/domid/Makefile
@@ -6,38 +6,10 @@
 
 XEN_ROOT=$(CURDIR)/../../..
 include $(XEN_ROOT)/tools/Rules.mk
+include $(XEN_ROOT)/tools/Tests.mk
 
 TESTS := test-domid
 
-define list-c-headers
-$(shell sed -n \
-    's/^[ \t]*# *include[ \t]*[<"]\([^">]*\)[">].*/\1/p' $(1) 2>/dev/null)
-endef
-
-# $1 target
-# $2 list of test harnesses
-define emit-harness-nested-rule
-$(1): $(2)
-	mkdir -p $$(@D); \
-	for i in $$<; do ln -sf $$$$i $$@; done
-
-endef
-
-define emit-harness-rules
-$(foreach x,$(2),$(call \
-    emit-harness-nested-rule,$(CURDIR)/generated/$(x),$(CURDIR)/harness.h))
-$(1:.c=.o): $(addprefix $(CURDIR)/generated/,$(2))
-endef
-
-define emit-harness-deps
-$(if $(strip $(2)),$(call emit-harness-rules,$1,$2),)
-endef
-
-define vpath-with-harness-deps
-vpath $(1) $(2)
-$(call emit-harness-deps,$(1),$(call list-c-headers,$(2)$(1)))
-endef
-
 .PHONY: all
 all: $(TESTS)
 
-- 
2.52.0

Re: [PATCH v1 2/2] tests: introduce Tests.mk fragment

[Jan Beulich]

On 04.12.2025 13:37, dmukhin@xen.org wrote:
> From: Denis Mukhin <dmukhin@ford.com> 
> 
> Add new make fragment for unit tests with auto-generated test harness
> dependencies.

Why is this (going to be) useful? And what exactly does "auto-generated"
refer to here? Not ...

> --- /dev/null
> +++ b/tools/Tests.mk
> @@ -0,0 +1,34 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# Unit test fragment.
> +#
> +# Copyright 2025 Ford Motor Company
> +
> +define list-c-headers
> +$(shell sed -n \
> +    's/^[ \t]*# *include[ \t]*[<"]\([^">]*\)[">].*/\1/p' $(1) 2>/dev/null)
> +endef

... the use of this construct, I suppose? When we talk of auto-generated dependencies,
we normally mean what the compiler can be told to produce via -MMD. If you want such,
why would you introduce (and now generalize) a fragile, custom alternative? (Fragile
because afaict transitively included headers wouldn't be accounted for.

Jan

Re: [PATCH v1 0/2] tools/tests: test harness fragment

[Anthony PERARD]

On Thu, Dec 04, 2025 at 04:37:10AM -0800, dmukhin@xen.org wrote:
> This is a follow-on mini-series based on the original feedback in [1].
> 
> Patch 1 addresses the remaining feedback from [1].

You mean the feedback about the comment? I guess that's addressed by
removing the comment, there's just more unrelated changes in the patch.

> Patch 2 adds a new fragment for auto-generating test harness dependencies.

I feel like this second patch could be made useful, and easier to
review, if a second user of Tests.mk was added.

> [1] https://lore.kernel.org/xen-devel/aLmZLm2_G48yfPWR@l14/
> [2] CI: https://gitlab.com/xen-project/people/dmukhin/xen/-/pipelines/2195624771
> 
> Denis Mukhin (2):
>   tests: fixup domid test harness dependencies
>   tests: introduce Tests.mk fragment
> 
>  tools/Tests.mk             | 34 ++++++++++++++++++++++++++++++++++

If this new file is useful, could you move it "tools/tests/"?

BTW, if we really want some generic test harness makefile, maybe looking
at what was done for "tools/libs/libs.mk" for example.

Cheers,

-- 
Anthony PERARD