Hi,
Beside of listed in the review tests there are another tests in the
folder. In case I'd update TEST.properties the not listed tests also
will get unnecessary dependency from java.logging.
--
With best regards,
Sergei
On 1 Nov 2016, at 18:40, Roger Riggs <Roger.Riggs at oracle.com
<http://mail.openjdk.java.net/mailman/listinfo/net-dev>> wrote:
>//>/Hi, />//>/Ok, leave the logging in. (There are currently 3 issues marked as
intermittent that refer to httpclient). />//>/Roger />//>/p.s. I'm also a fan of using the TEST.properties for test directives
that apply to multiple tests />/in a directory. In this case, "modules = java.logging”. /
If I understand correctly, the list of ‘modules' in TEST.properties is only used
if the test does not explicitly have any @modules tags. So, if you want any
specific exports, or modules, then you need to specify them all.
-Chris.
>//>/On 11/1/2016 2:34 PM, Daniel Fuchs wrote: />>/Hi Roger, />>//>>/I think we agree :-) />>//>>/On 01/11/16 18:01, Roger Riggs wrote: />>>/Hi Daniel, />>>//>>>/It seemed useful to be able to run the test in as many environments as />>>/possible />>>/though realistically java.util.logging may be there too. />>>//>>>/I don't see that setting the logging levels is intrinsic to the tests />>>/and would be used />>>/for debugging so perhaps that function can be dropped or configured
via the />>>/java.util.logging.config.file system property if/when needed. />>//>>/For the java.util.logging.config.file system property to work then />>/you would need java.logging to be linked in - so if you do want />>/a test to spit out logging traces then you should require java.logging />>/in @modules - whether you use logging.properties or programmatic />>/interface to configure logging. />>//>>/So it all depends on how useful said traces are when investigating />>/a test failure. If a test is known to fail intermittently and />>/test failure would be very difficult to analyze without the logging />>/traces then the test should probably require and configure java.logging />>/upfront. />>//>>/Otherwise I agree you might want to remove the useless code, unless />>/you do want to validate that no NPE or else happen while logging... />>//>>/best regards, />>//>>/-- daniel />>//>>//>>>//>>>/$.02, Roger />>>//>>>//>>>//>>>/On 11/1/2016 1:53 PM, Daniel Fuchs wrote: />>>>/Hi Roger, />>>>//>>>>/On 01/11/16 17:21, Roger Riggs wrote: />>>>>/Hi Sergei, />>>>>//>>>>>/I think it would be preferable to convert the tests to use />>>>>/System.getLogger. />>>>>/Is that possible? />>>>//>>>>/Some of the tests want to configure the logging, rather />>>>/than simply produce traces - so they will need java.logging />>>>/to do that: />>>>//>>>>/670 Logger logger = Logger.getLogger("com.sun.net.httpserver"); />>>>/671 ConsoleHandler ch = new ConsoleHandler(); />>>>/672 logger.setLevel(Level.ALL); />>>>/673 ch.setLevel(Level.ALL); />>>>/674 logger.addHandler(ch); />>>>//>>>>/It's recommended to use System.Logger to log messages, />>>>/but you will have to use java.util.logging if you want to configure />>>>/the logging framework. Of course a library shouldn't do that, />>>>/but a test is well in its right to configure logging to make />>>>/sure the traces will appear in the log. />>>>/Unless you do want to run the test in a VM that does not have />>>>/java.logging linked in. />>>>//>>>>/cheers, />>>>//>>>>/-- daniel />>>>//>>>>//>>>>>//>>>>>/Thanks, Roger />>>>>//>>>>>//>>>>>/On 11/1/2016 1:15 PM, Sergei Kovalev wrote: />>>>>>/Hello all, />>>>>>//>>>>>>/Please review a small fix for tests. />>>>>>//>>>>>>/BugID: https://bugs.openjdk.java.net/browse/JDK-8169002 />>>>>>/WebRev: http://cr.openjdk.java.net/~skovalev/8169002/webrev.00/
<http://cr.openjdk.java.net/%7Eskovalev/8169002/webrev.00/> />>>>>>//>>>>>>/Issue: Several tests from java/net/httpclient folder have undeclared />>>>>>/dependency on java.logging module. This issue leads the test to fail />>>>>>/in case module limitation. />>>>>>/Solution: added module declaration into jtreg header and organized />>>>>>/imports. /
