On 05/15/2018 08:58 PM, Igor Ignatyev wrote:
Hi Erik,

please see my answers inline.

can I consider this RFR as reviewed by you?

Yep!

Thanks,
-- Igor

On May 14, 2018, at 4:23 AM, Erik Helin <erik.he...@oracle.com> wrote:

On 05/08/2018 12:35 AM, Igor Ignatyev wrote:
Hi all,

Hi Igor,

On 05/08/2018 12:35 AM, Igor Ignatyev wrote:
could you please review the patch which open sources GC tests from vm testbase? 
it introduces the following test groups:
- vmTestbase_vm_g1classunloading
- vmTestbase_vm_gc_compact
- vmTestbase_vm_gc_concurrent
- vmTestbase_vm_gc_container
- vmTestbase_vm_gc_juggle
- vmTestbase_vm_gc_locker
- vmTestbase_vm_gc_ref
- vmTestbase_vm_gc_misc

This is a very welcome, and pretty massive, change :) I won't be able to read 
through each test, there are simple too many, but I can sample a few of them 
and have a look.

On 05/08/2018 12:35 AM, Igor Ignatyev wrote:
As usually w/ VM testbase code, these tests are old, they have been run in 
hotspot testing for a long period of time. Originally, these tests were run by 
a test harness different from jtreg and had different build and execution 
schemes, some parts couldn't be easily translated to jtreg, so tests might have 
actions or pieces of code which look weird. In a long term, we are planning to 
rework them.

I'm also assuming that to help the open sourcing of these tests, most comments 
will likely be deferred until later? If so, that is fine with me.
y, unless it is something very important and cost of delaying changes is high, 
I'd prefer to defer all comments/improvements till later.

Ok, normally I would like it to be cleaned up prior to pushing, but I'm also aware of the history of these tests. Right now it seems more valuable to push these tests so we can collaborate on cleaning them up in upstream.


On 05/08/2018 12:35 AM, Igor Ignatyev wrote:
JBS: https://bugs.openjdk.java.net/browse/JDK-8199370
webrev: http://cr.openjdk.java.net/~iignatyev/8199370/webrev.00/index.html

Many of the tests are a bit cryptic, but that can be refactored later. Could 
you please file bugs for the two tests written in shell? Particularly 
parOld/test.sh should be trivial to rewrite in Java.
sure, I've filed 8203239 and 8203238.

Thanks!

It seems like a lot of tests contains a TEST.properties file with the content 
`exclusiveAccess.dirs=.`. Could this become the default value somewhere, so we 
don't need all those TEST.properties files?
y, but this will require finding a most top directory whose all tests have this 
TEST.properties file and it will also make it harder to understand how a test 
is executed. there was/is ongoing discussing w/ Jon on moving 'exclusiveAccess' 
to test description. anyhow I've filed an RFE to clean that up -- 8203241.

Ok, we can continue the discussion in 8203241.

Thanks,
Erik


Thanks,
Erik

Thanks,
-- Igor

Reply via email to