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


Reply via email to