On 21/05/15 02:09, Mandy Chung wrote:
Actually - I think the test should be type != Type.SUMMARY rather
than type == Type.VERBOSE.
jdeps has -filter:none option.
$ jdeps -s -filter:none -e use.unsafe.UseUnsafeClass *.jar
When -filter:none is used, I think it’s right for the summary page
should include unsafe.jar in that case. The default will filter out
same package dependency and hasDependency returns false in that case.
Ah - I understand your point now.
New webrev:
http://cr.openjdk.java.net/~dfuchs/webrev_8080608/webrev.01/
Analyzer.java looks good except that I think it doesn’t need type !=
Type.SUMMARY check.
OK done.
Thanks for adding the great tests. The test can be extracted as a jdeps
test library. Do you have the cycle to refactor it so that it can be
used in future jdeps tests?
Maybe we should wait a bit and see what is generic enough to
be extracted. There are a few things that the test expects and
which are verified by the inputs & classes to be analyzed.
For instance - the pattern given to -e must match only one class name.
The test depends on this.
TestCaseData now takes 3 String[][] to list classes, dependencies and
archives. You can consider making it a jdeps data Builder that will
make the data more explicit expected dependencies of each class.
OK - Let's consider doing this when we extract the test to
put in the library. In the mean time I have added some javadoc
comments to the static TestCaseData.make() method which should
hopefully better clarify what the String[][] arrays should
contain.
Can you run all test cases in the same VM? The test calls
com.sun.tools.jdeps.Main directly and it’d be good to avoid running each
test case as a separate VM unless it’s necessary.
Done. I lazily used /othervm at first because I was resetting the
default locale to Locale.ENGLISH to make sure we get a canonical
output. I have now added a runWithLocale method that should make
it possible to run the test in same VM mode.
Can you also add
@modules jdk.dev/com.sun.tools.jdeps following @summary as it uses
internal API.
done.
http://cr.openjdk.java.net/~dfuchs/webrev_8080608/webrev.02/
best regards,
-- daniel
Mandy
