RFR(XS): 8162670: make of jtreg_tests fails if no tests are run, causing jprt test runs to also fail

David Holmes david.holmes at oracle.com
Wed Aug 3 02:04:30 UTC 2016


Looks good.

Thanks,
David

On 3/08/2016 12: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
>>>>
>>>>
>>
>


More information about the core-libs-dev mailing list