RFR(S/T) : 8219158 : use 'test.root' property instead of traversing test-src path
Igor Ignatyev
igor.ignatyev at oracle.com
Thu Feb 21 18:25:46 UTC 2019
> On Feb 21, 2019, at 1:20 AM, David Holmes <david.holmes at oracle.com> wrote:
>
> On 21/02/2019 5:49 am, Igor Ignatyev wrote:
>> David,
>> I gave it a second though, and as this patch changes jdk/test/lib/Utils class, which potentially can be used by any of our test suites (and is already used by jdk and jaxp), it's safer to updated all TEST.ROOT files. this, as Alas mentioned in 8219417 RFR thread, requires some socialization and coordination. that's to say, I will hold pushing 8219158, till we get 8219417 integrated and/or come to another consensus.
>
> The change to the Utils class doesn't affect any tests that don't try to use TEST.ROOT. The Utils class itself is not dependent on the jtreg version as if the property does not exist it just returns an empty string.
David,
that's true, but my point was different. let's say someone is adding new tests which use this static field in jaxp test suite and we haven't updated requiredVersion there. so the person which adds new tests, tested them using jib, which means using jtreg4.2-b14, and the tests work as expected, and there is nothing in Utils which say "this property won't be available unless jtreg >= b14" (and I don't think we need such comment there), so this person doesn't have any reasons to update TEST.ROOT in jaxp test suite. however, next time someone who doesn't use jib or jtreg4.2-b14 runs jaxp test suites, all these tests fail. and, from my point of view, this case isn't much different from one you mentioned in this thread. therefore, I believe it'll be much safer/better if we update all TEST.ROOT files before we start to use any features from 4.2-b14 in shared classes.
Thanks,
-- Igor
> Cheers,
> David
>
>> Cheers,
>> -- Igor
>>> On Feb 20, 2019, at 12:41 AM, David Holmes <david.holmes at oracle.com> wrote:
>>>
>>> On 20/02/2019 5:05 pm, Igor Ignatyev wrote:
>>>> Hi David,
>>>> Hi David,
>>>> Hi David,
>> I don't know how this russian tradition to do things three times crept into my email ;)
>>>> http://cr.openjdk.java.net/~iignatyev//8219158/webrev.0-1/index.html is the incremental webrev w/ bumping requiredVersion only in hotspot.
>>>
>>> Looks good.
>>>
>>> Thanks,
>>> David
>>>
>>>> -- Igor
>>>>> On Feb 19, 2019, at 10:48 PM, David Holmes <david.holmes at oracle.com <mailto:david.holmes at oracle.com>> wrote:
>>>>>
>>>>> Hi Igor,
>>>>>
>>>>> On 20/02/2019 4:24 pm, Igor Ignatyev wrote:
>>>>>> Hi David,
>>>>>> you are right, I've filed 8219417 to do that in all test suites and sent RFR to jdk-dev alias.
>>>>>
>>>>> I responded to that RFR and I don't think it's the right approach. This bug should simply update the requiresVersion for the hotspot tests.
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>> -- Igor
>>>>>>> On Feb 19, 2019, at 10:07 PM, David Holmes <david.holmes at oracle.com <mailto:david.holmes at oracle.com>> wrote:
>>>>>>>
>>>>>>> Hi Igor,
>>>>>>>
>>>>>>> On 20/02/2019 3:54 pm, Igor Ignatyev wrote:
>>>>>>>> http://cr.openjdk.java.net/~iignatyev//8219158/webrev.00/index.html
>>>>>>>>> 73 lines changed: 9 ins; 32 del; 32 mod;
>>>>>>>> Hi all,
>>>>>>>> could you please review this trivial clean-up? jtreg4.2-b14 added a new property 'test.root', this patches replaces all code in hotspot testbase which determines the root directory w/ simple reading of this property.
>>>>>>>
>>>>>>> You will have to update the jtreg requiresVersion to b14 to use this, else it will fail for anyone not using b14.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> David
>>>>>>> ----
>>>>>>>
>>>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8219158
>>>>>>>> webrev: http://cr.openjdk.java.net/~iignatyev//8219158/webrev.00/index.html
>>>>>>>> testing: vmTestbase/gc/g1/unloading, vmTestbase/nsk/monitoring/stress, vmTestbase/vm/mlvm/, vmTestbase/vm/runtime/defmeth
>>>>>>>> Thanks,
>>>>>>>> -- Igor
More information about the hotspot-dev
mailing list