On Mon, 22 Aug 2022 17:18:21 GMT, Ramesh Bhagavatam Gangadhar <rgangad...@openjdk.org> wrote:
>> There are total 160 scenarios written with combination of client properties >> (Client Scenarios) and Server Response (Server Scenarios). >> >> In tabular format, Client and Server scenarios along with expected output >> are documented >> here:[Permalink](https://bugs.openjdk.org/browse/JDK-8291226?focusedCommentId=14519074&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14519074) >> >> This Program Should be run mandatorily in othervm mode itself since it has >> system property changes so can't be clubbed with other scenarios. so each >> scenario should be run in individual JVM. >> >> For each and every scenario, ServerSocket is created and waits for clients >> to connect to it. >> isProxySet and serverReady are shared variables between server thread and >> client thread(main) and it should be set and reset to false for each and >> every scenario. >> >> isProxySet and serverReady variables should be set by server thread before >> proceeding to client thread(main). >> >> if isProxySet variable is set to true then client set the proxy value to >> url.openConnection(Proxy) >> <SNIPPET> >> if (isProxySet) { >> httpUrlConnection = (sun.net.www.protocol.http.HttpURLConnection) >> url .openConnection(new Proxy(Type.HTTP, new InetSocketAddress("localhost", >> SERVER_PORT))); } >> else { >> httpUrlConnection = (sun.net.www.protocol.http.HttpURLConnection) >> url.openConnection(); >> } >> </SNIPPET> >> >> Program tries to fetch the Value of <Key, Value> Pairs of HashMap >> KeepAliveCache where Key is KeepAliveKey and Value is ClientVector >> KeepAliveTimeout is stored in Value ClientVector of HashMap KeepAliveCache. >> >> if connection is cached then KeepAliveTimeout is stored in ClientVector. >> KeepAliveTimeout stored in Value(ClientVector) of HashMap KeepAliveCache is >> compared with Expected Value. >> >> if connection is not cached then connection is terminated immediately. > > Ramesh Bhagavatam Gangadhar has updated the pull request incrementally with > one additional commit since the last revision: > > Update and rename KeepAliveTest to KeepAliveTest.java > > 1. Increased space from 2 to 4. > 2. skipped test scenarios from 113 to 128 && 145 to 160 Changes requested by dfuchs (Reviewer). test/jdk/sun/net/www/http/HttpClient/KeepAliveTest.java line 263: > 261: private static final String H = C + NEW_LINE + > KEEP_ALIVE_TIMEOUT_NEG; > 262: private static final String I = C + NEW_LINE + > KEEP_ALIVE_TIMEOUT_ZERO; > 263: private static Logger logger = > Logger.getLogger("sun.net.www.protocol.http.HttpURLConnection"); should be final test/jdk/sun/net/www/http/HttpClient/KeepAliveTest.java line 264: > 262: private static final String I = C + NEW_LINE + > KEEP_ALIVE_TIMEOUT_ZERO; > 263: private static Logger logger = > Logger.getLogger("sun.net.www.protocol.http.HttpURLConnection"); > 264: private String[] serverScenarios = { should be final test/jdk/sun/net/www/http/HttpClient/KeepAliveTest.java line 279: > 277: * following are client scenarios which are repeated. > 278: */ > 279: private String[] a = { should be final test/jdk/sun/net/www/http/HttpClient/KeepAliveTest.java line 287: > 285: SERVER_100 + CLIENT_SEPARATOR + PROXY_200_NEGATIVE, > SERVER_100_NEGATIVE + CLIENT_SEPARATOR + PROXY_200 > 286: }; > 287: private String[] clientScenarios = { should be final test/jdk/sun/net/www/http/HttpClient/KeepAliveTest.java line 299: > 297: a[0] , a[1], a[2], a[3], a[4], a[5], a[6], a[7], a[8], a[9], > a[10], a[11], a[12], a[13], a[14], a[15], > 298: }; > 299: private int[] expectedValues = { should be final test/jdk/sun/net/www/http/HttpClient/KeepAliveTest.java line 311: > 309: 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, > 0, 0, 0, > 310: }; > 311: private CountDownLatch countDownLatch = null; should be final and initialized to `new CountDownLatch(1);` test/jdk/sun/net/www/http/HttpClient/KeepAliveTest.java line 317: > 315: if((scenarioNumber >= 112 && scenarioNumber <= 127) || > (scenarioNumber >= 144 && scenarioNumber <= 159)) { > 316: System.out.println("Scenario Skipped:"+scenarioNumber); > 317: countDownLatch = new CountDownLatch(0); should count down the latch instead of replacing its value test/jdk/sun/net/www/http/HttpClient/KeepAliveTest.java line 328: > 326: } > 327: System.out.println(); > 328: serverInit(scenarioNumber); should probably be called `startServer` test/jdk/sun/net/www/http/HttpClient/KeepAliveTest.java line 329: > 327: System.out.println(); > 328: serverInit(scenarioNumber); > 329: clientInit(scenarioNumber); should probably be called `runClient` test/jdk/sun/net/www/http/HttpClient/KeepAliveTest.java line 466: > 464: > 465: // wait until ServerSocket moves to listening state. > 466: while (!serverReady) { Could use a cyclic barrier or a count down latch instead of sleeping here. test/jdk/sun/net/www/http/HttpClient/KeepAliveTest.java line 571: > 569: } > 570: } > 571: } There is an alignment problem here: it looks as if a close brace is missing, test/jdk/sun/net/www/http/HttpClient/KeepAliveTest.java line 576: > 574: System.out.println("Usage:java KeepAliveTest.java > <scenarioNumber>"); > 575: System.out.println("e.g.:java KeepAliveTest.java 1"); > 576: System.exit(1); Since this is a test I would advise throwing an `IllegalArgumentException` or `AssertionError` instead of calling `System.exit(1);` Though arguably it doesn't matter much since this test must run in `/othervm` mode. test/jdk/sun/net/www/http/HttpClient/KeepAliveTest.java line 584: > 582: KeepAliveTest keepAliveTest = new KeepAliveTest(); > 583: if (args.length != 0) { > 584: keepAliveTest.myinit(Integer.valueOf(args[0])); It would be good to have method names that reflect what the method does. Here for instance - what `myinit` does is actually running (or starting) the given scenario. So maybe it should be called `startScenario`. ------------- PR: https://git.openjdk.org/jdk/pull/9958
