RFR: 8192837 Need new test for release file info

[Randy Crihfield]

I think I have it now.

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


Thanks for the help again!

Randy


On 12/20/17 08:30 PM, David Holmes wrote:
> On 21/12/2017 2:51 AM, Randy Crihfield wrote:
>>
>> Could this be it?
>>
>> http://cr.openjdk.java.net/~shurailine/8192837/webrev.04/
>
> 29  * @run testing NegReleaseSOURCE
>
> If you are going to write a jtreg test you have to actually run it 
> under jtreg! That @run line is invalid - "testing" is not an @run 
> action (did you perchance try to copy a testng @run entry?). A minimal 
> @run for this test would be:
>
> @run main NegReleaseSOURCE
>
> As for the name ... sorry I don't know what NegReleaseSOURCE is 
> supposed to mean. There's no reason to be cryptic (that's why we 
> stopped using bug numbers to name tests). You could even add a 
> subdirectory for release file related tests:
>
> sanity/releaseFile/CheckSOURCE.java
>
> Thanks,
> David
>
>> 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
>>>>>>>
>>>>>>
>>>>
>>