[9] RFR: 8167240: Write new tests to cover functionality of existing 'jimage' options (original) (raw)

Jim Laskey (Oracle) james.laskey at oracle.com
Tue Nov 15 15:29:10 UTC 2016


+1

Really nice, thank you.

On Nov 15, 2016, at 11:16 AM, Denis Kononenko <denis.kononenko at oracle.com> wrote:

Hi, Please do re-review for these changes. 1) tests for list --include were rewritten accordingly to https://bugs.openjdk.java.net/browse/JDK-8167384; 2) removed tests for '@filename', see https://bugs.openjdk.java.net/browse/JDK-8169720; 3) two new CRs were submitted: https://bugs.openjdk.java.net/browse/JDK-8169715, https://bugs.openjdk.java.net/browse/JDK-8169713; WEBREV: http://cr.openjdk.java.net/~dkononenko/8167240/webrev.04/ BUGURL: https://bugs.openjdk.java.net/browse/JDK-8167240 Thank you, Denis.

Hi Andrey, No, it isn't. I submitted a new CR: https://bugs.openjdk.java.net/browse/JDK-8167384. Thank you, Denis.

Hi,

Looks OK. Is it 100% pass rate? —Andrey On 9 Nov 2016, at 20:36, Denis Kononenko <denis.kononenko at oracle.com> wrote:

Hi, After discussion with Andrey we decided to add more tests for corner cases. The new changes are available by the link below. WEBREV: http://cr.openjdk.java.net/~dkononenko/8167240/webrev.03/ BUGURL: https://bugs.openjdk.java.net/browse/JDK-8167240 Thank you, Denis. Hi, Looks OK to me. I can suggest two more cases. A directory and file symlink can be passed in options where tool requires a file path. —Andrey On 8 Nov 2016, at 16:17, Denis Kononenko <denis.kononenko at oracle.com> wrote:

Hi, The new version of changes. - Switched back to jdk/test/testlibrary to avoid unwanted dependencies (JImageToolTest.java); - Verified tests on smallest possible JDK build. WEBREV: http://cr.openjdk.java.net/~dkononenko/8167240/webrev.02/ BUGURL: https://bugs.openjdk.java.net/browse/JDK-8167240 Thank you, Denis. Denis, I can see that you have switched to the top level test library with this change. With that you are getting more module dependencies than just java.base. First of all, it would probably make sense to build only the classes you needed (which would be jdk.test.lib.process.ProcessTools, I assume), but even if you only build that, jdk.test.lib.process.ProcessTools has dependencies outside java.base module. You either have to declare @modules in your test or go back to the jdk/test/lib/testlibrary. Then, of course, unneeded module dependencies are questionable. Shura

On Nov 3, 2016, at 6:29 AM, Denis Kononenko <denis.kononenko at oracle.com> wrote: Hi, I've done some rework accordingly to Alan's and Shura's comments: 1) removed overlapped tests from JImageToolTest.java; 2) added new tests JImageVerifyTest.java for jimage verify; 3) reorganized jtreg's tags; The new WEBREV can be found here: http://cr.openjdk.java.net/~dkononenko/8167240/webrev.01/ Thank you, Denis. On 06.10.2016 19:37, Denis Kononenko wrote: Hi, Could someone please review these new tests for jimage utility. There're 5 new files containing tests to cover use cases for 'info', 'list', 'extract' and other options. No new tests for 'verify'. BUGURL: https://bugs.openjdk.java.net/browse/JDK-8167240 WEBREV: http://cr.openjdk.java.net/~dkononenko/8167240/webrev.00/

Thank you, Denis.



More information about the core-libs-dev mailing list