On 2014-11-12 16:21, Evgeniya Stepanova wrote:
Hi everyone!

Since the decision was made to change only tests which fail because of conflict for now (skip "selfish" tests), I post new webrev for jdk part of the JDK-8019361 <https://bugs.openjdk.java.net/browse/JDK-8019361>:
http://cr.openjdk.java.net/~avstepan/eistepan/8062536/webrev.03/

Thanks for updating the webrev!

A couple of comments:

MemoryTestAllGC.sh

The test is run three times, once with no params, once with ParallelGC and once with CMS. So, I think the @requires should just be vm.gc == "null". Similarly to what was done for PendingAllGC.sh.


TestInputArgument.sh

The changes here seem unrelated to @requires.


EnqueuePollRace.java

Can you explain why it is safe to remove -XX:+UseSerialGC for this test?


JpsHelper.java

Can you explain why it is safe to remvoe -XX:+UseParallelGC for this test?


When I use Aurora to check what tests that currently are considered known because of JDK-8019361 I get a pretty long list:

http://aurora.ru.oracle.com/functional/faces/CRRules.xhtml?cr=JDK-8019361

Are the tests in webrev.03 the only tests that still fail? Have the others been fixed in other ways?

Thanks,
Bengt




Thanks,
Evgeniya Stepanova
On 07.11.2014 15:34, Evgeniya Stepanova wrote:
David, Filipp, Katja

Diff have been updated one more time:
java/lang/management/RuntimeMXBean/TestInputArgument.sh and test/java/lang/ref/EnqueuePollRace.java have been changed
http://cr.openjdk.java.net/~eistepan/8062536/webrev.02/

Thanks

On 07.11.2014 9:37, David Holmes wrote:
On 7/11/2014 12:36 AM, Evgeniya Stepanova wrote:
New webrev:
http://cr.openjdk.java.net/~eistepan/8062536/webrev.01/

In:

test/java/lang/management/RuntimeMXBean/TestInputArgument.sh

the use of the gc options seems incidental - it's just picking two innocuous options to use - similar to the JpsHelper case. You could replace +UseParallelGC with something like +UseFastJNIAccessors (platform independent and normally true).

David
-----

Thanks,
Evgeniya Stepanova
On 06.11.2014 17:35, Yekaterina Kantserova wrote:
Thanks a lot!

On 11/06/2014 02:05 PM, Evgeniya Stepanova wrote:
Hi Katja,

Ok, this seems to be a perfect solution. Thank you. I'll change the
diff accordingly.


Thanks,
Evgeniya Stepanova
On 06.11.2014 16:56, Yekaterina Kantserova wrote:
Hi Dima,

On 11/06/2014 11:22 AM, Dmitry Fazunenko wrote:
Hi Katja,

You are right, there will be no conflict, because test ignores any
external VM flags.
So, adding @requires seems unnecessary here, but...

Ignoring external options is bad thing, such "selfish" tests are
not applicable for other areas, like GC, compiler, RT.

This particular case is to test the defined flags are shown up as
expected.

Evgeniya,

would you mind to change JpsHelper.java instead?

+++ b/test/sun/tools/jps/JpsHelper.java
@@ -93,7 +93,7 @@
     /**
      * VM arguments to start test application with
      */
-    public static final String[] VM_ARGS = {"-Xmx512m",
"-XX:+UseParallelGC"};
+    public static final String[] VM_ARGS = {"-Xmx512m",
"-XX:+PrintGCDetails"};
     /**
      * VM flag to start test application with
      */

Best regards,
Katja




@requires  will allow to modify tests to include external vm
options without any risk of bumping into conflict and extend area
of test applicability.

But if you still believe, that @requires is not necessary - it's
not a problem, tests could be kept as is.

Thanks,
Dima


On 06.11.2014 16:27, Yekaterina Kantserova wrote:

Hi Evgeniya,

As David has pointed out these jps tests are not testing gc. The
-XX:+UseParallelGC is just an arbitrary chosen test flag. There
should not be any conflicts either since these tests are running
in driver mode:

...
 * @run driver TestJpsJar
...

which means no flags from above are accepted.

Thanks,
Katja



On 11/06/2014 11:05 AM, Evgeniya Stepanova wrote:
Hi David,

tag added because tests contain string
 cmd.addAll(JpsHelper.getVmArgs());

and JpsHelper defines
...
public static final String[] VM_ARGS = {"-Xmx512m",
"-XX:+UseParallelGC"};
...
public static List<String> getVmArgs() throws IOException {
        if (testVmArgs == null) {
            testVmArgs = new ArrayList<>();
testVmArgs.addAll(Arrays.asList(VM_ARGS));
            testVmArgs.add("-XX:Flags=" +
getVmFlagsFile().getAbsolutePath());
        }
        return testVmArgs;
    }

Tests itself wouldn't fail if we use another GC, tag added for
cleanup-if we use for example SerialGC we must be sure that tests
passed with this GC, not with another one. Now you will assume
that concrete test passed with Serial GC, but it run only with
Parallel GC. Plus there is no any sense to run test twice in TC
(with different GC, since it use only Parallel)

Thanks,
Evgeniya Stepanova
On 06.11.2014 6:20, David Holmes wrote:
Hi Evgeniya,

On 6/11/2014 1:33 AM, Evgeniya Stepanova wrote:
Hi,

Please review changes for 8062536, the OpenJDK/jdk part of the
JDK-8019361

bug: https://bugs.openjdk.java.net/browse/JDK-8062536
fix: http://cr.openjdk.java.net/~eistepan/8062536/webrev.00/

Problem: Some tests explicitly set GC and fail when another GC
is set
outside

I don't see why you have done this for the

test/sun/tools/jps/TestJps*.java

tests. They don't set any GC flags.

Solution: Such tests marked with the jtreg tag "requires" to
skip test
if there is a conflict

Just wondering: Does a skipped test get a .jtr file showing it
was skipped; or does it only appear in the higher-level jtreg log?

Thanks,
David

Tested locally with different GC flags (-XX:+UseG1GC,
-XX:+UseParallelGC, -XX:+UseSerialGC, -XX:+UseConcMarkSweep and
without
any GC flag). Tests are being excluded as expected. No tests
failed
because of the conflict.

Thanks,
Evgeniya Stepanova

//

--
/Evgeniya Stepanova/






--
/Evgeniya Stepanova/


--
/Evgeniya Stepanova/

--
/Evgeniya Stepanova/

--
/Evgeniya Stepanova/

Reply via email to