From b405e0417151915b98c3d2033adb9770336a7bbb Mon Sep 17 00:00:00 2001 From: "guice.mirrorbot@gmail.com" Date: Sun, 16 Oct 2011 22:34:21 +0000 Subject: 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 --- core/src/com/google/inject/Guice.java | 6 ++---- core/src/com/google/inject/spi/ConvertedConstantBinding.java | 2 +- extensions/persist/src/log4j.properties | 6 ++++++ extensions/persist/test/log4j.properties | 6 ------ .../google/inject/service/SingleServiceIntegrationTest.java | 12 +++++++++--- .../servlet/src/com/google/inject/servlet/GuiceFilter.java | 4 +--- 6 files changed, 19 insertions(+), 17 deletions(-) create mode 100644 extensions/persist/src/log4j.properties delete mode 100644 extensions/persist/test/log4j.properties diff --git a/core/src/com/google/inject/Guice.java b/core/src/com/google/inject/Guice.java index 703187f8..9b21193b 100644 --- a/core/src/com/google/inject/Guice.java +++ b/core/src/com/google/inject/Guice.java @@ -52,8 +52,7 @@ public final class Guice { private Guice() {} /** - * Creates an injector for the given set of modules. This is equivalent to - * calling {@link #createInjector(Stage, Module...)} with Stage.DEVELOPMENT. + * Creates an injector for the given set of modules. * * @throws CreationException if one or more errors occur during injector * construction @@ -63,8 +62,7 @@ public final class Guice { } /** - * Creates an injector for the given set of modules. This is equivalent to - * calling {@link #createInjector(Stage, Iterable)} with Stage.DEVELOPMENT. + * Creates an injector for the given set of modules. * * @throws CreationException if one or more errors occur during injector * creation diff --git a/core/src/com/google/inject/spi/ConvertedConstantBinding.java b/core/src/com/google/inject/spi/ConvertedConstantBinding.java index 97db6402..d4ae073a 100644 --- a/core/src/com/google/inject/spi/ConvertedConstantBinding.java +++ b/core/src/com/google/inject/spi/ConvertedConstantBinding.java @@ -43,7 +43,7 @@ public interface ConvertedConstantBinding extends Binding, HasDependencies TypeConverterBinding getTypeConverterBinding(); /** - * Returns the key for the source binding. That binding can be retrieved from an injector using + * Returns the key for the source binding. That binding can e retrieved from an injector using * {@link com.google.inject.Injector#getBinding(Key) Injector.getBinding(key)}. */ Key getSourceKey(); diff --git a/extensions/persist/src/log4j.properties b/extensions/persist/src/log4j.properties new file mode 100644 index 00000000..1654ecd0 --- /dev/null +++ b/extensions/persist/src/log4j.properties @@ -0,0 +1,6 @@ +log4j.rootLogger=warn, console + +log4j.appender.console=org.apache.log4j.ConsoleAppender +log4j.appender.console.Target=System.out +log4j.appender.console.layout=org.apache.log4j.PatternLayout +log4j.appender.console.layout.ConversionPattern=%p [%c{1}] - %m %n diff --git a/extensions/persist/test/log4j.properties b/extensions/persist/test/log4j.properties deleted file mode 100644 index 1654ecd0..00000000 --- a/extensions/persist/test/log4j.properties +++ /dev/null @@ -1,6 +0,0 @@ -log4j.rootLogger=warn, console - -log4j.appender.console=org.apache.log4j.ConsoleAppender -log4j.appender.console.Target=System.out -log4j.appender.console.layout=org.apache.log4j.PatternLayout -log4j.appender.console.layout.ConversionPattern=%p [%c{1}] - %m %n 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 -- cgit v1.2.3