fix: improves method name handling executors(#215)

* Use the logical and not the actual test method name and escape reserved characters in test method names when composing urls in ServletMethodExecutor.
* Fix two incorrect implementations of of TestMethodExecutor.getMethodName()
* Consistently use TestMethodExecutor.getMethodName() instead of TestMethodExecutor.getMethod().getName()

Fixes #214

Author: erdi

container/test-impl-base/src/test/java/org/jboss/arquillian/container/test/impl/ContainerEventControllerTestCase.java

@@ -174,6 +174,11 @@ public void shouldInvokeTestInContainerDeploymentContext() throws Exception {
             public void invoke(Object... parameters) throws Throwable {
             }
 
+            @Override
+            public String getMethodName() {
+                return getMethod().getName();
+            }
+
             @Override
             public Method getMethod() {
                 return testMethod();
@@ -198,6 +203,11 @@ public void shouldNotInvokeTestInContainerDeploymentContextIfNoDeploymentFound()
             public void invoke(Object... parameters) throws Throwable {
             }
 
+            @Override
+            public String getMethodName() {
+                return getMethod().getName();
+            }
+
             @Override
             public Method getMethod() {
                 return testMethod();
@@ -220,6 +230,11 @@ public void shouldThrowExceptionIfTryingToOperateOnANonExistingContext() throws
             public void invoke(Object... parameters) throws Throwable {
             }
 
+            @Override
+            public String getMethodName() {
+                return getMethod().getName();
+            }
+
             @Override
             public Method getMethod() {
                 return nonExistingOperatesOnDeploymentMethod();

junit/core/src/main/java/org/jboss/arquillian/junit/MethodInvoker.java

@@ -18,6 +18,11 @@ public void invoke(Object... parameters) throws Throwable {
                 invokeMethod(parameters);
             }
 
+            @Override
+            public String getMethodName() {
+                return method.getName();
+            }
+
             public Method getMethod() {
                 return method.getMethod();
             }

protocols/jmx/src/main/java/org/jboss/arquillian/protocol/jmx/JMXMethodExecutor.java

@@ -64,7 +64,7 @@ public TestResult invoke(TestMethodExecutor testMethodExecutor) {
         }
 
         String testClass = testMethodExecutor.getInstance().getClass().getName();
-        String testMethod = testMethodExecutor.getMethod().getName();
+        String testMethod = testMethodExecutor.getMethodName();
         String testCanonicalName = testClass + "." + testMethod;
 
         NotificationListener commandListener = null;
@@ -125,4 +125,4 @@ public void handleNotification(Notification notification, Object handback) {
             }
         }
     }
-}
+}

protocols/jmx/src/test/java/org/jboss/arquillian/protocol/jmx/JMXTestRunnerTestCase.java

@@ -151,6 +151,11 @@ public void shouldBeAbleToSendReceiveCommands() throws Throwable {
                 public void invoke(Object... parameters) throws Throwable {
                 }
 
+                @Override
+                public String getMethodName() {
+                    return getMethod().getName();
+                }
+
                 @Override
                 public Method getMethod() {
                     return testMethod();

protocols/servlet/src/main/java/org/jboss/arquillian/protocol/servlet/ServletMethodExecutor.java

@@ -23,6 +23,7 @@
 import java.net.URI;
 import java.net.URL;
 import java.net.URLConnection;
+import java.net.URLEncoder;
 import java.util.Collection;
 import java.util.Timer;
 import java.util.TimerTask;
@@ -75,16 +76,18 @@ public TestResult invoke(final TestMethodExecutor testMethodExecutor) {
         URI targetBaseURI = uriHandler.locateTestServlet(testMethodExecutor.getMethod());
 
         Class<?> testClass = testMethodExecutor.getInstance().getClass();
-        final String url = targetBaseURI.toASCIIString() + ARQUILLIAN_SERVLET_MAPPING
-            + "?outputMode=serializedObject&className=" + testClass.getName() + "&methodName="
-            + testMethodExecutor.getMethod().getName();
-
-        final String eventUrl = targetBaseURI.toASCIIString() + ARQUILLIAN_SERVLET_MAPPING
-            + "?outputMode=serializedObject&className=" + testClass.getName() + "&methodName="
-            + testMethodExecutor.getMethod().getName() + "&cmd=event";
 
         Timer eventTimer = null;
         try {
+            String urlEncodedMethodName = URLEncoder.encode(testMethodExecutor.getMethodName(), "UTF-8");
+            final String url = targetBaseURI.toASCIIString() + ARQUILLIAN_SERVLET_MAPPING
+                + "?outputMode=serializedObject&className=" + testClass.getName() + "&methodName="
+                + urlEncodedMethodName;
+
+            final String eventUrl = targetBaseURI.toASCIIString() + ARQUILLIAN_SERVLET_MAPPING
+                + "?outputMode=serializedObject&className=" + testClass.getName() + "&methodName="
+                + urlEncodedMethodName + "&cmd=event";
+
             eventTimer = createCommandServicePullTimer(eventUrl);
             return executeWithRetry(url, TestResult.class);
         } catch (Exception e) {

protocols/servlet/src/test/java/org/jboss/arquillian/protocol/servlet/AbstractServerBase.java

@@ -136,6 +136,10 @@ public static class MockTestExecutor implements TestMethodExecutor, Serializable
         public void invoke(Object... parameters) throws Throwable {
         }
 
+        public String getMethodName() {
+            return getMethod().getName();
+        }
+
         public Method getMethod() {
             try {
                 return this.getClass().getMethod("getMethod");

protocols/servlet/src/test/java/org/jboss/arquillian/protocol/servlet/ProtocolTestCase.java

@@ -50,6 +50,27 @@ public void shouldReturnTestResult() throws Exception {
             result.getThrowable());
     }
 
+    @Test
+    public void shouldUseLogicalMethodNameAndSupportUrlReservedCharactersInIt() throws Exception {
+        final String testMethodNameWithReservedUrlCharacters = "non standard!test&name";
+
+        MockTestRunner.add(TestResult.passed());
+
+        ServletMethodExecutor executor = createExecutor();
+        executor.invoke(new MockTestExecutor() {
+            @Override
+            public String getMethodName() {
+                return testMethodNameWithReservedUrlCharacters;
+            }
+        });
+
+        Assert.assertEquals(
+            "Should use the logical method name",
+            testMethodNameWithReservedUrlCharacters,
+            MockTestRunner.testRequests.get(0).getMethodName()
+        );
+    }
+
     @Test
     public void shouldReturnThrownException() throws Exception {
         MockTestRunner.add(TestResult.failed(new Exception().fillInStackTrace()));

protocols/servlet/src/test/java/org/jboss/arquillian/protocol/servlet/test/MockTestRunner.java

@@ -37,6 +37,8 @@ public class MockTestRunner implements TestRunner {
     public static List<Command<?>> commands = new ArrayList<Command<?>>();
     public static List<Object> commandResults = new ArrayList<Object>();
 
+    public static List<TestRequest> testRequests = new ArrayList<TestRequest>();
+
     public static void add(Command<?> command) {
         commands.add(command);
     }
@@ -49,13 +51,33 @@ public static void clear() {
         wantedResults = null;
         commands.clear();
         commandResults.clear();
+        testRequests.clear();
     }
 
     public TestResult execute(Class<?> testClass, String methodName) {
+        testRequests.add(new TestRequest(testClass, methodName));
         for (Command<?> command : commands) {
             commandResults.add(new ServletCommandService().execute(command));
         }
 
         return wantedResults;
     }
+
+    public static class TestRequest {
+        private final Class<?> testClass;
+        private final String methodName;
+
+        public TestRequest(Class<?> testClass, String methodName) {
+            this.testClass = testClass;
+            this.methodName = methodName;
+        }
+
+        public Class<?> getTestClass() {
+            return testClass;
+        }
+
+        public String getMethodName() {
+            return methodName;
+        }
+    }
 }

test/spi/src/main/java/org/jboss/arquillian/test/spi/TestMethodExecutor.java

@@ -25,6 +25,12 @@
  * @version $Revision: $
  */
 public interface TestMethodExecutor {
+
+    /**
+     * The method name to invoke
+     */
+    String getMethodName();
+
     /**
      * The method to invoke.
      */

testng/core/src/main/java/org/jboss/arquillian/testng/Arquillian.java

@@ -172,6 +172,10 @@ private void swapWithClassNames(Object[] source) {
                     }
                 }
 
+                public String getMethodName() {
+                    return testResult.getMethod().getMethodName();
+                }
+
                 public Method getMethod() {
                     return testResult.getMethod().getMethod();
                 }