> On Jun 5, 2017, at 10:50 PM, Alan Bateman <[email protected]> wrote:
> 
> On 06/06/2017 06:10, Igor Ignatyev wrote:
>> http://cr.openjdk.java.net/~iignatyev/8181335/webrev.00/index.html
>> 
> I added this so-called "packageless" CompilerUtils some time ago. I see the 
> version copied into the top-level repo is an older version and doesn't 
> specify UOE or handle the JRE case. Can you update the version in the top 
> repo as part of this so that they are the same?
I have updated it, just forgot to include into webrev, here is the diff:

> diff -r 1bed3f29d894 test/lib/jdk/test/lib/compiler/CompilerUtils.java
> --- a/test/lib/jdk/test/lib/compiler/CompilerUtils.java       Sat Jun 03 
> 02:43:31 2017 +0000
> +++ b/test/lib/jdk/test/lib/compiler/CompilerUtils.java       Mon Jun 05 
> 22:51:49 2017 -0700
> @@ -59,6 +59,11 @@
>          throws IOException
>      {
>          JavaCompiler compiler = ToolProvider.getSystemJavaCompiler();
> +        if (compiler == null) {
> +            // no compiler available
> +            throw new UnsupportedOperationException("Unable to get system 
> java compiler. "
> +                    + "Perhaps, jdk.compiler module is not available.");
> +        }
>          StandardJavaFileManager jfm = compiler.getStandardFileManager(null, 
> null, null);
>  
>          List<Path> sources


> A minor annoyance with this change is that it makes the line lengths 
> inconsistent in a number of tests -- e.g. 
> Class/forName/modules/TestDriver.java now has a 100+ line which should be 
> split into two to avoid horizontal scrolling.
sure, I'll split such lines.
> 
> Also can you explain what the difference is between /lib/testlibrary and 
> /test/lib? Maybe this is just work in process?
y, it's work in progress, AYMK, we had several almost identical copies of 
testlibaries, one in jdk/test, another in hotspot/test and yet another one in 
jaxp/test. that happened because we weren't able to share a testlibrary across 
repos. it was decided to have one common testlibrary in the top level repo, 
JDK-8141526[1] introduced top level testlibrary. unfortunately, when it was 
done, jdk and hotspot testlibraries had significantly differences, so we 
weren't able to merge them. instead we merged hotspot and top level library, 
removed hotspot library and annotated jdk version by @Deprecated by 
JDK-8157957[2]. JDK-8075327[3] aims to finish this merge and remove jdk version 
of testlibrary. this particular patch and several other my recent patches are 
part of this effort. 
answering your question, /test/lib references to the top-level testlibrary in 
<root>/test/lib, /lib/testlibrary references to jdk copy of testlibrary in 
jdk/test/lib/testlibrary. 

[1] https://bugs.openjdk.java.net/browse/JDK-8141526
[2] https://bugs.openjdk.java.net/browse/JDK-8157957
[3] https://bugs.openjdk.java.net/browse/JDK-8075327

> 
> Finally, one thing to keep in mind is that you are changing tests and test 
> infrastructure that is still changing in jdk9/dev. We have one (we hope) 
> final module system refresh coming that changes several of the tests that you 
> are changing. This might be a bit annoying for whoever sync's up jdk10/jdk10 
> from jdk9/jdk9 in a few weeks time.
how many tests will be changed by this module system refresh?

> 
> -Alan.

Reply via email to