Changed B6890349.java to use URL constructor.
Changed sequence of URIBuilder calls to `.host().port().path()`.
Added missing `.path("/")`.
Added logging for most of the constructed URLs.PTAL: http://cr.openjdk.java.net/~aeubanks/8220575/webrev.01/test/jdk/com/sun/net/httpserver/bugs/B6373555.java.udiff.html I was considering adding a buildURL() to URIBuilder since so many of these URIs end up getting converted to a URL in the end. Not sure if that's a good idea or not. On Tue, Mar 26, 2019 at 7:15 AM Chris Hegarty <chris.hega...@oracle.com> wrote: > Arthur, > > I like the way this is turning out. > > > On 26 Mar 2019, at 12:05, Daniel Fuchs <daniel.fu...@oracle.com> wrote: > > > > ... > > 2. B6890349.java > > > > Using URI.toURL() in this test will change the nature of > > the test. I believe that in this specific case using the > > multi-arg URL constuctor should be preferred. > > The reason is that URI as per its specification encodes > > illegal characters, while URL as per its specification, > > does not. So going through URI here will lead to a > > different URL than the one that the test is testing. > > > > I see from your mail that you have noticed the difference :-) > > > > For this one test I'd suggest to make an exception and > > change the URL construction to: > > > > URL u = new URL("http", > > InetAddress.getLoopbackAddress().getHostAddress(), > > port, > > "/foo\nbar"); > > > > Even if we get to deprecating the URL constructors in the > > future this would still be a valid case for using them in > > a test. > > Right. The above accepting-illegal-characters behaviour is one of > the reasons, non-test code, should avoid the URL constructors. > > > 3. on the 'nit' side it's a bit strange to see > > the sequence of calls `.host(..).path(..).port(..)`, > > as `.host(..).port(..).path(..)` would feel > > natural - but don't feel obliged to make this change. > > URI component-wise ordering makes the code a little easier to > read. I’d just do it. > > I dislike noisy tests, but it would be helpful for these tests to print > the URL that has been constructed ( System.out.println("URL: " + url) ). > It will make it easier to verify the test's behaviour on different > system configuration ( I usually just look in the jtr file ) > > -Chris. > > >
