STORM-1255: address pr comments
Project: http://git-wip-us.apache.org/repos/asf/storm/repo Commit: http://git-wip-us.apache.org/repos/asf/storm/commit/d912d50a Tree: http://git-wip-us.apache.org/repos/asf/storm/tree/d912d50a Diff: http://git-wip-us.apache.org/repos/asf/storm/diff/d912d50a Branch: refs/heads/master Commit: d912d50a7fcb82883543e004f801490cf41865a2 Parents: a8edd51 Author: Alessandro Bellina <[email protected]> Authored: Thu Feb 18 22:15:42 2016 -0600 Committer: Alessandro Bellina <[email protected]> Committed: Thu Feb 18 22:15:42 2016 -0600 ---------------------------------------------------------------------- .../src/jvm/org/apache/storm/utils/Time.java | 1 + .../jvm/org/apache/storm/utils/TimeTest.java | 52 +++++++++++--------- .../jvm/org/apache/storm/utils/UtilsTest.java | 14 ++++-- 3 files changed, 41 insertions(+), 26 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/storm/blob/d912d50a/storm-core/src/jvm/org/apache/storm/utils/Time.java ---------------------------------------------------------------------- diff --git a/storm-core/src/jvm/org/apache/storm/utils/Time.java b/storm-core/src/jvm/org/apache/storm/utils/Time.java index fd01fb8..1b36070 100644 --- a/storm-core/src/jvm/org/apache/storm/utils/Time.java +++ b/storm-core/src/jvm/org/apache/storm/utils/Time.java @@ -127,6 +127,7 @@ public class Time { public static void advanceTime(long ms) { if(!simulating.get()) throw new IllegalStateException("Cannot simulate time unless in simulation mode"); + if(ms < 0) throw new IllegalArgumentException("advanceTime only accepts positive time as an argument"); simulatedCurrTimeMs.set(simulatedCurrTimeMs.get() + ms); } http://git-wip-us.apache.org/repos/asf/storm/blob/d912d50a/storm-core/test/jvm/org/apache/storm/utils/TimeTest.java ---------------------------------------------------------------------- diff --git a/storm-core/test/jvm/org/apache/storm/utils/TimeTest.java b/storm-core/test/jvm/org/apache/storm/utils/TimeTest.java index 354095c..d27b4b8 100644 --- a/storm-core/test/jvm/org/apache/storm/utils/TimeTest.java +++ b/storm-core/test/jvm/org/apache/storm/utils/TimeTest.java @@ -34,7 +34,7 @@ public class TimeTest { } @Test(expected=IllegalStateException.class) - public void ifNotSimulatingAdvanceTimeThrows() { + public void ifNotSimulatingAdvanceTimeThrowsTest() { Time.advanceTime(1000); } @@ -42,39 +42,47 @@ public class TimeTest { public void isSimulatingReturnsTrueDuringSimulationTest() { Assert.assertFalse(Time.isSimulating()); Time.startSimulating(); - Assert.assertTrue(Time.isSimulating()); - Time.stopSimulating(); + try { + Assert.assertTrue(Time.isSimulating()); + } finally { + Time.stopSimulating(); + } } @Test public void shouldNotAdvanceTimeTest() { Time.startSimulating(); - long current = Time.currentTimeMillis(); - Time.advanceTime(0); - Assert.assertEquals(Time.deltaMs(current), 0); - Time.stopSimulating(); + try{ + long current = Time.currentTimeMillis(); + Time.advanceTime(0); + Assert.assertEquals(Time.deltaMs(current), 0); + } finally { + Time.stopSimulating(); + } } @Test public void shouldAdvanceForwardTest() { Time.startSimulating(); - long current = Time.currentTimeMillis(); - Time.advanceTime(1000); - Assert.assertEquals(Time.deltaMs(current), 1000); - Time.advanceTime(500); - Assert.assertEquals(Time.deltaMs(current), 1500); - Time.stopSimulating(); + try { + long current = Time.currentTimeMillis(); + Time.advanceTime(1000); + Assert.assertEquals(Time.deltaMs(current), 1000); + Time.advanceTime(500); + Assert.assertEquals(Time.deltaMs(current), 1500); + } finally { + Time.stopSimulating(); + } } - @Test - public void shouldAdvanceBackwardsTest() { + @Test(expected=IllegalArgumentException.class) + public void shouldThrowIfAttemptToAdvanceBackwardsTest() { Time.startSimulating(); - long current = Time.currentTimeMillis(); - Time.advanceTime(1000); - Assert.assertEquals(Time.deltaMs(current), 1000); - Time.advanceTime(-1500); - Assert.assertEquals(Time.deltaMs(current), -500); - Time.stopSimulating(); + try { + Time.advanceTime(-1500); + } finally { + Time.stopSimulating(); + } } @Test @@ -87,7 +95,7 @@ public class TimeTest { } @Test - public void deltaSecsTruncatesFractionalSeconds() { + public void deltaSecsTruncatesFractionalSecondsTest() { Time.startSimulating(); int current = Time.currentTimeSecs(); Time.advanceTime(1500); http://git-wip-us.apache.org/repos/asf/storm/blob/d912d50a/storm-core/test/jvm/org/apache/storm/utils/UtilsTest.java ---------------------------------------------------------------------- diff --git a/storm-core/test/jvm/org/apache/storm/utils/UtilsTest.java b/storm-core/test/jvm/org/apache/storm/utils/UtilsTest.java index 8583a16..a74522a 100644 --- a/storm-core/test/jvm/org/apache/storm/utils/UtilsTest.java +++ b/storm-core/test/jvm/org/apache/storm/utils/UtilsTest.java @@ -54,11 +54,18 @@ public class UtilsTest { Assert.assertEquals(policy.getSleepTimeMs(10, 0), expectedCeiling); } - @Test(expected = RuntimeException.class) - public void getConfiguredClientThrowsRuntimeExceptionOnBadArgsTest () throws RuntimeException, TTransportException { + public void getConfiguredClientThrowsRuntimeExceptionOnBadArgsTest () throws TTransportException { Map config = ConfigUtils.readStormConfig(); config.put(Config.STORM_NIMBUS_RETRY_TIMES, 0); - new NimbusClient(config, "", 65535); + + try { + new NimbusClient(config, "", 65535); + Assert.fail("Expected exception to be thrown"); + } catch (RuntimeException e){ + Assert.assertTrue( + "Cause is not TTransportException " + e, + Utils.exceptionCauseIsInstanceOf(TTransportException.class, e)); + } } private Map mockMap(String key, String value) { @@ -124,7 +131,6 @@ public class UtilsTest { try { System.setProperty("java.security.auth.login.config", "anything"); Assert.assertTrue(Utils.isZkAuthenticationConfiguredStormServer(emptyMockMap())); - } catch (Exception ignore) { } finally { // reset property if (oldValue == null) {
