RFR(XL/M) : 8178788: wrap JCStress test suite as jtreg tests

[David Holmes]

Hi Igor,

On 19/04/2017 8:12 AM, Igor Ignatyev wrote:
> http://cr.openjdk.java.net/~iignatyev//8178788/webrev.00/index.html
>> 69903 lines changed: 69903 ins; 0 del; 0 mod;
> (69524 lines are generated)
>
> Hi all,
>
> could you please review this patch which adds a jtreg test wrapper for jcstress test suite and jtreg tests which run jsctress tests thru this wrapper?
>
> webrev: http://cr.openjdk.java.net/~iignatyev//8178788/webrev.00/index.html
> JBS: https://bugs.openjdk.java.net/browse/JDK-8178788
> testing: applications/jcstress

This looks really good! Only concern is that according to the docs jtreg 
doesn't seem to support multiple test declarations in a single file ?? 
Maybe the docs are out of date?

http://openjdk.java.net/jtreg/tag-spec.html

A few grammatical nits in the README:

23 The tests located under this directory run tests from Java 
Concurrency Stress

from Java -> from the Java

24 [1] aka jcstress test suite. This suite aims to verify the correctness of

Suggest:

test suite [1] (a.k.a. jcstress). This suite ...

25 concurrency support in JDK.

in JDK -> in the JDK

27 All the tests are run thought test driver class -- JcstressRunner, which

s/thought/through the/

29 spawns a new JVM to run one jsctress test and checks that it is finished

is finished -> finishes

33 changed, one should make corresponding changes in the artifact 
description in

in the -> to the

36 */Test.java files should never be modified directly, because they are

Insert "The" at start.

---

General Java style nits re long ling wrapping. You seem to be using a 
consistent "indent by 8" style when wrapping long lines. The "usual" 
style guidelines suggest a more structured indentation style, for example:

- when wrapping the arguments to a call align after the opening ( of the 
call ie:

        throw new Error("TESTBUG: Can not resolve artifacts for "
                        + JcstressRunner.class.getName(), e);

- when you have chained method calls, try to align at the dots ie:

     return artifacts.get("org.openjdk.jcstress.jcstress-tests-all-0.3")
                     .toAbsolutePath();

and

ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(getCmd(args))
                                 .redirectErrorStream(true)
                                 .redirectOutput(out.toFile());

---

test/applications/jcstress/TestGenerator.java

44  * This is a test generator, should be run only when jsctress is changed

Typo: jsctress -> jcstress

  51  * Use jcstress test suite to generates

generates -> generate

Strictly speaking the copyright years should only be updated when there 
is a substantive change to the text of the file, so updating whenever 
the files are regenerated is not really correct. This may need to be 
manually adjusted when commiting updated files ... though if there is no 
change other than to copyright year then the changes can simply be 
ignored (hg revert) and no update pushed.

Thanks,
David
------

> Thanks,
> -- Igor
>