RFR: 8192837 Need new test for release file info

[Randy Crihfield]

Could this be it?

http://cr.openjdk.java.net/~shurailine/8192837/webrev.04/

Thanks!

Randy

On 12/20/17 04:51 AM, David Holmes wrote:
> 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
>>>>>
>>>>
>>