On 20/12/2017 2:23 AM, Randy Crihfield wrote:
This ought to be what you were asking for.
http://cr.openjdk.java.net/~shurailine/8192837/webrev.03/
Closer :) A few nits:
- the test needs a proper @run tag
- NegSOURCE is hardly informative - how about CheckReleaseFile?
- there are a couple of style nits with indentation of wrapped lines:
71 throw new RuntimeException("File " + fileName +
72 " not found reading data!", fileExcept);
73 } catch (IOException ioExcept) {
74 throw new RuntimeException("Unexpected problem reading
data!",
75 ioExcept);
should be:
71 throw new RuntimeException("File " + fileName +
72 " not found reading data!",
fileExcept);
73 } catch (IOException ioExcept) {
74 throw new RuntimeException("Unexpected problem reading
data!",
75 ioExcept);
As a general style comment there's no need to use instance methods here,
you could just define readFile as static, or even inline it into main
directly.
Thanks,
David
Thanks for the help
Randy
On 12/18/17 09:32 PM, David Holmes wrote:
Hi Randy,
jdk/sanity/Test8192837.java
We don't name tests with bug numbers any more - the file/class should
be renamed to something appropriate to its actual function.
64 // grab the line
65 if (readIn.startsWith("SOURCE="))
66 fishForSOURCE = readIn;
Do you expect to find more than one SOURCE line? If not this should
"break". If so, then you're only going to check the last one found.
98 if (runtime.contains("OpenJDK"))
99 new Test8192837(jdkPath + "/release");
100 else
101 System.out.println("Not an OpenJDK.");
It would be preferable if this can be done via some @requires tag
rather than within the test. But otherwise it would be better to print
"Test skipped: not an OpenJDK build".
Thanks,
David
On 19/12/2017 3:23 AM, Erik Joelsson wrote:
Redirecting to correct list.
The test seems to do what it set out to do.
/Erik
On 2017-12-18 17:55, Randy Crihfield wrote:
I have created an OpenJDK negative test that confirms the closed
source files are not included in the SOURCE.
Version of the actual test for review:
http://cr.openjdk.java.net/~shurailine/8192837/webrev.00/
Any comments/suggestions are welcome, also I will need a sponsor for
it at the end…
Randy
