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 > > > > > > > > > >
