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...@oracle.com
<mailto:huaming...@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
