Test PluginManager.removePermission

Static methods are death to testability.  However, irrelevant static
methods can be negotiated with until a later time in which they can be
removed.  When instantiating a new Permission object, static calls are
made to the Bukkit class during a recalculatePermissibles logic path.
This recalculatePermissibles call should probably be moved
appropriately, but until the time such testing can be accomplished
itself, these tests work around that situation by simply verifying the
static Bukkit server references are satisfied since what is called as
a result is irrelevant currently.

This commit also updates a few other tests for PluginManagerTest to
work towards the standard of using the Hamcrest unit testing library.

2 files changed, 129 insertions, 25 deletions

diff --git a/src/test/java/org/bukkit/TestServer.java b/src/test/java/org/bukkit/TestServer.java
index 63e8aeff..c521e2b3 100644
--- a/src/test/java/org/bukkit/TestServer.java
+++ b/src/test/java/org/bukkit/TestServer.java
@@ -1,41 +1,93 @@
 package org.bukkit;

 

-import java.lang.reflect.Constructor;

 import java.lang.reflect.InvocationHandler;

 import java.lang.reflect.Method;

 import java.lang.reflect.Proxy;

-import java.util.HashMap;

+import java.util.Map;

+import java.util.logging.Logger;

 

+import org.bukkit.command.SimpleCommandMap;

+import org.bukkit.plugin.PluginManager;

+import org.bukkit.plugin.SimplePluginManager;

+

+import com.google.common.collect.ImmutableMap;

 

 public class TestServer implements InvocationHandler {

     private static interface MethodHandler {

         Object handle(TestServer server, Object[] args);

     }

-    private static final Constructor<? extends Server> constructor;

-    private static final HashMap<Method, MethodHandler> methods = new HashMap<Method, MethodHandler>();

+

+    private static final Map<Method, MethodHandler> methods;

+

     static {

         try {

-            methods.put(Server.class.getMethod("isPrimaryThread"),

-                new MethodHandler() {

-                    public Object handle(TestServer server, Object[] args) {

-                        return Thread.currentThread().equals(server.creatingThread);

+            ImmutableMap.Builder<Method, MethodHandler> methodMap = ImmutableMap.builder();

+            methodMap.put(

+                    Server.class.getMethod("isPrimaryThread"),

+                    new MethodHandler() {

+                        public Object handle(TestServer server, Object[] args) {

+                            return Thread.currentThread().equals(server.creatingThread);

+                        }

+                    }

+                );

+            methodMap.put(

+                    Server.class.getMethod("getPluginManager"),

+                    new MethodHandler() {

+                        public Object handle(TestServer server, Object[] args) {

+                            return server.pluginManager;

+                        }

+                    }

+                );

+            methodMap.put(

+                    Server.class.getMethod("getLogger"),

+                    new MethodHandler() {

+                        final Logger logger = Logger.getLogger(TestServer.class.getCanonicalName());

+                        public Object handle(TestServer server, Object[] args) {

+                            return logger;

+                        }

+                    }

+                );

+            methodMap.put(

+                    Server.class.getMethod("getName"),

+                    new MethodHandler() {

+                        public Object handle(TestServer server, Object[] args) {

+                            return TestServer.class.getSimpleName();

+                        }

                     }

-                });

-            constructor = Proxy.getProxyClass(Server.class.getClassLoader(), Server.class).asSubclass(Server.class).getConstructor(InvocationHandler.class);

+                );

+            methodMap.put(

+                    Server.class.getMethod("getVersion"),

+                    new MethodHandler() {

+                        public Object handle(TestServer server, Object[] args) {

+                            return "Version_" + TestServer.class.getPackage().getImplementationVersion();

+                        }

+                    }

+                );

+            methodMap.put(

+                    Server.class.getMethod("getBukkitVersion"),

+                    new MethodHandler() {

+                        public Object handle(TestServer server, Object[] args) {

+                            return "BukkitVersion_" + TestServer.class.getPackage().getImplementationVersion();

+                        }

+                    }

+                );

+            methods = methodMap.build();

+

+            TestServer server = new TestServer();

+            Server instance = Proxy.getProxyClass(Server.class.getClassLoader(), Server.class).asSubclass(Server.class).getConstructor(InvocationHandler.class).newInstance(server);

+            Bukkit.setServer(instance);

+            server.pluginManager = new SimplePluginManager(instance, new SimpleCommandMap(instance));

         } catch (Throwable t) {

             throw new Error(t);

         }

     }

 

     private Thread creatingThread = Thread.currentThread();

+    private PluginManager pluginManager;

     private TestServer() {};

 

     public static Server getInstance() {

-        try {

-            return constructor.newInstance(new TestServer());

-        } catch (Throwable t) {

-            throw new RuntimeException(t);

-        }

+        return Bukkit.getServer();

     }

 

     public Object invoke(Object proxy, Method method, Object[] args) {

diff --git a/src/test/java/org/bukkit/plugin/PluginManagerTest.java b/src/test/java/org/bukkit/plugin/PluginManagerTest.java
index b5cb7408..c4a240ee 100644
--- a/src/test/java/org/bukkit/plugin/PluginManagerTest.java
+++ b/src/test/java/org/bukkit/plugin/PluginManagerTest.java
@@ -1,12 +1,14 @@
 package org.bukkit.plugin;

 

-import static junit.framework.Assert.*;

+import static org.hamcrest.Matchers.*;

+import static org.junit.Assert.*;

 

-import org.bukkit.Server;

 import org.bukkit.TestServer;

-import org.bukkit.command.SimpleCommandMap;

 import org.bukkit.event.Event;

 import org.bukkit.event.TestEvent;

+import org.bukkit.permissions.Permission;

+

+import org.junit.After;

 import org.junit.Test;

 

 public class PluginManagerTest {

@@ -14,10 +16,9 @@ public class PluginManagerTest {
         volatile Object value = null;

     }

 

-    final Server server = TestServer.getInstance();

-    final SimpleCommandMap commandMap = new SimpleCommandMap(server);

-    final PluginManager pm = new SimplePluginManager(server, commandMap);

-    final MutableObject store = new MutableObject();

+    private static final PluginManager pm = TestServer.getInstance().getPluginManager();

+    

+    private final MutableObject store = new MutableObject();

 

     @Test

     public void testAsyncSameThread() {

@@ -25,7 +26,7 @@ public class PluginManagerTest {
         try {

             pm.callEvent(event);

         } catch (IllegalStateException ex) {

-            assertEquals(event.getEventName() + " cannot be triggered asynchronously from primary server thread.", ex.getMessage());

+            assertThat(event.getEventName() + " cannot be triggered asynchronously from primary server thread.", is(ex.getMessage()));

             return;

         }

         throw new IllegalStateException("No exception thrown");

@@ -53,8 +54,8 @@ public class PluginManagerTest {
                 }});

         secondThread.start();

         secondThread.join();

-        assertTrue(store.value instanceof IllegalStateException);

-        assertEquals(event.getEventName() + " cannot be triggered asynchronously from inside synchronized code.", ((Throwable) store.value).getMessage());

+        assertThat(store.value, is(instanceOf(IllegalStateException.class)));

+        assertThat(event.getEventName() + " cannot be triggered asynchronously from inside synchronized code.", is(((Throwable) store.value).getMessage()));

     }

 

     @Test

@@ -115,4 +116,55 @@ public class PluginManagerTest {
             throw new RuntimeException((Throwable) store.value);

         }

     }

+

+    @Test

+    public void testRemovePermissionByNameLower() {

+        this.testRemovePermissionByName("lower");

+    }

+

+    @Test

+    public void testRemovePermissionByNameUpper() {

+        this.testRemovePermissionByName("UPPER");

+    }

+

+    @Test

+    public void testRemovePermissionByNameCamel() {

+        this.testRemovePermissionByName("CaMeL");

+    }

+

+    public void testRemovePermissionByPermissionLower() {

+        this.testRemovePermissionByPermission("lower");

+    }

+

+    @Test

+    public void testRemovePermissionByPermissionUpper() {

+        this.testRemovePermissionByPermission("UPPER");

+    }

+

+    @Test

+    public void testRemovePermissionByPermissionCamel() {

+        this.testRemovePermissionByPermission("CaMeL");

+    }

+

+    private void testRemovePermissionByName(final String name) {

+        final Permission perm = new Permission(name);

+        pm.addPermission(perm);

+        assertThat("Permission \"" + name + "\" was not added", pm.getPermission(name), is(perm));

+        pm.removePermission(name);

+        assertThat("Permission \"" + name + "\" was not removed", pm.getPermission(name), is(nullValue()));

+    }

+

+    private void testRemovePermissionByPermission(final String name) {

+        final Permission perm = new Permission(name);

+        pm.addPermission(perm);

+        assertThat("Permission \"" + name + "\" was not added", pm.getPermission(name), is(perm));

+        pm.removePermission(perm);

+        assertThat("Permission \"" + name + "\" was not removed", pm.getPermission(name), is(nullValue()));

+    }

+    

+    @After

+    public void tearDown() {

+        pm.clearPlugins();

+        assertThat(pm.getPermissions(), is(empty()));

+    }

 }