RFR(M) : 8181761: add explicit @build actions for jdk.test.lib classes in all :tier2 tests

[Hamlin Li]

On 2017/6/9 12:19, Igor Ignatyev wrote:
> Hi Hamlin,
Hi Igor,

Thank you for explanation.
> I'm going to add explicit @build for jdk.test.lib.** classes in jdk 
> tests which are in :tier[1-3].
Got it.
> unfortunately this approach does not work if a test depends on a 
> library indirectly, e.g. you might have a couple of tests which share 
> some code (but not from /test/lib or /jdk/test/lib/testlibrary) and 
> this code depends on a testlibrary, so you will need to analyze test 
> classes files, which is not much different from analyzing them to get 
> all explicit @build.
> since your approach works only if all testlibrary classes are in a 
> package, we will need to update lots of test which use classes from 
> default package, which makes it a bit more painful.
I see. Yes, it's lot of extra clean-up of test library.
> but the main disadvantage is the assumption that all tests must follow 
> this rule, and if one test does not follow it, this test might pass, 
> while other innocent tests will fail.
I agree, the most failing tests are not the root cause but just victim, 
that's the reason to clean up all tests not just "fix" the failing ones.
>  if we were considering approaches w/ such flaw, I'd strongly 
> recommend to simply remove all @build actions (except needed due to 
> reflection accesses) and have the straightforward rule to follow and 
> one-liner to check this rule.
Got it. Maybe the most thorough/simple solution is to just separate 
tests output from each other totally.

Thank you for
-Hamlin
>
> -- Igor
>> On Jun 8, 2017, at 7:58 PM, Hamlin Li <huaming.li at oracle.com 
>> <mailto:huaming.li at oracle.com>> wrote:
>>
>> Hi Igor,
>>
>> I'm coping Jon as it needs Jon's comments.
>>
>>
>> Thank you for doing such a great refactoring, I believe it will make 
>> tests run more stable.
>>
>> I saw you are adding explicit @build to lots of test, are you going 
>> to clean up all tests to add explicit @build? If the answer is "yes", 
>> then I have a possible simple solution for you to consider.
>>
>> Steps:
>>
>>   1. refactor all test library classes in unnamed packages to named 
>> package.
>>
>>   2. just need to add explicit @build for every "import x.y.z;" in a 
>> test, means you only need to add @build for every test lib class 
>> directly used by your test, lib classes' dependency is not needed. 
>> e.g. there is a test containing only "import 
>> jdk.test.lib.process.ProcessTools;", then only need to add "@build 
>> jdk.test.lib.process.ProcessTools", and no @build is needed to be 
>> added for jdk.test.lib.process.StreamPumper, 
>> jdk.test.lib.JDKToolFinder and jdk.test.lib.Utils and further 
>> dependency if they have.
>>
>> This solution is based on the situation that all tests will be added 
>> @build for all test lib classes they directly used, that means no 
>> test is allow to just add "@library xxx".
>>
>> The advantage of this solution is:
>>
>>   1. the rule is simple to follow, reduce the pain for engineer 
>> writing the tests.
>>
>>   2. it might be possible to do it automatically, and have a tool to 
>> monitor all checked-in test code by this rule;
>>
>>   3. no need to refactor the test lib too much, e.g. no need to 
>> remove the dependency between test lib packages;
>>
>>
>> Of course, Jon's original strict solution is absolutely correct: to 
>> add @build for all test lib classes even if for the implicitly 
>> dependent classes. Jon's strict solution is for a more complicated 
>> situation where some tests are using explicit @build, but others are 
>> not, in this mixed situation my solution will *NOT* work. But as 
>> you're cleaning up all the tests, I think it's not necessary to 
>> follow such a strict rule.
>>
>>
>> I might miss something, please correct me then.
>>
>>
>> Thank you
>>
>> -Hamlin
>>
>>
>> On 2017/6/8 15:20, Igor Ignatyev wrote:
>>> http://cr.openjdk.java.net/~iignatyev//8181761/webrev.00/index.html
>>>> 432 lines changed: 404 ins; 1 del; 27 mod;
>>> Hi all,
>>>
>>> could you please review this changeset which adds explicit @build actions to tier2 jdk tests? other tests will be updated by the corresponding sub-tasks of 8181758[1].
>>>
>>> webrev:http://cr.openjdk.java.net/~iignatyev//8181761/webrev.00/index.html
>>> jbs:https://bugs.openjdk.java.net/browse/JDK-8181761
>>> testing: :tier2 (in progress)
>>>
>>> [1]https://bugs.openjdk.java.net/browse/JDK-8181758
>>>
>>> Thanks,
>>> -- Igor
>>
>