[10] RFR of JDK-8173411: Some testng tests check nothing in java time

[Amy Lu]

Thank you Roger and Jon for the comments!

Change request withdrawn :-)

Thanks,
Amy

On 10/14/17 3:04 AM, Roger Riggs wrote:
> Hi Amy,
>
> I don't see a problem with the cases of 'Total tests run: 0'.  It is 
> very explicit, no tests were selected.
> Testng has a number of dynamic mechanisms for test selection such that
> even files in the test directories might not have any tests to run. As 
> Jon pointed out that's just part of TestNG.
>
> Does this throw off the jtreg reports of tests run vs failures?  I 
> haven't seen that.
>
> I'd leave well enough alone.
>
> Thanks, Roger
>
>
>
>
> On 10/13/2017 1:39 AM, Amy Lu wrote:
>> non-test be run as a test, the result may lead to confusion.
>>
>> For example, if look at MockSimplePeriod.java test result, it shows:
>> ===============================================
>> java/time/test/java/time/MockSimplePeriod.java
>> Total tests run: 0, Failures: 0, Skips: 0
>> ===============================================
>>
>> Without looking into the detail of the test code, one may raise 
>> question why this test has 0 testcase.
>>
>> If this is not a concern, I can withdraw this change request.
>>
>> Thanks,
>> Amy
>>
>> On 10/13/17 1:06 AM, Roger Riggs wrote:
>>> Hi Amy,
>>>
>>> I'm not convinced this is the right move.  Moving parts of tests 
>>> further from the test
>>> is not helpful from a maintenance point of view.
>>> I'm not concerned about compilation time.  Its not significant.
>>>
>>> For example, AbstractDateTimeTest has @Test directives and is not 
>>> just library code.
>>>
>>> The files in the 'tck' hierarchy are intended to be self sufficient 
>>> and a direct mirror
>>> or the tests in the JCK.
>>>
>>> Please provide a more compelling reason for the change.
>>>
>>> Thanks, Roger
>>>
>>>
>>>
>>> On 10/10/2017 11:30 PM, Amy Lu wrote:
>>>> Please review the patch to change TestNG.dirs and lib.dirs (def and 
>>>> file structure) for test/jdk/java/time.
>>>>
>>>> test/jdk/java/time contains three sets of tests:
>>>> nontestng/
>>>> tck/
>>>> test/
>>>>
>>>> Tests from directory "tck" and "test" are testng tests with 
>>>> properties:
>>>> TestNG.dirs = ..
>>>> lib.dirs = ../../../lib/testlibrary
>>>> lib.build = jdk.testlibrary.RandomFactory
>>>>
>>>> But not all files under "TestNG.dirs" are real tests, some of them 
>>>> are "libraries" thus should not be put under "TestNG.dirs" (thus be 
>>>> run as testng test). Moreover, due to this def, when one runs tests 
>>>> from "test" directory, extra files (nontestng/* and tck/*) will 
>>>> also be compiled (which are unnecessary compiling).
>>>>
>>>> In this patch:
>>>> Non-test files ("libraries") are moved to "lib" directory;
>>>> Real tests previously under "test" dir are moved to test/jdk/, and 
>>>> tests under "tck" dir are moved to test/tck/;
>>>> test/jdk/ and test/tck/ each has TEST.properties with def:
>>>>     TestNG.dirs = .
>>>>     lib.dirs = /java/time/lib /lib/testlibrary
>>>> MockIOExceptionAppendable.java is not used anywhere, removed.
>>>> MockSimplePeriod.java previously exist in both "tck" and "test", 
>>>> now it is under "lib".
>>>>
>>>> With this change, non-test file then won't be run as testng test 
>>>> and no unnecessary compiling.
>>>>
>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8173411
>>>> webrev: http://cr.openjdk.java.net/~amlu/8173411/webrev.00/
>>>>
>>>> Thanks,
>>>> Amy
>>>>
>>>
>>
>