Hi David,

thanks for your review, pushed to jdk15.

-- Igor

> On Jul 14, 2020, at 9:01 PM, David Holmes <[email protected]> wrote:
> 
> Hi Igor,
> 
> Looks good to me.
> 
> On 15/07/2020 9:29 am, Igor Ignatyev wrote:
>> http://cr.openjdk.java.net/~iignatyev/8249040/webrev.00/
>>> 135 lines changed: 2 ins; 63 del; 70 mod;
>> Hi all,
>> could you please review the clean-up of nsk_jdb tests?
>> from main issue(8204985) :
>>> all vmTestbase tests have '@run driver jdk.test.lib.FileInstaller . .' to 
>>> mimic old test harness behavior and copy all files from a test source 
>>> directory to a current work directory. some tests depend on this step, so 
>>> we need 1st identify such tests and then either rewrite them not to have 
>>> this dependency or leave FileInstaller only in these tests.
>> the patch removes FileInstaller actions in all the tests. as in all previous 
>> patches, the biggest (and tedious) part of the patch is just `ag -l  '@run 
>> driver jdk.test.lib.FileInstaller . .' $DIR  | xargs -I{} gsed -i '/@run 
>> driver jdk.test.lib.FileInstaller \. \./d' {}` with $DIR being 
>> test/hotspot/jtreg/vmTestbase/nsk/jdb/; two tests however required manual 
>> intervention: nsk/jdb/read/read001 and use001/use001 were misusing 
>> `-workdir` option. these tests expected this option to point to the 
>> directory w/ test's source code. 'workdir' option is processed by 
>> nsk/share/jdb/JdbArgumentHandler and is meant to store "full path to current 
>> test directory", which obviously doesn't have to be in the source tree. the 
>> tests were updated to use `test.src` property value instead of `workdir`.
> 
> All the commented out stuff in use001 is a little confusing, but not due to 
> this change of course.
true, yet that's not the most awful code you can find in vmTestbase ;) I'll let 
it for svc team to decide if/when/how they want to clean this up.
> 
> 
> Thanks,
> David
> 
>> testing: :vmTestbase_nsk_jdb on linux-x64
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8249040 
>> <https://bugs.openjdk.java.net/browse/JDK-8249040>
>> webrev: http://cr.openjdk.java.net/~iignatyev/8249040/webrev.00/ 
>> <http://cr.openjdk.java.net/~iignatyev/8249040/webrev.00/>
>> Thanks,
>> -- Igor

Reply via email to