Hi Christoph Many thanks for your effort!
It's really better than the old version! However I have 2 comments although I am not a reviewer(as you often stated :)) 1. we could also write java code to copy/delete JDK. 2. 8169827 is to track PropertiesTest failed in copying JDK intermittently. I suggest to use another bug for your improvement because I am not sure if this patch can improve the issue of 8169827. To Christoph and Daniel Btw, Christoph's patch inspired me. You know, PropertiesTest copies a JDK in order to isolate the file change of the JDK, actually there are some other similar tests(to test some other JDK property or config files) in JDK repo, they take the same way as well as have similar code... I have another idea for this test, that is we can only make symbolic link to the JDK files/directories except conf directory, create a directory named with "conf" under the new jdk directory, and then make symbolic link to the files/directories in conf to the real jdk/conf except jaxp.properties. This will reduce the most of copying work and may reduce the risk of copying work. What's your opinion? Thanks Frank > -----Original Message----- > From: Langer, Christoph [mailto:christoph.lan...@sap.com] > To: Daniel Fuchs; Frank Yuan; core-libs-dev@openjdk.java.net > Subject: RE: RFR (JAXP) 8169827: > javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed > > Hi Daniel, > > thanks for checking/reviewing. So I'll submit with removing the > ProblemList.txt entry and I'll also remove the intermittent flag. > > Sounds fair to check later if problems will still show up. Although I have > the feeling that the issue of > https://bugs.openjdk.java.net/browse/JDK-8147431 might appear again... > > Best regards > Christoph > > > -----Original Message----- > > From: Daniel Fuchs [mailto:daniel.fu...@oracle.com] > > Sent: Montag, 23. Januar 2017 17:12 > > To: Langer, Christoph <christoph.lan...@sap.com>; Frank Yuan > > <frank.y...@oracle.com>; core-libs-dev@openjdk.java.net > > Subject: Re: RFR (JAXP) 8169827: > > javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed > > > > Hi Christoph, > > > > Thanks for fixing this test! > > > > I imported your patch, modified ProblemList.txt to not skip the test, > > and sent it through our test system, and I'm happy to report that > > the test was run on all available platforms with no failure. > > > > So I think you should simply remove the line from ProblemList.txt > > (no need for a new webrev). > > If it fails again on more exotic platform we'll simply add it > > back to ProblemList.txt for those platforms where it fails > > (I guess it could happen if there's not enough disk space). > > > > Otherwise I have looked over the changes you proposed and they > > do seem OK to me. > > > > +1 > > > > best regards, > > > > -- daniel > > > > On 23/01/17 10:03, Langer, Christoph wrote: > > > Hi, > > > > > > while working on jaxp changes and running jtreg tests I found that test > > javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh does not work. I then > > saw > > that this was already reported with bug 8169827. But, as I had already spent > > some time to fix this test I'd like to contribute my fix: > > > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8169827 > > > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8169827.0/ > > > > > > I converted the test to Java and removed the shell script > > > PropertiesTest.sh. > > This resolves the compilation issue. > > > > > > However, the test needs to copy an isolated jdk as it modifies files in > > > the JDK. > > So I'm still using the copy script to first copy the jdk and afterwards > > remove the > > copy. These are separate 'shell' test steps. And in the actual test I'm > > running a > > child process with the isolated JDK. > > > > > > I also don't know if the test should be kept in the problem list and/or > > > also be > > tagged as 'intermittent' as the whole jdk copying procedure by means of a > > shell > > script seems error prone. In case we keep the entry in the problem list, I > > can > > also open a separate bug for my change. > > > > > > @Frank: I don't know if you have some larger change in mind which improves > > the isolated jdk type testing greatly, otherwise I think this fix could at > > least > > make things better than they are at the moment. > > > > > > Thanks & Best regards > > > Christoph > > >
