You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@storm.apache.org by bo...@apache.org on 2016/02/23 21:20:00 UTC

[5/8] storm git commit: STORM-1255: address pr comments

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 <ab...@yahoo-inc.com>
Authored: Thu Feb 18 22:15:42 2016 -0600
Committer: Alessandro Bellina <ab...@yahoo-inc.com>
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) {