Patrick Georgi has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/41771 )
Change subject: tests: Allow emitting junit output for unit tests ......................................................................
tests: Allow emitting junit output for unit tests
Change-Id: Iab0c4250b1baa77d4eab7538ec1fd3310f9e63e4 Signed-off-by: Patrick Georgi pgeorgi@google.com --- M tests/Makefile.inc 1 file changed, 6 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/41771/1
diff --git a/tests/Makefile.inc b/tests/Makefile.inc index f84fa30..2da29c3 100644 --- a/tests/Makefile.inc +++ b/tests/Makefile.inc @@ -135,7 +135,13 @@ .PHONY: $(alltests) $(addprefix clean-,$(alltests)) .PHONY: unit-tests build-unit-tests run-unit-tests clean-unit-tests
+ifeq ($(JUNIT_OUTPUT),y) +$(alltests): export CMOCKA_MESSAGE_OUTPUT=xml +$(alltests): export CMOCKA_XML_FILE=$(testobj)/junit-$(subst /,_,$^).xml +endif + $(alltests): $$($$(@)-bin) + rm -f $(testobj)/junit-$(subst /,_,$^).xml ./$^
unit-tests: build-unit-tests run-unit-tests
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41771 )
Change subject: tests: Allow emitting junit output for unit tests ......................................................................
Patch Set 1: Code-Review+1
Jan Dabros has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41771 )
Change subject: tests: Allow emitting junit output for unit tests ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41771/1/tests/Makefile.inc File tests/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/41771/1/tests/Makefile.inc@140 PS1, Line 140: $(alltests): export CMOCKA_XML_FILE=$(testobj)/junit-$(subst /,_,$^).xml That's nice. Wasn't aware that we can use $^ (automatic variables in general) in per-target variable definition. Is this actually expanded during the execution of recipe?
https://review.coreboot.org/c/coreboot/+/41771/1/tests/Makefile.inc@144 PS1, Line 144: rm -f $(testobj)/junit-$(subst /,_,$^).xml When checkout on this commit, we may end up with some stale xmls saying that test passed, even though it was not executed (due to failure of some test earlier in the queue). You are changing this behavior in 4/4 - maybe we should change the order of commits?
Anyway, why not to remove all xmls with "make clean-unit-tests" and force "what-jenkins-does" to issue "clean-unit-tests" before "make unit-tests"?
Jan Dabros has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41771 )
Change subject: tests: Allow emitting junit output for unit tests ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41771/1/tests/Makefile.inc File tests/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/41771/1/tests/Makefile.inc@144 PS1, Line 144: rm -f $(testobj)/junit-$(subst /,_,$^).xml
When checkout on this commit, we may end up with some stale xmls saying that test passed, even thoug […]
Hmm.. ok - I can answer my own question. Probably we want to reduce time spent on this :) Yes, so I think what you propose here is the best option.
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41771 )
Change subject: tests: Allow emitting junit output for unit tests ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/41771/1/tests/Makefile.inc File tests/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/41771/1/tests/Makefile.inc@140 PS1, Line 140: $(alltests): export CMOCKA_XML_FILE=$(testobj)/junit-$(subst /,_,$^).xml
That's nice. […]
Yes, it is.
https://review.coreboot.org/c/coreboot/+/41771/1/tests/Makefile.inc@144 PS1, Line 144: rm -f $(testobj)/junit-$(subst /,_,$^).xml
Hmm.. ok - I can answer my own question. […]
I think the main issue is stale xml files when removing or renaming tests. That's a temporary issue best solved with cleaning the tree manually (as is required for similar changes to source code elsewhere, too).
Adding special handling to what-jenkins-does means that this would surprise users running tests locally. For some twisted reason cmocka's semantics are to write to the file in CMOCKA_XML_FILE _only_ if the file doesn't exist yet and can be created. That's neat, I guess, for data preservation, but it's very unlike the behavior of most other tools.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41771 )
Change subject: tests: Allow emitting junit output for unit tests ......................................................................
Patch Set 1: Code-Review+2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41771 )
Change subject: tests: Allow emitting junit output for unit tests ......................................................................
Patch Set 1: Code-Review+2
Jan Dabros has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41771 )
Change subject: tests: Allow emitting junit output for unit tests ......................................................................
Patch Set 1: Code-Review+1
(2 comments)
https://review.coreboot.org/c/coreboot/+/41771/1/tests/Makefile.inc File tests/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/41771/1/tests/Makefile.inc@140 PS1, Line 140: $(alltests): export CMOCKA_XML_FILE=$(testobj)/junit-$(subst /,_,$^).xml
Yes, it is.
Done
https://review.coreboot.org/c/coreboot/+/41771/1/tests/Makefile.inc@144 PS1, Line 144: rm -f $(testobj)/junit-$(subst /,_,$^).xml
I think the main issue is stale xml files when removing or renaming tests. […]
OK, thanks for explanation.
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/41771 )
Change subject: tests: Allow emitting junit output for unit tests ......................................................................
tests: Allow emitting junit output for unit tests
Change-Id: Iab0c4250b1baa77d4eab7538ec1fd3310f9e63e4 Signed-off-by: Patrick Georgi pgeorgi@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/41771 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Martin Roth martinroth@google.com Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Jan Dabros jsd@semihalf.com --- M tests/Makefile.inc 1 file changed, 6 insertions(+), 0 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 f84fa30..2da29c3 100644 --- a/tests/Makefile.inc +++ b/tests/Makefile.inc @@ -135,7 +135,13 @@ .PHONY: $(alltests) $(addprefix clean-,$(alltests)) .PHONY: unit-tests build-unit-tests run-unit-tests clean-unit-tests
+ifeq ($(JUNIT_OUTPUT),y) +$(alltests): export CMOCKA_MESSAGE_OUTPUT=xml +$(alltests): export CMOCKA_XML_FILE=$(testobj)/junit-$(subst /,_,$^).xml +endif + $(alltests): $$($$(@)-bin) + rm -f $(testobj)/junit-$(subst /,_,$^).xml ./$^
unit-tests: build-unit-tests run-unit-tests
-
Angel Pons (Code Review) -
Jan Dabros (Code Review)
-
Martin Roth (Code Review)
-
Patrick Georgi (Code Review)