Forráskód Böngészése

ZOOKEEPER-3113: EphemeralType.get() fails to verify ephemeralOwner when currentElapsedTime() is small enough

I refactored the unit test `testServerIds` to verify server id verification code explicitly instead of through `EphemeralType.get()` method.

Reasons:
- The original test doesn't work on machines which booted recently, because the generated `Time.currentElapsedTime()` value is not high enough and it's possible to generate a valid ephemeralOwner even with high 0xff byte. Server ID cannot be verified reliably this way.
- EphemeralType.get() is already covered in other unit tests,
- Unit tests should test the smallest piece of logic and call the method under testing directly.

Author: Andor Molnar <andor@apache.org>

Reviewers: fangmin@apache.org, eolivelli@gmail.com, hanm@apache.org, andor@apache.org

Closes #651 from anmolnar/ZOOKEEPER-3113 and squashes the following commits:

7454d7e7 [Andor Molnar] ZOOKEEPER-3113. Added unit tests to cover EphemeralType.get edge cases
47dece7a [Andor Molnar] ZOOKEEPER-3113. Refactored unit test to validate server Id verification code explicitly
Andor Molnar 6 éve
szülő
commit
5c85a236c9

+ 12 - 0
zookeeper-server/src/test/java/org/apache/zookeeper/server/Emulate353TTLTest.java

@@ -29,6 +29,8 @@ import org.junit.Test;
 
 import java.util.concurrent.atomic.AtomicLong;
 
+import static org.hamcrest.CoreMatchers.equalTo;
+
 public class Emulate353TTLTest extends ClientBase {
     private TestableZooKeeper zk;
 
@@ -83,6 +85,16 @@ public class Emulate353TTLTest extends ClientBase {
         Assert.assertNull("Ttl node should have been deleted", zk.exists("/foo", false));
     }
 
+    @Test
+    public void testEphemeralOwner_emulationTTL() {
+        Assert.assertThat(EphemeralType.get(-1), equalTo(EphemeralType.TTL));
+    }
+
+    @Test
+    public void testEphemeralOwner_emulationContainer() {
+        Assert.assertThat(EphemeralType.get(EphemeralType.CONTAINER_EPHEMERAL_OWNER), equalTo(EphemeralType.CONTAINER));
+    }
+
     private ContainerManager newContainerManager(final AtomicLong fakeElapsed) {
         return new ContainerManager(serverFactory.getZooKeeperServer()
                 .getZKDatabase(), serverFactory.getZooKeeperServer().firstProcessor, 1, 100) {

+ 21 - 6
zookeeper-server/src/test/java/org/apache/zookeeper/server/EphemeralTypeTest.java

@@ -24,6 +24,8 @@ import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
+import static org.hamcrest.CoreMatchers.equalTo;
+
 public class EphemeralTypeTest {
     @Before
     public void setUp() {
@@ -70,15 +72,28 @@ public class EphemeralTypeTest {
 
     @Test
     public void testServerIds() {
-        for ( int i = 0; i < 255; ++i ) {
-            Assert.assertEquals(EphemeralType.NORMAL, EphemeralType.get(SessionTrackerImpl.initializeNextSession(i)));
+        for ( int i = 0; i <= EphemeralType.MAX_EXTENDED_SERVER_ID; ++i ) {
+            EphemeralType.validateServerId(i);
         }
         try {
-            Assert.assertEquals(EphemeralType.TTL, EphemeralType.get(SessionTrackerImpl.initializeNextSession(255)));
-            Assert.fail("Should have thrown IllegalArgumentException");
-        } catch (IllegalArgumentException e) {
+            EphemeralType.validateServerId(EphemeralType.MAX_EXTENDED_SERVER_ID + 1);
+            Assert.fail("Should have thrown RuntimeException");
+        } catch (RuntimeException e) {
             // expected
         }
-        Assert.assertEquals(EphemeralType.NORMAL, EphemeralType.get(SessionTrackerImpl.initializeNextSession(EphemeralType.MAX_EXTENDED_SERVER_ID)));
+    }
+
+    @Test
+    public void testEphemeralOwner_extendedFeature_TTL() {
+        // 0xff = Extended feature is ON
+        // 0x0000 = Extended type id TTL (0)
+        Assert.assertThat(EphemeralType.get(0xff00000000000000L), equalTo(EphemeralType.TTL));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testEphemeralOwner_extendedFeature_extendedTypeUnsupported() {
+        // 0xff = Extended feature is ON
+        // 0x0001 = Unsupported extended type id (1)
+        EphemeralType.get(0xff00010000000000L);
     }
 }