I am ok with the recent changes

On 04.08.2016 00:31, David Holmes wrote:
On 4/08/2016 2:35 PM, Chris Plummer wrote:
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.

Must be an old sh that uses a [ built-in which doesn't alias == to =.

Changes look fine.

Thanks,
David

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