Unfortunately I need another review of these changes. I ran into a minor problem. Although all the jprt testing I tried passed, when I looked into the log files I noticed an issue. I saw the following:

Test results: passed: 1
Report written to /opt/jprt/T/P1/003906.cplummer/s/hotspot/testoutput/JTreport/html/report.html Results written to /opt/jprt/T/P1/003906.cplummer/s/hotspot/testoutput/JTwork
/bin/sh: 15: [: 0: unexpected operator
Summary:
TEST STATS: name=  run=1  pass=1  fail=0
EXIT CODE: 0
EXIT CODE: 0

Notice the "unexpected operator" message in the middle. This only happens on our arm platforms. It doesn't seem to affect the test results, but should be cleaned up anyway. The "0" turns out to be the exit code. I see the same message if a test fails, except the 0 changes to a 2. Looking more closely at the code I added, I see I used ==, whereas other code nearby uses =. I change it to = and the error message went away. I retested with various test cases that fail, pass, or run no tests, and all of them do as expected still.

New webrev can be found at:

http://cr.openjdk.java.net/~cjplummer/8162670/webrev-02/

thanks,

Chris

On 8/2/16 7:50 AM, Chris Plummer wrote:
On 8/1/16 9:30 PM, Chris Plummer wrote:
On 8/1/16 7:25 PM, David Holmes wrote:
On 2/08/2016 12:11 PM, Chris Plummer wrote:
On 8/1/16 5:58 PM, David Holmes wrote:
Hi Chris,

On 2/08/2016 8:46 AM, Chris Plummer wrote:
Hello,

Please review this simple change:

https://bugs.openjdk.java.net/browse/JDK-8162670
http://cr.openjdk.java.net/~cjplummer/8162670/webrev-00/

You've split a compound expression with your code:

 227   jtregExitCode=$$? && \
 228   if [ $${jtregExitCode} == 1 ]; then \
 229     jtregExitCode=0; \
 230   fi ; \
 231   _summary="$(SUMMARY_TXT)"; \

I'm not clear exactly why the && was needed here but rather than find
out later I suggest rearranging the above to:

   jtregExitCode=$$? && \
   _summary="$(SUMMARY_TXT)"; \
   if [ $${jtregExitCode} == 1 ]; then \
     jtregExitCode=0; \
   fi ; \

Yeah, that makes sense. I'll make the change. However, it's really
unclear what the use case for && is here. How can jtregExitCode=$$? ever
fail?

I wonder if it evaluates to the $? value and so only sets _summary if we had a zero exit code? (Not that I understand why we would only set _summary in that context.)
I tried the following:

bash-4.1$ x=0 && echo "foo";

And "foo" is always printed. I also tried non-zero values for x. Here are some other examples:

bash-4.1$ (exit 1) && echo "foo"
bash-4.1$ (exit 0) && echo "foo"
foo

Commands evaluate to true if the exit status is 0, and false otherwise, so I guess you could say commands evaluate to !?$. For &&, the rhs is only executed if the lhs has a zero exit status.
Updated webrev:

http://cr.openjdk.java.net/~cjplummer/8162670/webrev-01/

thanks,

Chris

Chris

David

thanks,

Chris
Thanks,
David

Note the copyright dates haven't been updated in this webrev, but I did
update them locally after noticing that.

Tested with jprt test case given in the CR, and also with a jprt run
using "testset -hotspot" to make sure I didn't break anything.

thanks,

Chris





Reply via email to