From 7924c2ed589b25fee91a3fbac0d1251dcf1b12f7 Mon Sep 17 00:00:00 2001 From: cpovirk Date: Fri, 17 Apr 2026 08:43:59 -0700 Subject: [PATCH] Fix https://errorprone.info/bugpattern/NullArgumentForNonNullParameter findings in test. (Creating an actual `AbstractService` instance is [not free but hardly prohibitively expensive](http://screen/889xnnVf7tEspvW).) And perform assorted other cleanups. RELNOTES=n/a PiperOrigin-RevId: 901317906 --- .../AbstractScheduledServiceTest.java | 49 ++++++++++++------- .../concurrent/AbstractScheduledService.java | 4 +- .../AbstractScheduledServiceTest.java | 49 ++++++++++++------- .../concurrent/AbstractScheduledService.java | 4 +- 4 files changed, 66 insertions(+), 40 deletions(-) diff --git a/android/guava-tests/test/com/google/common/util/concurrent/AbstractScheduledServiceTest.java b/android/guava-tests/test/com/google/common/util/concurrent/AbstractScheduledServiceTest.java index 3feed9814f7d..3f3b25f4b276 100644 --- a/android/guava-tests/test/com/google/common/util/concurrent/AbstractScheduledServiceTest.java +++ b/android/guava-tests/test/com/google/common/util/concurrent/AbstractScheduledServiceTest.java @@ -60,11 +60,11 @@ public class AbstractScheduledServiceTest extends TestCase { volatile Scheduler configuration = newFixedDelaySchedule(0, 10, MILLISECONDS); - volatile @Nullable ScheduledFuture future = null; + volatile @Nullable ScheduledFuture future; - volatile boolean atFixedRateCalled = false; - volatile boolean withFixedDelayCalled = false; - volatile boolean scheduleCalled = false; + volatile boolean atFixedRateCalled; + volatile boolean withFixedDelayCalled; + volatile boolean scheduleCalled; final ScheduledExecutorService executor = new ScheduledThreadPoolExecutor(10) { @@ -83,7 +83,7 @@ public void testServiceStartStop() throws Exception { assertTrue(future.isCancelled()); } - private class NullService extends AbstractScheduledService { + private final class NullService extends AbstractScheduledService { @Override protected void runOneIteration() throws Exception {} @@ -98,6 +98,18 @@ protected ScheduledExecutorService executor() { } } + private static final class NullAbstractService extends AbstractService { + @Override + protected void doStart() { + notifyStarted(); + } + + @Override + protected void doStop() { + notifyStopped(); + } + } + public void testFailOnExceptionFromRun() throws Exception { TestService service = new TestService(); service.runException = new Exception(); @@ -295,18 +307,18 @@ protected String serviceName() { .isEqualTo("Timed out waiting for Foo [STARTING] to reach the RUNNING state."); } - private class TestService extends AbstractScheduledService { + private final class TestService extends AbstractScheduledService { final CyclicBarrier runFirstBarrier = new CyclicBarrier(2); final CyclicBarrier runSecondBarrier = new CyclicBarrier(2); - volatile boolean startUpCalled = false; - volatile boolean shutDownCalled = false; + volatile boolean startUpCalled; + volatile boolean shutDownCalled; final AtomicInteger numberOfTimesRunCalled = new AtomicInteger(0); final AtomicInteger numberOfTimesExecutorCalled = new AtomicInteger(0); final AtomicInteger numberOfTimesSchedulerCalled = new AtomicInteger(0); - volatile @Nullable Exception runException = null; - volatile @Nullable Exception startUpException = null; - volatile @Nullable Exception shutDownException = null; + volatile @Nullable Exception runException; + volatile @Nullable Exception startUpException; + volatile @Nullable Exception shutDownException; @Override protected void runOneIteration() throws Exception { @@ -371,7 +383,7 @@ protected Scheduler scheduler() { public void run() {} }; - boolean called = false; + boolean called; private void assertSingleCallWithCorrectParameters( Runnable command, long initialDelay, long delay, TimeUnit unit) { @@ -387,7 +399,7 @@ public void testFixedRateSchedule() { Scheduler schedule = Scheduler.newFixedRateSchedule(INITIAL_DELAY, DELAY, UNIT); Cancellable unused = schedule.schedule( - null, + new NullAbstractService(), new ScheduledThreadPoolExecutor(1) { @Override public ScheduledFuture scheduleAtFixedRate( @@ -404,7 +416,7 @@ public void testFixedDelaySchedule() { Scheduler schedule = newFixedDelaySchedule(INITIAL_DELAY, DELAY, UNIT); Cancellable unused = schedule.schedule( - null, + new NullAbstractService(), new ScheduledThreadPoolExecutor(10) { @Override public ScheduledFuture scheduleWithFixedDelay( @@ -472,8 +484,8 @@ protected Schedule getNextSchedule() throws Exception { service.awaitTerminated(); } - private static class TestCustomScheduler extends AbstractScheduledService.CustomScheduler { - private final AtomicInteger scheduleCounter = new AtomicInteger(0); + private static final class TestCustomScheduler extends AbstractScheduledService.CustomScheduler { + final AtomicInteger scheduleCounter = new AtomicInteger(0); @Override protected Schedule getNextSchedule() throws Exception { @@ -498,7 +510,8 @@ public void testCustomSchedule_startStop() throws Exception { } }; TestCustomScheduler scheduler = new TestCustomScheduler(); - Cancellable future = scheduler.schedule(null, newScheduledThreadPool(10), task); + Cancellable future = + scheduler.schedule(new NullAbstractService(), newScheduledThreadPool(10), task); firstBarrier.await(); assertEquals(1, scheduler.scheduleCounter.get()); secondBarrier.await(); @@ -627,7 +640,7 @@ public void testCustomSchedulerFailure() throws Exception { assertThat(service.state()).isEqualTo(State.FAILED); } - private static class TestFailingCustomScheduledService extends AbstractScheduledService { + private static final class TestFailingCustomScheduledService extends AbstractScheduledService { final AtomicInteger numIterations = new AtomicInteger(0); final CyclicBarrier firstBarrier = new CyclicBarrier(2); final CyclicBarrier secondBarrier = new CyclicBarrier(2); diff --git a/android/guava/src/com/google/common/util/concurrent/AbstractScheduledService.java b/android/guava/src/com/google/common/util/concurrent/AbstractScheduledService.java index c016e75e508d..606bf1b8665a 100644 --- a/android/guava/src/com/google/common/util/concurrent/AbstractScheduledService.java +++ b/android/guava/src/com/google/common/util/concurrent/AbstractScheduledService.java @@ -149,7 +149,7 @@ public static Scheduler newFixedDelaySchedule(long initialDelay, long delay, Tim checkArgument(delay > 0, "delay must be > 0, found %s", delay); return new Scheduler() { @Override - public Cancellable schedule( + Cancellable schedule( AbstractService service, ScheduledExecutorService executor, Runnable task) { return new FutureAsCancellable( executor.scheduleWithFixedDelay(task, initialDelay, delay, unit)); @@ -185,7 +185,7 @@ public static Scheduler newFixedRateSchedule(long initialDelay, long period, Tim checkArgument(period > 0, "period must be > 0, found %s", period); return new Scheduler() { @Override - public Cancellable schedule( + Cancellable schedule( AbstractService service, ScheduledExecutorService executor, Runnable task) { return new FutureAsCancellable( executor.scheduleAtFixedRate(task, initialDelay, period, unit)); diff --git a/guava-tests/test/com/google/common/util/concurrent/AbstractScheduledServiceTest.java b/guava-tests/test/com/google/common/util/concurrent/AbstractScheduledServiceTest.java index 3feed9814f7d..3f3b25f4b276 100644 --- a/guava-tests/test/com/google/common/util/concurrent/AbstractScheduledServiceTest.java +++ b/guava-tests/test/com/google/common/util/concurrent/AbstractScheduledServiceTest.java @@ -60,11 +60,11 @@ public class AbstractScheduledServiceTest extends TestCase { volatile Scheduler configuration = newFixedDelaySchedule(0, 10, MILLISECONDS); - volatile @Nullable ScheduledFuture future = null; + volatile @Nullable ScheduledFuture future; - volatile boolean atFixedRateCalled = false; - volatile boolean withFixedDelayCalled = false; - volatile boolean scheduleCalled = false; + volatile boolean atFixedRateCalled; + volatile boolean withFixedDelayCalled; + volatile boolean scheduleCalled; final ScheduledExecutorService executor = new ScheduledThreadPoolExecutor(10) { @@ -83,7 +83,7 @@ public void testServiceStartStop() throws Exception { assertTrue(future.isCancelled()); } - private class NullService extends AbstractScheduledService { + private final class NullService extends AbstractScheduledService { @Override protected void runOneIteration() throws Exception {} @@ -98,6 +98,18 @@ protected ScheduledExecutorService executor() { } } + private static final class NullAbstractService extends AbstractService { + @Override + protected void doStart() { + notifyStarted(); + } + + @Override + protected void doStop() { + notifyStopped(); + } + } + public void testFailOnExceptionFromRun() throws Exception { TestService service = new TestService(); service.runException = new Exception(); @@ -295,18 +307,18 @@ protected String serviceName() { .isEqualTo("Timed out waiting for Foo [STARTING] to reach the RUNNING state."); } - private class TestService extends AbstractScheduledService { + private final class TestService extends AbstractScheduledService { final CyclicBarrier runFirstBarrier = new CyclicBarrier(2); final CyclicBarrier runSecondBarrier = new CyclicBarrier(2); - volatile boolean startUpCalled = false; - volatile boolean shutDownCalled = false; + volatile boolean startUpCalled; + volatile boolean shutDownCalled; final AtomicInteger numberOfTimesRunCalled = new AtomicInteger(0); final AtomicInteger numberOfTimesExecutorCalled = new AtomicInteger(0); final AtomicInteger numberOfTimesSchedulerCalled = new AtomicInteger(0); - volatile @Nullable Exception runException = null; - volatile @Nullable Exception startUpException = null; - volatile @Nullable Exception shutDownException = null; + volatile @Nullable Exception runException; + volatile @Nullable Exception startUpException; + volatile @Nullable Exception shutDownException; @Override protected void runOneIteration() throws Exception { @@ -371,7 +383,7 @@ protected Scheduler scheduler() { public void run() {} }; - boolean called = false; + boolean called; private void assertSingleCallWithCorrectParameters( Runnable command, long initialDelay, long delay, TimeUnit unit) { @@ -387,7 +399,7 @@ public void testFixedRateSchedule() { Scheduler schedule = Scheduler.newFixedRateSchedule(INITIAL_DELAY, DELAY, UNIT); Cancellable unused = schedule.schedule( - null, + new NullAbstractService(), new ScheduledThreadPoolExecutor(1) { @Override public ScheduledFuture scheduleAtFixedRate( @@ -404,7 +416,7 @@ public void testFixedDelaySchedule() { Scheduler schedule = newFixedDelaySchedule(INITIAL_DELAY, DELAY, UNIT); Cancellable unused = schedule.schedule( - null, + new NullAbstractService(), new ScheduledThreadPoolExecutor(10) { @Override public ScheduledFuture scheduleWithFixedDelay( @@ -472,8 +484,8 @@ protected Schedule getNextSchedule() throws Exception { service.awaitTerminated(); } - private static class TestCustomScheduler extends AbstractScheduledService.CustomScheduler { - private final AtomicInteger scheduleCounter = new AtomicInteger(0); + private static final class TestCustomScheduler extends AbstractScheduledService.CustomScheduler { + final AtomicInteger scheduleCounter = new AtomicInteger(0); @Override protected Schedule getNextSchedule() throws Exception { @@ -498,7 +510,8 @@ public void testCustomSchedule_startStop() throws Exception { } }; TestCustomScheduler scheduler = new TestCustomScheduler(); - Cancellable future = scheduler.schedule(null, newScheduledThreadPool(10), task); + Cancellable future = + scheduler.schedule(new NullAbstractService(), newScheduledThreadPool(10), task); firstBarrier.await(); assertEquals(1, scheduler.scheduleCounter.get()); secondBarrier.await(); @@ -627,7 +640,7 @@ public void testCustomSchedulerFailure() throws Exception { assertThat(service.state()).isEqualTo(State.FAILED); } - private static class TestFailingCustomScheduledService extends AbstractScheduledService { + private static final class TestFailingCustomScheduledService extends AbstractScheduledService { final AtomicInteger numIterations = new AtomicInteger(0); final CyclicBarrier firstBarrier = new CyclicBarrier(2); final CyclicBarrier secondBarrier = new CyclicBarrier(2); diff --git a/guava/src/com/google/common/util/concurrent/AbstractScheduledService.java b/guava/src/com/google/common/util/concurrent/AbstractScheduledService.java index d0195135b4c2..7ea7f080ff2f 100644 --- a/guava/src/com/google/common/util/concurrent/AbstractScheduledService.java +++ b/guava/src/com/google/common/util/concurrent/AbstractScheduledService.java @@ -148,7 +148,7 @@ public static Scheduler newFixedDelaySchedule(long initialDelay, long delay, Tim checkArgument(delay > 0, "delay must be > 0, found %s", delay); return new Scheduler() { @Override - public Cancellable schedule( + Cancellable schedule( AbstractService service, ScheduledExecutorService executor, Runnable task) { return new FutureAsCancellable( executor.scheduleWithFixedDelay(task, initialDelay, delay, unit)); @@ -183,7 +183,7 @@ public static Scheduler newFixedRateSchedule(long initialDelay, long period, Tim checkArgument(period > 0, "period must be > 0, found %s", period); return new Scheduler() { @Override - public Cancellable schedule( + Cancellable schedule( AbstractService service, ScheduledExecutorService executor, Runnable task) { return new FutureAsCancellable( executor.scheduleAtFixedRate(task, initialDelay, period, unit));