Reviewer needed - fix for IcedTea bug#637
Dr Andrew John Hughes
ahughes at redhat.com
Tue Mar 8 08:48:39 PST 2011
On 10:22 Tue 08 Mar , Pavel Tisnovsky wrote:
> Mark Wielaard wrote:
> > Hi Pavel,
> >
> >
> >> hg diff generated against recent IcedTea6 HEAD is stored in attachment.
> >
> >> Addition to Changelog:
> >>
> >> 2011-03-03 Pavel Tisnovsky <ptisnovs at redhat.com>
> >>
> >> * Makefile.am:
> >> Added checking of JTreg results - if at least one
> >> regression test failed, make exits with error code
> >
> > Make sure to mention PR637 here and in the commit log.
> > Would be nice to explicitly mention the targets names you changed/added/
> >
> >> diff -r 0df8f7938769 Makefile.am
> >> --- a/Makefile.am Thu Mar 03 15:32:46 2011 +0100
> >> +++ b/Makefile.am Thu Mar 03 16:59:31 2011 +0100
> >> @@ -2069,6 +2069,7 @@
> >> $(ICEDTEA_JTREG_OPTIONS) \
> >> `pwd`/openjdk/hotspot/test \
> >> | tee test/$@.log
> >> + test -z `grep "^Error:\|^FAILED:" test/check-hotspot.log`
> >
> > Since above the log file is created through the usage of $@.log please
> > use that in the test also. $@ expands to the target name (check-hotspot
> > in this case).
> >
> >> check-langtools: stamps/jtreg.stamp
> >> mkdir -p test/langtools/JTwork test/langtools/JTreport
> >> @@ -2079,6 +2080,7 @@
> >> $(ICEDTEA_JTREG_OPTIONS) \
> >> `pwd`/openjdk/langtools/test \
> >> | tee test/$@.log
> >> + test -z `grep "^Error:\|^FAILED:" test/check-langtools.log`
> >
> > Likewise.
> >
> >> check-jdk: stamps/jtreg.stamp
> >> mkdir -p test/jdk/JTwork test/jdk/JTreport
> >> @@ -2095,6 +2097,7 @@
> >> $(ICEDTEA_JTREG_OPTIONS) \
> >> `pwd`/openjdk/jdk/test \
> >> | tee test/$@.log
> >> + test -z `grep "^Error:\|^FAILED:" test/check-jdk.log`
> >
> > Likewise.
> >
> >> clean-jtreg-reports:
> >> rm -rf test/hotspot test/langtools test/jdk
> >> @@ -2107,7 +2110,9 @@
> >> jtreg_pids = ps x --no-headers -ww -o pid,ppid,args \
> >> | awk '$$2 == 1 && $$3 ~ /^$(subst /,\/,$(CURDIR)/$(sdkimg))/ {print $$1}'
> >>
> >> -jtregcheck: jtreg check-hotspot check-langtools check-jdk
> >> +jtregcheck: jtreg check-hotspot check-langtools check-jdk finish-jtregcheck
> >> +
> >> +finish-jtregcheck:
> >> for i in hotspot langtools jdk; do \
> >> echo "--------------- jtreg console summary for $$i ---------------"; \
> >> egrep -v '^(Passed:|Directory|Re[a-z]+\ written\ to)' test/check-$$i.log; \
> >
> > Maybe a nicer target name would be jtregcheck-summary.
> >
> > You have to handle the case that the check-$$i.log files might not exist
> > (since you cannot depend on them). Either just touch them (easiest
> > solution) or explicitly look whether they exist inside the for loop and
> > not output a summary line for them if not.
> >
> > Also since this in a way a fake target (you want it to be executed
> > always and it doesn't create the target as file) you should add it as
> > a .PHONY target dependency below (see the chapter on Phony Targets in
> > the gnu make manual).
> >
>
> Hi Mark,
>
> can you please look at the second version of this fix? It's stored in
> attachment as hg diff against recent IcedTea6 HEAD.
>
> Fixes:
>
> 1) test names uses $@ macro instead of its full name
> 2) new target is named jtregcheck-summary
> 3) this target is specified as .PHONY
> 4) files check-$$i.log are now touched in a loop where result file is
> generated
> 5) added NEWS entry to ChangeLog
>
Comments inline.
> ChangeLog entry:
>
> 2011-03-08 Pavel Tisnovsky <ptisnovs at redhat.com>
>
> * Makefile.am:
> Added checking of JTreg results - if at least one
> regression test failed, make exits with error code
> * NEWS: updated, added PR637 to the list of bug fixes.
>
>
>
> Cheers
> Pavel
> diff -r 97a8f2681254 Makefile.am
> --- a/Makefile.am Mon Mar 07 15:17:27 2011 -0500
> +++ b/Makefile.am Tue Mar 08 10:13:03 2011 +0100
> @@ -625,7 +625,8 @@
> clean-rewriter clean-rewrite-rhino clean-add-systemtap clean-add-systemtap-debug \
> clean-add-pulseaudio clean-add-pulseaudio-debug clean-add-nss clean-add-nss-debug \
> clean-add-tzdata-support clean-add-tzdata-support-debug clean-add-systemtap-ecj \
> - clean-add-pulseaudio-ecj clean-add-nss-ecj clean-add-tzdata-support-ecj clean-fonts
> + clean-add-pulseaudio-ecj clean-add-nss-ecj clean-add-tzdata-support-ecj clean-fonts \
> + jtregcheck-summary
>
> env:
> @echo 'unset JAVA_HOME'
> @@ -2072,6 +2073,7 @@
> $(ICEDTEA_JTREG_OPTIONS) \
> `pwd`/openjdk/hotspot/test \
> | tee test/$@.log
> + test -z `grep "^Error:\|^FAILED:" test/$@.log`
>
> check-langtools: stamps/jtreg.stamp
> mkdir -p test/langtools/JTwork test/langtools/JTreport
> @@ -2082,6 +2084,7 @@
> $(ICEDTEA_JTREG_OPTIONS) \
> `pwd`/openjdk/langtools/test \
> | tee test/$@.log
> + test -z `grep "^Error:\|^FAILED:" test/$@.log`
>
> check-jdk: stamps/jtreg.stamp
> mkdir -p test/jdk/JTwork test/jdk/JTreport
> @@ -2098,6 +2101,7 @@
> $(ICEDTEA_JTREG_OPTIONS) \
> `pwd`/openjdk/jdk/test \
> | tee test/$@.log
> + test -z `grep "^Error:\|^FAILED:" test/$@.log`
>
> clean-jtreg-reports:
> rm -rf test/hotspot test/langtools test/jdk
> @@ -2112,8 +2116,11 @@
>
> jtreg_checks = hotspot langtools jdk
>
> -jtregcheck: jtreg $(jtreg_checks:%=check-%)
> +jtregcheck: jtreg $(jtreg_checks:%=check-%) jtregcheck-summary
> +
> +jtregcheck-summary:
This is still wrong and hasn't been fixed.
> for i in hotspot langtools jdk; do \
> + touch test/check-$$i.log; \
> echo "--------------- jtreg console summary for $$i ---------------"; \
> egrep -v '^(Passed:|Directory|Re[a-z]+\ written\ to)' test/check-$$i.log; \
> done | tee test/jtreg-summary.log
> diff -r 97a8f2681254 NEWS
> --- a/NEWS Mon Mar 07 15:17:27 2011 -0500
> +++ b/NEWS Tue Mar 08 10:13:03 2011 +0100
> @@ -12,6 +12,10 @@
>
> * Use HotSpot 20 as the default virtual machine.
>
> +* Bug fixes
> + - PR637: make check should exit with an error code if any of regression test failed.
> + Use make check -k if you want to run all three test suites.
> +
There shouldn't be this blank line before Bug fixes.
> New in release 1.10 (2011-XX-XX):
>
> * NetX and the plugin moved to the IcedTea-Web project with a separate
--
Andrew :)
Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
Support Free Java!
Contribute to GNU Classpath and IcedTea
http://www.gnu.org/software/classpath
http://icedtea.classpath.org
PGP Key: F5862A37 (https://keys.indymedia.org/)
Fingerprint = EA30 D855 D50F 90CD F54D 0698 0713 C3ED F586 2A37
More information about the distro-pkg-dev
mailing list