Revision: 6708 Author: [email protected] Date: Thu Nov 5 11:47:39 2009 Log: tr...@6707 was merged into this branch Show which browser failed when a browser dies a mysterious death. svn merge --ignore-ancestry -c r6707 https://google-web-toolkit.googlecode.com/svn/trunk .
Patch by: jlabanca http://code.google.com/p/google-web-toolkit/source/detail?r=6708 Modified: /releases/2.0/branch-info.txt /releases/2.0/user/src/com/google/gwt/junit/JUnitMessageQueue.java /releases/2.0/user/src/com/google/gwt/junit/JUnitShell.java /releases/2.0/user/src/com/google/gwt/junit/RunStyle.java /releases/2.0/user/src/com/google/gwt/junit/RunStyleExternalBrowser.java /releases/2.0/user/src/com/google/gwt/junit/RunStyleRemoteWeb.java /releases/2.0/user/src/com/google/gwt/junit/RunStyleSelenium.java /releases/2.0/user/test/com/google/gwt/junit/CompileStrategyTest.java /releases/2.0/user/test/com/google/gwt/junit/JUnitMessageQueueTest.java ======================================= --- /releases/2.0/branch-info.txt Thu Nov 5 11:34:16 2009 +++ /releases/2.0/branch-info.txt Thu Nov 5 11:47:39 2009 @@ -362,3 +362,8 @@ tr...@6616 was merged into this branch Clarify JsniCollector.SourceCache comment svn merge --ignore-ancestry -c 6616 https://google-web-toolkit.googlecode.com/svn/trunk . + +tr...@6707 was merged into this branch + Show which browser failed when a browser dies a mysterious death. + svn merge --ignore-ancestry -c r6707 https://google-web-toolkit.googlecode.com/svn/trunk . + ======================================= --- /releases/2.0/user/src/com/google/gwt/junit/JUnitMessageQueue.java Wed Nov 4 17:44:00 2009 +++ /releases/2.0/user/src/com/google/gwt/junit/JUnitMessageQueue.java Thu Nov 5 11:47:39 2009 @@ -75,7 +75,7 @@ /** * The number of TestCase clients executing in parallel. */ - private final int numClients; + private int numClients = 1; /** * Maps the TestInfo to the results from each clientId. If JUnitResult is @@ -97,14 +97,8 @@ /** * Only instantiable within this package. - * - * @param numClients The number of parallel clients being served by this - * queue. */ - JUnitMessageQueue(int numClients) { - synchronized (clientStatusesLock) { - this.numClients = numClients; - } + JUnitMessageQueue() { } /** @@ -427,6 +421,22 @@ testResults.remove(testInfo); } } + + /** + * Set the number of clients. This must be called before adding any tests. + * + * @param numClients The number of parallel clients being served by this + * queue. + */ + void setNumClients(int numClients) { + synchronized (clientStatusesLock) { + if (testBlocks.size() > 0) { + throw new IllegalStateException( + "Cannot change number of clients after adding tests"); + } + this.numClients = numClients; + } + } void waitForResults(int millis) { synchronized (clientStatusesLock) { ======================================= --- /releases/2.0/user/src/com/google/gwt/junit/JUnitShell.java Thu Nov 5 06:53:33 2009 +++ /releases/2.0/user/src/com/google/gwt/junit/JUnitShell.java Thu Nov 5 11:47:39 2009 @@ -237,7 +237,8 @@ @Override public boolean setString(String runStyleArg) { - return createRunStyle(runStyleArg); + runStyleName = runStyleArg; + return true; } }); @@ -565,11 +566,7 @@ if (!argProcessor.processArgs(args)) { throw new JUnitFatalLaunchException("Error processing shell arguments"); } - if (unitTestShell.runStyle == null) { - unitTestShell.createRunStyle("HtmlUnit"); - } - unitTestShell.messageQueue = new JUnitMessageQueue( - unitTestShell.numClients); + unitTestShell.messageQueue = new JUnitMessageQueue(); if (!unitTestShell.startUp()) { throw new JUnitFatalLaunchException("Shell failed to start"); @@ -667,6 +664,12 @@ */ private RunStyle runStyle = null; + /** + * The argument passed to -runStyle. This is parsed later so we can pass in + * a logger. + */ + private String runStyleName = "HtmlUnit"; + private boolean shouldAutoGenerateResources = true; private boolean standardsMode = false; @@ -729,6 +732,11 @@ if (!super.doStartup()) { return false; } + if (!createRunStyle(runStyleName)) { + // RunStyle already logged reasons for its failure + return false; + } + if (!runStyle.setupMode(getTopLogger(), developmentMode)) { getTopLogger().log(TreeLogger.ERROR, "Run style does not support " + (developmentMode ? "development" : "production") + " mode"); @@ -813,8 +821,16 @@ + "\n Actual time elapsed: " + elapsed + " seconds.\n"); } - if (runStyle.wasInterrupted()) { - throw new TimeoutException("A remote browser died a mysterious death."); + // Check that we haven't lost communication with a remote host. + String[] interruptedHosts = runStyle.getInterruptedHosts(); + if (interruptedHosts != null) { + StringBuilder msg = new StringBuilder(); + msg.append("A remote browser died a mysterious death.\n"); + msg.append(" We lost communication with:"); + for (String host : interruptedHosts) { + msg.append("\n ").append(host); + } + throw new TimeoutException(msg.toString()); } if (messageQueue.hasResults(currentTestInfo)) { @@ -877,6 +893,7 @@ */ void setNumClients(int numClients) { this.numClients = numClients; + messageQueue.setNumClients(numClients); } void setStandardsMode(boolean standardsMode) { ======================================= --- /releases/2.0/user/src/com/google/gwt/junit/RunStyle.java Wed Nov 4 17:44:00 2009 +++ /releases/2.0/user/src/com/google/gwt/junit/RunStyle.java Thu Nov 5 11:47:39 2009 @@ -38,6 +38,15 @@ public RunStyle(JUnitShell shell) { this.shell = shell; } + + /** + * Tests whether the test was interrupted. + * + * @return the interrupted hosts, or null if not interrupted + */ + public String[] getInterruptedHosts() { + return null; + } /** * Returns the number of times this test should be tried to run. A test @@ -89,15 +98,6 @@ public boolean shouldAutoGenerateResources() { return true; } - - /** - * Tests whether the test was interrupted. - * - * @return <code>true</code> if the test has been interrupted. - */ - public boolean wasInterrupted() { - return false; - } /** * Gets the shell logger. ======================================= --- /releases/2.0/user/src/com/google/gwt/junit/RunStyleExternalBrowser.java Mon Nov 2 12:44:54 2009 +++ /releases/2.0/user/src/com/google/gwt/junit/RunStyleExternalBrowser.java Thu Nov 5 11:47:39 2009 @@ -18,6 +18,9 @@ import com.google.gwt.core.ext.TreeLogger; import com.google.gwt.core.ext.UnableToCompleteException; +import java.util.HashSet; +import java.util.Set; + /** * Runs in web mode via browsers managed as an external process. This feature is * experimental and is not officially supported. @@ -72,6 +75,26 @@ public RunStyleExternalBrowser(JUnitShell shell) { super(shell); } + + @Override + public String[] getInterruptedHosts() { + // Make sure all browsers are still running + Set<String> toRet = null; + for (ExternalBrowser browser : externalBrowsers) { + try { + browser.getProcess().exitValue(); + + // The host exited, so return its path. + if (toRet == null) { + toRet = new HashSet<String>(); + } + toRet.add(browser.getPath()); + } catch (IllegalThreadStateException e) { + // The process is still active, keep looking. + } + } + return toRet == null ? null : toRet.toArray(new String[toRet.size()]); + } @Override public boolean initialize(String args) { @@ -82,6 +105,7 @@ return false; } String browsers[] = args.split(","); + shell.setNumClients(browsers.length); synchronized (this) { this.externalBrowsers = new ExternalBrowser[browsers.length]; for (int i = 0; i < browsers.length; ++i) { @@ -118,20 +142,4 @@ browser.setProcess(child); } } - - @Override - public boolean wasInterrupted() { - - // Make sure all browsers are still running - for (ExternalBrowser browser : externalBrowsers) { - try { - browser.getProcess().exitValue(); - } catch (IllegalThreadStateException e) { - // The process is still active, keep looking. - continue; - } - return true; - } - return false; - } -} +} ======================================= --- /releases/2.0/user/src/com/google/gwt/junit/RunStyleRemoteWeb.java Fri Oct 30 14:33:17 2009 +++ /releases/2.0/user/src/com/google/gwt/junit/RunStyleRemoteWeb.java Thu Nov 5 11:47:39 2009 @@ -27,6 +27,8 @@ import java.net.SocketTimeoutException; import java.rmi.Naming; import java.rmi.server.RMISocketFactory; +import java.util.HashSet; +import java.util.Set; /** * Runs in web mode via browsers managed over RMI. This feature is experimental @@ -134,13 +136,13 @@ private RemoteBrowser[] remoteBrowsers; /** - * Whether one of the remote browsers was interrupted. + * The list of hosts that were interrupted. */ - private boolean wasInterrupted; + private Set<String> interruptedHosts; /** - * A separate lock to control access to {...@link #wasInterrupted}. This keeps - * the main thread calls into {...@link #wasInterrupted()} from having to + * A separate lock to control access to {...@link #interruptedHosts}. This keeps + * the main thread calls into {...@link #getInterruptedHosts()} from having to * synchronized on the containing instance and potentially block on RPC calls. * It is okay to take the {...@link #wasInterruptedLock} while locking the * containing instance; it is NOT okay to do the opposite or deadlock could @@ -154,7 +156,17 @@ public RunStyleRemoteWeb(JUnitShell shell) { super(shell); } - + + @Override + public String[] getInterruptedHosts() { + synchronized (wasInterruptedLock) { + if (interruptedHosts == null) { + return null; + } + return interruptedHosts.toArray(new String[interruptedHosts.size()]); + } + } + @Override public boolean initialize(String args) { if (args == null || args.length() == 0) { @@ -246,13 +258,6 @@ keepAliveThread.setDaemon(true); keepAliveThread.start(); } - - @Override - public boolean wasInterrupted() { - synchronized (wasInterruptedLock) { - return wasInterrupted; - } - } private synchronized boolean doKeepAlives() { for (RemoteBrowser remoteBrowser : remoteBrowsers) { @@ -278,7 +283,10 @@ } remoteBrowser.setToken(0); synchronized (wasInterruptedLock) { - wasInterrupted = true; + if (interruptedHosts == null) { + interruptedHosts = new HashSet<String>(); + } + interruptedHosts.add(remoteBrowser.getRmiUrl()); } break; } @@ -286,7 +294,7 @@ } synchronized (wasInterruptedLock) { - return !wasInterrupted; + return interruptedHosts == null; } } } ======================================= --- /releases/2.0/user/src/com/google/gwt/junit/RunStyleSelenium.java Mon Nov 2 12:44:54 2009 +++ /releases/2.0/user/src/com/google/gwt/junit/RunStyleSelenium.java Thu Nov 5 11:47:39 2009 @@ -24,6 +24,8 @@ import java.net.InetAddress; import java.net.UnknownHostException; +import java.util.HashSet; +import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -68,13 +70,13 @@ private RCSelenium remotes[]; /** - * Whether one of the remote browsers was interrupted. + * The list of hosts that were interrupted. */ - private boolean wasInterrupted; + private Set<String> interruptedHosts; /** - * A separate lock to control access to {...@link #wasInterrupted}. This keeps - * the main thread calls into {...@link #wasInterrupted()} from having to be + * A separate lock to control access to {...@link #interruptedHosts}. This keeps + * the main thread calls into {...@link #getInterruptedHosts()} from having to be * synchronized on the containing instance and potentially block on RPC calls. * It is okay to take the {...@link #wasInterruptedLock} while locking the * containing instance; it is NOT okay to do the opposite or deadlock could @@ -85,7 +87,17 @@ public RunStyleSelenium(final JUnitShell shell) { super(shell); } - + + @Override + public String[] getInterruptedHosts() { + synchronized (wasInterruptedLock) { + if (interruptedHosts == null) { + return null; + } + return interruptedHosts.toArray(new String[interruptedHosts.size()]); + } + } + @Override public boolean initialize(String args) { if (args == null || args.length() == 0) { @@ -159,13 +171,6 @@ } } } - - @Override - public boolean wasInterrupted() { - synchronized (wasInterruptedLock) { - return wasInterrupted; - } - } /** * Create the keep-alive thread. @@ -199,17 +204,17 @@ remote.getSelenium().getTitle(); } } catch (Throwable e) { - setWasInterrupted(true); + synchronized (wasInterruptedLock) { + if (interruptedHosts == null) { + interruptedHosts = new HashSet<String>(); + } + interruptedHosts.add(remote.getHost() + ":" + remote.getPort() + + "/" + remote.getBrowser()); + } } } } - return !wasInterrupted(); - } - - private void setWasInterrupted(boolean wasInterrupted) { - synchronized (wasInterruptedLock) { - this.wasInterrupted = wasInterrupted; - } + return interruptedHosts == null; } } ======================================= --- /releases/2.0/user/test/com/google/gwt/junit/CompileStrategyTest.java Tue Oct 13 16:57:19 2009 +++ /releases/2.0/user/test/com/google/gwt/junit/CompileStrategyTest.java Thu Nov 5 11:47:39 2009 @@ -63,7 +63,7 @@ private boolean isLastBlock; public MockJUnitMessageQueue() { - super(1); + super(); } @Override ======================================= --- /releases/2.0/user/test/com/google/gwt/junit/JUnitMessageQueueTest.java Wed Nov 4 17:44:00 2009 +++ /releases/2.0/user/test/com/google/gwt/junit/JUnitMessageQueueTest.java Thu Nov 5 11:47:39 2009 @@ -36,7 +36,8 @@ private final static int ONE_TEST_PER_BLOCK = 1; public void testAddTestBlocks() { - JUnitMessageQueue queue = new JUnitMessageQueue(10); + JUnitMessageQueue queue = new JUnitMessageQueue(); + queue.setNumClients(10); assertEquals(0, queue.getTestBlocks().size()); List<TestInfo[]> expectedBlocks = new ArrayList<TestInfo[]>(); @@ -457,6 +458,17 @@ // check that the updated result appears now. assertTrue(queue.getResults(testInfo).get("client0").getException() == null); } + + public void testSetNumClients() { + JUnitMessageQueue queue = new JUnitMessageQueue(); + queue.addTestBlocks(createTestBlocks(ONE_BLOCK, ONE_TEST_PER_BLOCK), true); + try { + queue.setNumClients(2); + fail("Expected IllegalStateException"); + } catch (IllegalStateException e) { + // expected. + } + } /** * Assert that two arrays are the same size and contain the same elements. @@ -489,7 +501,8 @@ */ private JUnitMessageQueue createQueue(int numClients, int numBlocks, int testsPerBlock) { - JUnitMessageQueue queue = new JUnitMessageQueue(numClients); + JUnitMessageQueue queue = new JUnitMessageQueue(); + queue.setNumClients(numClients); queue.addTestBlocks(createTestBlocks(numBlocks, testsPerBlock), true); return queue; } --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
