| Message ID | 87a5g2gnej.fsf@oldenburg.str.redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] Implement run-built-tests=no for make xcheck, always build xtests | expand |
On 9/20/24 3:01 PM, Florian Weimer wrote: > Previously, the second occurrence of the xtests target > expected all xtests to run (as the result of specifying > $(xtests)), but these tests have not been run due to > the the first xtests target is set up for run-built-tests=no: > it only runs tests in $(xtests-special). Consequently, > xtests are reported as UNSUPPORTED with “make xcheck > run-built-tests=no”. The xtests were not built, either. > > After this change always, xtests are built regardless > of the $(run-built-tests) variable (except for xtests listed > in $(tests-unsupported)). To fix the UNSUPPORTED issue, > introduce xtests-expected and use that manage test > expectations in the second xtests target. This looks really good. Thank you for taking some of the suggestions and improving the patch. Reviewed-by: Carlos O'Donell <carlos@redhat.com> > > --- > v2: Slightly different approach. Also build xtests unconditionally. > Rules | 17 +++++++++++------ > 1 file changed, 11 insertions(+), 6 deletions(-) > > diff --git a/Rules b/Rules > index 27846abf82..713c225d2e 100644 > --- a/Rules > +++ b/Rules > @@ -143,8 +143,9 @@ endif > others: $(py-const) > > ifeq ($(run-built-tests),no) > +# The $(xtests) dependency ensures that xtests are always built. > tests: $(addprefix $(objpfx),$(filter-out $(tests-unsupported), \ > - $(tests) $(tests-internal) \ > + $(tests) $(tests-internal) $(xtests) \ OK. Solid improvement to ensure the tests keep building. By definition we don't know how to "build" xtests-special, so this is the best we can do. > $(tests-container) \ > $(tests-mcheck:%=%-mcheck) \ > $(tests-malloc-check:%=%-malloc-check) \ > @@ -153,8 +154,10 @@ tests: $(addprefix $(objpfx),$(filter-out $(tests-unsupported), \ > $(test-srcs)) $(tests-special) \ > $(tests-printers-programs) > xtests: tests $(xtests-special) > -else > +else # $(run-built-tests) != no > +# The $(xtests) dependency ensures that xtests are always built. > tests: $(tests:%=$(objpfx)%.out) $(tests-internal:%=$(objpfx)%.out) \ > + $(addprefix $(objpfx),$(filter-out $(tests-unsupported), $(xtests))) \ OK. Again ensures we build them but filter them. > $(tests-container:%=$(objpfx)%.out) \ > $(tests-mcheck:%=$(objpfx)%-mcheck.out) \ > $(tests-malloc-check:%=$(objpfx)%-malloc-check.out) \ > @@ -162,26 +165,28 @@ tests: $(tests:%=$(objpfx)%.out) $(tests-internal:%=$(objpfx)%.out) \ > $(tests-malloc-hugetlb2:%=$(objpfx)%-malloc-hugetlb2.out) \ > $(tests-special) $(tests-printers-out) > xtests: tests $(xtests:%=$(objpfx)%.out) $(xtests-special) > -endif > +endif # $(run-built-tests) != no OK. > > tests-special-notdir = $(patsubst $(objpfx)%, %, $(tests-special)) > xtests-special-notdir = $(patsubst $(objpfx)%, %, $(xtests-special)) > ifeq ($(run-built-tests),no) > tests-expected = > -else > +xtests-expected = OK. Nice to mirror the same setup. > +else # $(run-built-tests) != no > tests-expected = $(tests) $(tests-internal) $(tests-printers) \ > $(tests-container) $(tests-malloc-check:%=%-malloc-check) \ > $(tests-malloc-hugetlb1:%=%-malloc-hugetlb1) \ > $(tests-malloc-hugetlb2:%=%-malloc-hugetlb2) \ > $(tests-mcheck:%=%-mcheck) > -endif > +xtests-expected = $(xtests) > +endif # $(run-built-tests) != no > tests: > $(..)scripts/merge-test-results.sh -s $(objpfx) $(subdir) \ > $(sort $(tests-expected) $(tests-special-notdir:.out=)) \ > > $(objpfx)subdir-tests.sum > xtests: > $(..)scripts/merge-test-results.sh -s $(objpfx) $(subdir) \ > - $(sort $(xtests) $(xtests-special-notdir:.out=)) \ > + $(sort $(xtests-expected) $(xtests-special-notdir:.out=)) \ OK. > > $(objpfx)subdir-xtests.sum > > ifeq ($(build-programs),yes) > > base-commit: e64a1e81aadf6c401174ac9471ced0f0125c2912 >
diff --git a/Rules b/Rules index 27846abf82..713c225d2e 100644 --- a/Rules +++ b/Rules @@ -143,8 +143,9 @@ endif others: $(py-const) ifeq ($(run-built-tests),no) +# The $(xtests) dependency ensures that xtests are always built. tests: $(addprefix $(objpfx),$(filter-out $(tests-unsupported), \ - $(tests) $(tests-internal) \ + $(tests) $(tests-internal) $(xtests) \ $(tests-container) \ $(tests-mcheck:%=%-mcheck) \ $(tests-malloc-check:%=%-malloc-check) \ @@ -153,8 +154,10 @@ tests: $(addprefix $(objpfx),$(filter-out $(tests-unsupported), \ $(test-srcs)) $(tests-special) \ $(tests-printers-programs) xtests: tests $(xtests-special) -else +else # $(run-built-tests) != no +# The $(xtests) dependency ensures that xtests are always built. tests: $(tests:%=$(objpfx)%.out) $(tests-internal:%=$(objpfx)%.out) \ + $(addprefix $(objpfx),$(filter-out $(tests-unsupported), $(xtests))) \ $(tests-container:%=$(objpfx)%.out) \ $(tests-mcheck:%=$(objpfx)%-mcheck.out) \ $(tests-malloc-check:%=$(objpfx)%-malloc-check.out) \ @@ -162,26 +165,28 @@ tests: $(tests:%=$(objpfx)%.out) $(tests-internal:%=$(objpfx)%.out) \ $(tests-malloc-hugetlb2:%=$(objpfx)%-malloc-hugetlb2.out) \ $(tests-special) $(tests-printers-out) xtests: tests $(xtests:%=$(objpfx)%.out) $(xtests-special) -endif +endif # $(run-built-tests) != no tests-special-notdir = $(patsubst $(objpfx)%, %, $(tests-special)) xtests-special-notdir = $(patsubst $(objpfx)%, %, $(xtests-special)) ifeq ($(run-built-tests),no) tests-expected = -else +xtests-expected = +else # $(run-built-tests) != no tests-expected = $(tests) $(tests-internal) $(tests-printers) \ $(tests-container) $(tests-malloc-check:%=%-malloc-check) \ $(tests-malloc-hugetlb1:%=%-malloc-hugetlb1) \ $(tests-malloc-hugetlb2:%=%-malloc-hugetlb2) \ $(tests-mcheck:%=%-mcheck) -endif +xtests-expected = $(xtests) +endif # $(run-built-tests) != no tests: $(..)scripts/merge-test-results.sh -s $(objpfx) $(subdir) \ $(sort $(tests-expected) $(tests-special-notdir:.out=)) \ > $(objpfx)subdir-tests.sum xtests: $(..)scripts/merge-test-results.sh -s $(objpfx) $(subdir) \ - $(sort $(xtests) $(xtests-special-notdir:.out=)) \ + $(sort $(xtests-expected) $(xtests-special-notdir:.out=)) \ > $(objpfx)subdir-xtests.sum ifeq ($(build-programs),yes)