RFR(S) : 8252522 : nsk/share/test/StressOptions should multiple stressTime by jtreg's timeout-factor

[David Holmes]

Hi Igor,

On 7/09/2020 2:20 am, Igor Ignatyev wrote:
> 
> 
>> On Sep 6, 2020, at 6:32 AM, David Holmes <david.holmes at oracle.com 
>> <mailto:david.holmes at oracle.com>> wrote:
>>
>> Hi Igor,
>>
>> On 5/09/2020 6:22 am, Igor Ignatyev wrote:
>>> Hi David,
>>>> On Aug 30, 2020, at 9:24 PM, David Holmes <david.holmes at oracle.com 
>>>> <mailto:david.holmes at oracle.com><mailto:david.holmes at oracle.com>> wrote:
>>>>
>>>> Hi Igor,
>>>>
>>>> On 29/08/2020 5:32 am, Igor Ignatyev wrote:
>>>>> http://cr.openjdk.java.net/~iignatyev/8252522/webrev.01/
>>>>>> 118 lines changed: 32 ins; 36 del; 50 mod;
>>>>> Hi all,
>>>>> could you please review this small patch which updates 
>>>>> StressOptions to adjust allocated time according to TIMEOUT_FACTOR?
>>>>> from JBS:
>>>>>> nsk/share/test/StressOptions and Stresser aren't aware of jtreg's 
>>>>>> timeout-factor and hence don't provide enough stress time for 
>>>>>> testing in slow/stress configurations, e.g. Xcomp.
>>>>> the patch also includes small clean up and refactoring, such as 
>>>>> removal of unused c-tor and using switch instead if-elif. webrev 
>>>>> w/o these changes -- 
>>>>> http://cr.openjdk.java.net/~iignatyev/8252522/webrev.00/
>>>>
>>>> Both webrevs seem to be the same.
>>>>
>>> not sure how that happened.
>>>> But the changes seem fine.
>>> thanks for your review, the testing showed that tests which use 
>>> vmTestbase/nsk/share/jvmti/Injector started to fail w/ this patch b/c 
>>> of the switch statement being introduced in StressOptions. so I've 
>>> decided to revert that part of refactoring:
>>>  - http://cr.openjdk.java.net/~iignatyev/8252522/webrev.1-2 (diff)
>>>  - http://cr.openjdk.java.net/~iignatyev/8252522/webrev.02 (full)
>>
>> Reversion seems fine, but I don't understand why the change would have 
>> caused failures?
> frankly speaking, neither do I. I'll file an issue and look into it later.
>>
>> I cause this will need a PR now.
> yup, I've already created and integrated the PR.

Not quite the way I expected it to happen. I had assumed the PR would 
start things from scratch and I would indicate my review as part of the 
PR. :) I see you found a way to short-circuit the cutover process.

Cheers,
David
-----

>>
>> Cheers,
>> David
>>
>>> just to be on the safe side, I'm rerunning the testing.
>>> -- Igor
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>> webrev:http://cr.openjdk.java.net/~iignatyev/8252522/webrev.01/
>>>>> JBS:https://bugs.openjdk.java.net/browse/JDK-8252522
>>>>> testing: vmTestbase tests (in progress)
>>>>> Thanks,
>>>>> -- Igor
>