Thank you, Christoph Frank
> -----Original Message----- > From: Langer, Christoph [mailto:christoph.lan...@sap.com] > Sent: Tuesday, January 24, 2017 6:30 PM > To: Frank Yuan > Cc: 'Daniel Fuchs'; core-libs-dev@openjdk.java.net > Subject: RE: RFR (JAXP) 8169827: > javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed > > Hi, > > I submitted my fix under Bug https://bugs.openjdk.java.net/browse/JDK-8173261: > http://hg.openjdk.java.net/jdk9/dev/jaxp/rev/620d0c38581f > > I also added a comment to https://bugs.openjdk.java.net/browse/JDK-8169827 > > Best regards > Christoph > > > > -----Original Message----- > > From: Frank Yuan [mailto:frank.y...@oracle.com] > > Sent: Dienstag, 24. Januar 2017 09:45 > > To: Langer, Christoph <christoph.lan...@sap.com> > > Cc: 'Daniel Fuchs' <daniel.fu...@oracle.com>; core-libs-dev@openjdk.java.net > > Subject: RE: RFR (JAXP) 8169827: > > javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed > > > > > From: Langer, Christoph [mailto:christoph.lan...@sap.com] > > > To: Frank Yuan > > > Cc: 'Daniel Fuchs'; core-libs-dev@openjdk.java.net > > > Subject: RE: RFR (JAXP) 8169827: > > javax/xml/jaxp/isolatedjdk/catalog/PropertiesTest.sh copied JDK failed > > > > > > Hi Frank, > > > > > > thanks for your comments. > > > > > > I already thought it might be better to create a new bug for my repair > > > work - > > as 8169827 actually is about the intermittent > > failure which will > > > perhaps not be solved with my changes. So I'll create a new item and > > > submit > > my changes for that one. > > > > > > Furthermore, I would also support creating Java code to do the copy work > > instead of using the shell script, though doing this is > > rather out of > > > scope for me, at the moment. > > > > Just a suggestion, please feel free to leave it:) > > > > >This should be done in a common library, e.g. > > /test/javax/xml/jaxp/libs/jaxp/library. I also see that the same > > > requirement exists for tests in the jdk repo - unfortunately there is no > > > shared > > testing library with 'jdk'. So we'll probably end > > up with some > > > duplicated code :( > > > > > > By the way: In the jaxp test tree, there exists a lib > > /test/javax/xml/jaxp/libs/jdk/testlibrary out of which at least some stuff > > seems to be > > > obsolete for jaxp testing and/or significantly behind jdk's > > > test/lib/testlibrary. I > > guess here's some room for > > cleanup/modernization. > > > > > > > Yes, testlibrary is a copy from jdk repo, there are a few JAXP tests using > > the > > library, so we didn't keep it synced with JDK. In my > > mind, we don't need spend too much effort on it unless it impacts JAXP > > tests. > > > > > As for the idea with the links - it sounds interesting, but it also adds > > > more > > complexity. And maybe it is not such a good idea for > > Windows > > > systems... > > > > I didn't test it in Windows, please leave it to me. I will update to you > > and Joe > > when I have the result. Thanks > > > > Frank > > > > > > > > Best regards > > > Christoph > > > > > > > -----Original Message----- > > > > From: Frank Yuan [mailto:frank.y...@oracle.com] > > > > Sent: Dienstag, 24. Januar 2017 08:59 > > > > To: Langer, Christoph <christoph.lan...@sap.com>; 'Daniel Fuchs' > > > > <daniel.fu...@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 > > > > > > > > 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 > > > > > > > > > > > > >
