Fix flaky service test. The whole AsyncService thing probably should just be rm'd, but fixing the test for now.

Revision created by MOE tool push_codebase.
MOE_MIGRATION=3475


git-svn-id: https://google-guice.googlecode.com/svn/trunk@1591 d779f126-a31b-0410-b53b-1d3aecad763e

3 files changed, 10 insertions, 6 deletions

diff --git a/extensions/persist/test/log4j.properties b/extensions/persist/src/log4j.properties
index 1654ecd0..1654ecd0 100644
--- a/extensions/persist/test/log4j.properties
+++ b/extensions/persist/src/log4j.properties
diff --git a/extensions/service/test/com/google/inject/service/SingleServiceIntegrationTest.java b/extensions/service/test/com/google/inject/service/SingleServiceIntegrationTest.java
index ca36b472..9463b166 100644
--- a/extensions/service/test/com/google/inject/service/SingleServiceIntegrationTest.java
+++ b/extensions/service/test/com/google/inject/service/SingleServiceIntegrationTest.java
@@ -7,6 +7,7 @@ import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -17,7 +18,7 @@ import java.util.concurrent.atomic.AtomicInteger;
 
 public class SingleServiceIntegrationTest extends TestCase {
 
-  public final void testAsyncServiceLifecycle() throws InterruptedException {
+  public final void testAsyncServiceLifecycle() throws Exception {
     ExecutorService executor = Executors.newSingleThreadExecutor();
 
     final CountDownLatch startLatch = new CountDownLatch(1);
@@ -38,9 +39,14 @@ public class SingleServiceIntegrationTest extends TestCase {
       }
     };
 
-    service.start();
-    // This should not pass!
+    Future<?> future = service.start();
+    // This should not pass!  TODO(sameb): Why?  Looks like it should to me
     assertTrue(startLatch.await(2, TimeUnit.SECONDS));
+    // onStart() is called before the state is set to STARTED, so we need
+    // to wait until the Future finishes to guarantee it really was started.
+    // This still manages to test what we want because the startLatch check
+    // is before this.
+    future.get(1, TimeUnit.SECONDS);
 
     service.stop();
     assertTrue(stopLatch.await(2, TimeUnit.SECONDS));
diff --git a/extensions/servlet/src/com/google/inject/servlet/GuiceFilter.java b/extensions/servlet/src/com/google/inject/servlet/GuiceFilter.java
index d9a2ad31..4b602b5e 100644
--- a/extensions/servlet/src/com/google/inject/servlet/GuiceFilter.java
+++ b/extensions/servlet/src/com/google/inject/servlet/GuiceFilter.java
@@ -78,8 +78,6 @@ public class GuiceFilter implements Filter {
       + "in your web application. If this is deliberate, you may safely "
       + "ignore this message. If this is NOT deliberate however, "
       + "your application may not work as expected.";
-  
-  private static final Logger LOGGER = Logger.getLogger(GuiceFilter.class.getName());
 
   //VisibleForTesting
   @Inject
@@ -88,7 +86,7 @@ public class GuiceFilter implements Filter {
     // This can happen if you create many injectors and they all have their own
     // servlet module. This is legal, caveat a small warning.
     if (GuiceFilter.pipeline instanceof ManagedFilterPipeline) {
-      LOGGER.warning(MULTIPLE_INJECTORS_WARNING);
+        Logger.getLogger(GuiceFilter.class.getName()).warning(MULTIPLE_INJECTORS_WARNING);
     }
 
     // We overwrite the default pipeline