Reviewer needed - fix for IcedTea bug#637

[Pavel Tisnovsky]

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

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
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: hg_diff
Url: http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20110308/6a61581c/hg_diff.ksh