Re: RFR - JImageListTest test fix
On 02/20/2018 10:47 AM, Alan Bateman wrote: On 19/02/2018 14:51, Michal Vala wrote: Hi, sending patch fixing JImageListTest failing tests. It was in ProblemList and this fix greens the test case, so the patch also removes it from the list. These tests are failing on Windows now. Nothing to do with your changes, it seems the original tests added via JDK-8167240 weren't run enough to shake out issues with the tests. So we need to re-add them to the exclude list, Sundar has already reviewed. ok, I can take a look at them -Alan diff --git a/test/jdk/ProblemList.txt b/test/jdk/ProblemList.txt --- a/test/jdk/ProblemList.txt +++ b/test/jdk/ProblemList.txt @@ -464,6 +464,9 @@ tools/launcher/FXLauncherTest.java 8068049 linux-all,macosx-all +tools/jimage/JImageExtractTest.java 8198405 windows-all +tools/jimage/JImageListTest.java 8198405 windows-all + -- Michal Vala OpenJDK QE Red Hat Czech
Re: RFR - JImageListTest test fix
On 19/02/2018 14:51, Michal Vala wrote: Hi, sending patch fixing JImageListTest failing tests. It was in ProblemList and this fix greens the test case, so the patch also removes it from the list. These tests are failing on Windows now. Nothing to do with your changes, it seems the original tests added via JDK-8167240 weren't run enough to shake out issues with the tests. So we need to re-add them to the exclude list, Sundar has already reviewed. -Alan diff --git a/test/jdk/ProblemList.txt b/test/jdk/ProblemList.txt --- a/test/jdk/ProblemList.txt +++ b/test/jdk/ProblemList.txt @@ -464,6 +464,9 @@ tools/launcher/FXLauncherTest.java 8068049 linux-all,macosx-all +tools/jimage/JImageExtractTest.java 8198405 windows-all +tools/jimage/JImageListTest.java 8198405 windows-all +
Re: RFR - JImageListTest test fix
On 19/02/2018 14:51, Michal Vala wrote: Hi, sending patch fixing JImageListTest failing tests. It was in ProblemList and this fix greens the test case, so the patch also removes it from the list. It will probably need new bug id and I will need a sponsor for this. Fix itself is simple. There was incorrect assert for list of images contains java.base image as it was testing index of "java.base" > 0 and it's usually at first place, thus index 0. I've changed it to List#contains. This looks okay to me. -Alan
RFR - JImageListTest test fix
Hi, sending patch fixing JImageListTest failing tests. It was in ProblemList and this fix greens the test case, so the patch also removes it from the list. It will probably need new bug id and I will need a sponsor for this. Fix itself is simple. There was incorrect assert for list of images contains java.base image as it was testing index of "java.base" > 0 and it's usually at first place, thus index 0. I've changed it to List#contains. Thanks! -- Michal Vala OpenJDK QE Red Hat Czech --- old/test/jdk/ProblemList.txt 2018-02-19 15:40:56.429753232 +0100 +++ new/test/jdk/ProblemList.txt 2018-02-19 15:40:56.215752710 +0100 @@ -465,7 +465,6 @@ tools/launcher/FXLauncherTest.java 8068049 linux-all,macosx-all tools/jimage/JImageExtractTest.java 8170120 generic-all -tools/jimage/JImageListTest.java8170120 generic-all --- old/test/jdk/tools/jimage/JImageListTest.java 2018-02-19 15:40:56.869754306 +0100 +++ new/test/jdk/tools/jimage/JImageListTest.java 2018-02-19 15:40:56.657753788 +0100 @@ -57,7 +57,7 @@ .map(s -> s.substring(s.indexOf(':') + 1).trim()) .collect(Collectors.toList()); assertTrue(modules.size() > 0, "Image contains at least one module."); -assertTrue(modules.indexOf("java.base") > 0, "Module java.base found."); +assertTrue(modules.contains("java.base"), "Module java.base found."); }); } @@ -88,7 +88,7 @@ .map(s -> s.substring(s.indexOf(':') + 1).trim()) .collect(Collectors.toList()); assertTrue(modules.size() > 0, "Image contains at least one module."); -assertTrue(modules.indexOf("java.base") > 0, "Module java.base found."); +assertTrue(modules.contains("java.base"), "Module java.base found."); Set entries = Stream.of(lines) .filter(s -> { return !s.startsWith("Module: ") && !s.startsWith("Offset"); })