RFR: 8192837 Need new test for release file info

[David Holmes]

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
>>>>
>>>
>