Patrick Georgi has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41773 )
Change subject: tests: Always run all unit tests ......................................................................
tests: Always run all unit tests
So far, the semantics have been that run-unit-tests stopped at the first test suite that failed. This hides useful signal in later tests, so always run all tests and collect the result.
Change-Id: I407715f85513c2c95a1cf89cfb427317dff9fbab Signed-off-by: Patrick Georgi pgeorgi@google.com --- M tests/Makefile.inc 1 file changed, 13 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/41773/1
diff --git a/tests/Makefile.inc b/tests/Makefile.inc index 2da29c3..bfd1806 100644 --- a/tests/Makefile.inc +++ b/tests/Makefile.inc @@ -141,17 +141,25 @@ endif
$(alltests): $$($$(@)-bin) - rm -f $(testobj)/junit-$(subst /,_,$^).xml - ./$^ + rm -f $(testobj)/junit-$(subst /,_,$^).xml $(testobj)/$(subst /,_,$^).failed + -./$^ || echo failed > $(testobj)/$(subst /,_,$^).failed
unit-tests: build-unit-tests run-unit-tests
build-unit-tests: $(test-bins)
run-unit-tests: $(alltests) - echo "**********************" - echo " ALL TESTS PASSED" - echo "**********************" + if [ `find $(testobj) -name '*.failed' | wc -l` -gt 0 ]; then \ + echo "**********************"; \ + echo " TESTS FAILED"; \ + echo "**********************"; \ + exit 1; \ + else \ + echo "**********************"; \ + echo " ALL TESTS PASSED"; \ + echo "**********************"; \ + exit 0; \ + fi
$(addprefix clean-,$(alltests)): clean-%: rm -rf $(obj)/$*
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41773 )
Change subject: tests: Always run all unit tests ......................................................................
Patch Set 1:
Jan, I'm most unsure of this change: is there any dependency assumed between tests that would require us not to run "later" tests if "earlier" tests failed? Because that's what would change here. On the other hand, make is free to reorder dependencies and run them in parallel, so I don't think we could have relied on that in the first place.
Anyway, something I'd like your input on.
Jan Dabros has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41773 )
Change subject: tests: Always run all unit tests ......................................................................
Patch Set 1: Code-Review+1
Patch Set 1:
Jan, I'm most unsure of this change: is there any dependency assumed between tests that would require us not to run "later" tests if "earlier" tests failed? Because that's what would change here. On the other hand, make is free to reorder dependencies and run them in parallel, so I don't think we could have relied on that in the first place.
Anyway, something I'd like your input on.
No, the goal is to have completely isolated tests. Actually we will get similar effect when running "make unit-tests -j". Looks good to me.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41773 )
Change subject: tests: Always run all unit tests ......................................................................
Patch Set 1: Code-Review+2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41773 )
Change subject: tests: Always run all unit tests ......................................................................
Patch Set 1: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/41773 )
Change subject: tests: Always run all unit tests ......................................................................
tests: Always run all unit tests
So far, the semantics have been that run-unit-tests stopped at the first test suite that failed. This hides useful signal in later tests, so always run all tests and collect the result.
Change-Id: I407715f85513c2c95a1cf89cfb427317dff9fbab Signed-off-by: Patrick Georgi pgeorgi@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/41773 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Jan Dabros jsd@semihalf.com Reviewed-by: Martin Roth martinroth@google.com Reviewed-by: Angel Pons th3fanbus@gmail.com --- M tests/Makefile.inc 1 file changed, 13 insertions(+), 5 deletions(-)
Approvals: build bot (Jenkins): Verified Martin Roth: Looks good to me, approved Angel Pons: Looks good to me, approved Jan Dabros: Looks good to me, but someone else must approve
diff --git a/tests/Makefile.inc b/tests/Makefile.inc index 2da29c3..bfd1806 100644 --- a/tests/Makefile.inc +++ b/tests/Makefile.inc @@ -141,17 +141,25 @@ endif
$(alltests): $$($$(@)-bin) - rm -f $(testobj)/junit-$(subst /,_,$^).xml - ./$^ + rm -f $(testobj)/junit-$(subst /,_,$^).xml $(testobj)/$(subst /,_,$^).failed + -./$^ || echo failed > $(testobj)/$(subst /,_,$^).failed
unit-tests: build-unit-tests run-unit-tests
build-unit-tests: $(test-bins)
run-unit-tests: $(alltests) - echo "**********************" - echo " ALL TESTS PASSED" - echo "**********************" + if [ `find $(testobj) -name '*.failed' | wc -l` -gt 0 ]; then \ + echo "**********************"; \ + echo " TESTS FAILED"; \ + echo "**********************"; \ + exit 1; \ + else \ + echo "**********************"; \ + echo " ALL TESTS PASSED"; \ + echo "**********************"; \ + exit 0; \ + fi
$(addprefix clean-,$(alltests)): clean-%: rm -rf $(obj)/$*
-
Angel Pons (Code Review) -
Jan Dabros (Code Review)
-
Martin Roth (Code Review)
-
Patrick Georgi (Code Review)