Android: Fix Cucumber execution on Gradle (#1094)

* Fix AndroidInstrumentationReporter for Gradle builds

* the connected check tasks of the Android/Gradle build system do not like non-unique test names
* we add a unique index to the test names if they are non-unique (e.g. on scenario outlines with multiple examples)
* for this bug fix, we provide a unit test

* Fix AndroidInstrumentationReporter for Gradle builds

* the connected check tasks of the Android/Gradle build system do not like non-unique test names
* we add a unique index to the test names if they are non-unique (e.g. on scenario outlines with multiple examples)
* for this bug fix, we provide a unit test

This is a re-integration of PR-1094. The original pull request code was performed on 1.2.6-SNAPSHOT base. This code bases on version 2.3.2-SNAPSHOT.

* Update Cukeulator example project to work with newest Android build tools

- Update to Android Studio 3.0.1, SDK 26+ and Gradle 4.1
- Replace Instrumentation class by CucumberRunner
- CucumberRunner uses AndroidJUnitRunner (JUnit 4+)
- CalculationSteps class uses ActivityTestRule instead of deprecated ActivityInstrumentationTestCase2
- Fix permissions to write reports on internal storage

* Improve Cukeulator example project

* Update README.md
* Enable local Maven dependencies for better development experience
* Describe, how to use Cukeulator example project with locally built Cucumber-JVM

* Rename duplicated test case names in AndroidInstrumentationReporter like JUnitFormatter in cucumber-core

* Fix typo

* Improve readability of AndroidInstrumentationReporterTest

* Share common logic for test case name between JUnitFormatter and AndroidInstrumentationReporter

* Improve code quality

- Create merged method calculateUniqueTestName from getUniqueTestName and ensureUniqueTestName and make it private
- Use better test method name: test_case_names_are_unique_on_equal_scenario_names (instead of scenario_outline_all_test_names_unique)
- Refactor test code: now it should be readable

* Change misleading variable names

* [Android] Split up test of making test names unique.

To provide better documentation of the functionality, that is:
* test names within feature are made unique by appending blank and
  number
* test names within are made unique by appending underscore and number
  when no blank in name
* test names in different features can be the same
* test names are made unique also when not consecutive

Author: gnuechtel

android/src/main/java/cucumber/runtime/formatter/AndroidInstrumentationReporter.java

@@ -12,9 +12,15 @@
 import cucumber.api.event.TestStepFinished;
 import cucumber.api.formatter.Formatter;
 import cucumber.runtime.Runtime;
+import cucumber.runtime.Utils;
 
 import java.io.PrintWriter;
 import java.io.StringWriter;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.IdentityHashMap;
+import java.util.Map;
+import java.util.Set;
 
 /**
  * Reports the test results to the instrumentation through {@link Instrumentation#sendStatus(int, Bundle)} calls.
@@ -170,7 +176,8 @@ void startTestCase(final TestCase testCase) {
             currentUri = testCase.getUri();
             currentFeatureName = testSources.getFeatureName(currentUri);
         }
-        currentTestCaseName = testCase.getName();
+        // Since the names of test cases are not guaranteed to be unique, we must check for unique names
+        currentTestCaseName = calculateUniqueTestName(testCase);
         resetSeverestResult();
         final Bundle testStart = createBundle(currentFeatureName, currentTestCaseName);
         instrumentation.sendStatus(StatusCodes.START, testStart);
@@ -277,4 +284,51 @@ private static String getStackTrace(final Throwable throwable) {
         throwable.printStackTrace(printWriter);
         return stringWriter.getBuffer().toString();
     }
+
+    /**
+     * The stored unique test name for a test case.
+     * We use an identity hash-map since we want to distinct all test case objects.
+     * Thus, the key is a unique test case object.<br/>
+     * The mapped value is the unique test name, which maybe differs from test case original non-unique name.
+     */
+    private final Map<TestCase, String> uniqueTestNameForTestCase = new IdentityHashMap<TestCase, String>();
+
+    /**
+     * The stored unique test names grouped by feature.<br/>
+     * The key contains the feature file.<br/>
+     * The mapped value is a set of unique test names for the feature.
+     */
+    private final Map<String, Set<String>> uniqueTestNamesForFeature = new HashMap<String, Set<String>>();
+
+    /**
+     * Creates a unique test name for the given test case by filling the internal maps
+     * {@link #uniqueTestNameForTestCase} and {@link #uniqueTestNamesForFeature}.<br/>
+     * If the test case name is unique, it will be used, otherwise, a index will be added " 2", " 3", " 4", ...
+     * @param testCase the test case
+     * @return a unique test name
+     */
+    private String calculateUniqueTestName(TestCase testCase) {
+        String existingName = uniqueTestNameForTestCase.get(testCase);
+        if (existingName != null) {
+            // Nothing to do: there is already a test name for the passed test case object
+            return existingName;
+        }
+        final String feature = testCase.getUri();
+        String uniqueTestCaseName = testCase.getName();
+        if (!uniqueTestNamesForFeature.containsKey(feature)) {
+            // First test case of the feature
+            uniqueTestNamesForFeature.put(feature, new HashSet<String>());
+        }
+        final Set<String> uniqueTestNamesSetForFeature = uniqueTestNamesForFeature.get(feature);
+        // If "name" already exists, the next one is "name_2" or "name with spaces 2"
+        int i = 2;
+        while (uniqueTestNamesSetForFeature.contains(uniqueTestCaseName)) {
+            uniqueTestCaseName = Utils.getUniqueTestNameForScenarioExample(testCase.getName(), i);
+            i++;
+        }
+        uniqueTestNamesSetForFeature.add(uniqueTestCaseName);
+        uniqueTestNameForTestCase.put(testCase, uniqueTestCaseName);
+        return uniqueTestNameForTestCase.get(testCase);
+    }
+
 }

android/src/test/java/cucumber/runtime/formatter/AndroidInstrumentationReporterTest.java

@@ -1,6 +1,8 @@
 package cucumber.runtime.formatter;
 
+import static java.util.Arrays.asList;
 import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.equalTo;
 import static org.junit.Assert.assertThat;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.eq;
@@ -28,6 +30,8 @@
 import org.robolectric.RobolectricTestRunner;
 import org.robolectric.annotation.Config;
 
+import java.util.List;
+
 @Config(manifest = Config.NONE)
 @RunWith(RobolectricTestRunner.class)
 public class AndroidInstrumentationReporterTest {
@@ -458,8 +462,118 @@ public void step_result_contains_only_the_current_scenarios_severest_result() {
         inOrder.verify(instrumentation).sendStatus(eq(StatusCodes.OK), secondCaptor.capture());
     }
 
+    @Test
+    public void test_names_within_feature_are_made_unique_by_appending_blank_and_number() {
+
+        // given
+        final AndroidInstrumentationReporter formatter = new AndroidInstrumentationReporter(runtime, instrumentation);
+        TestCase testCase1 = mockTestCase(testCaseName("not unique name"));
+        TestCase testCase2 = mockTestCase(testCaseName("not unique name"));
+        TestCase testCase3 = mockTestCase(testCaseName("not unique name"));
+
+        // when
+        simulateRunningTestCases(formatter, asList(testCase1, testCase2, testCase3));
+
+        // then
+        final InOrder inOrder = inOrder(instrumentation);
+        assertThat(captureTestName(inOrder, instrumentation), equalTo("not unique name"));
+        assertThat(captureTestName(inOrder, instrumentation), equalTo("not unique name 2"));
+        assertThat(captureTestName(inOrder, instrumentation), equalTo("not unique name 3"));
+    }
+
+    @Test
+    public void test_names_within_are_made_unique_by_appending_underscore_and_number_when_no_blank_in_name() {
+
+        // given
+        final AndroidInstrumentationReporter formatter = new AndroidInstrumentationReporter(runtime, instrumentation);
+        TestCase testCase1 = mockTestCase(testCaseName("not_unique_name"));
+        TestCase testCase2 = mockTestCase(testCaseName("not_unique_name"));
+        TestCase testCase3 = mockTestCase(testCaseName("not_unique_name"));
+
+        // when
+        simulateRunningTestCases(formatter, asList(testCase1, testCase2, testCase3));
+
+        // then
+        final InOrder inOrder = inOrder(instrumentation);
+        assertThat(captureTestName(inOrder, instrumentation), equalTo("not_unique_name"));
+        assertThat(captureTestName(inOrder, instrumentation), equalTo("not_unique_name_2"));
+        assertThat(captureTestName(inOrder, instrumentation), equalTo("not_unique_name_3"));
+    }
+
+    @Test
+    public void test_names_in_different_features_can_be_the_same() {
+
+        // given
+        final AndroidInstrumentationReporter formatter = new AndroidInstrumentationReporter(runtime, instrumentation);
+        TestCase testCase1 = mockTestCase(featureUri("path/file1.feature"), testCaseName("not unique name"));
+        TestCase testCase2 = mockTestCase(featureUri("path/file2.feature"), testCaseName("not unique name"));
+
+        // when
+        simulateRunningTestCases(formatter, asList(testCase1, testCase2));
+
+        // then
+        final InOrder inOrder = inOrder(instrumentation);
+        assertThat(captureTestName(inOrder, instrumentation), equalTo("not unique name"));
+        assertThat(captureTestName(inOrder, instrumentation), equalTo("not unique name"));
+    }
+
+    @Test
+    public void test_names_are_made_unique_also_when_not_consecutive() {
+
+        // given
+        final AndroidInstrumentationReporter formatter = new AndroidInstrumentationReporter(runtime, instrumentation);
+        TestCase testCase1 = mockTestCase(testCaseName("not unique name"));
+        TestCase testCase2 = mockTestCase(testCaseName("unique name"));
+        TestCase testCase3 = mockTestCase(testCaseName("not unique name"));
+
+        // when
+        simulateRunningTestCases(formatter, asList(testCase1, testCase2, testCase3));
+
+        // then
+        final InOrder inOrder = inOrder(instrumentation);
+        assertThat(captureTestName(inOrder, instrumentation), equalTo("not unique name"));
+        assertThat(captureTestName(inOrder, instrumentation), equalTo("unique name"));
+        assertThat(captureTestName(inOrder, instrumentation), equalTo("not unique name 2"));
+    }
+
     private void mockResultStatus(Result result, Result.Type status) {
         when(result.getStatus()).thenReturn(status);
         when(result.is(Result.Type.PASSED)).thenReturn(status == Result.Type.PASSED);
     }
+
+    private TestCase mockTestCase(String testCaseName) {
+        return mockTestCase(featureUri("path/file.feature"), testCaseName);
+    }
+
+    private TestCase mockTestCase(String featureUri, String testCaseName) {
+        TestCase testCase = mock(TestCase.class);
+        when(testCase.getUri()).thenReturn(featureUri);
+        when(testCase.getName()).thenReturn(testCaseName);
+        return testCase;
+    }
+
+    private String testCaseName(String name) {
+        return name;
+    }
+
+    private String featureUri(String uri) {
+        return uri;
+    }
+
+    private void simulateRunningTestCases(AndroidInstrumentationReporter formatter, List<TestCase> testCases) {
+        mockResultStatus(firstResult, Result.Type.PASSED);
+        formatter.setNumberOfTests(testCases.size());
+        for (TestCase testCase : testCases) {
+            formatter.startTestCase(testCase);
+            formatter.finishTestStep(firstResult);
+            formatter.finishTestCase();
+        }
+    }
+
+    private String captureTestName(InOrder inOrder, Instrumentation intrumentation) {
+        final ArgumentCaptor<Bundle> captor = ArgumentCaptor.forClass(Bundle.class);
+        inOrder.verify(instrumentation).sendStatus(eq(StatusCodes.START), captor.capture());
+        final Bundle actualBundle = captor.getValue();
+        return actualBundle.getString(AndroidInstrumentationReporter.StatusKeys.TEST);
+    }
 }

core/src/main/java/cucumber/runtime/Utils.java

@@ -8,7 +8,6 @@
 import java.lang.reflect.TypeVariable;
 import java.net.MalformedURLException;
 import java.net.URL;
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 
@@ -128,4 +127,13 @@ public static String htmlEscape(String s) {
                 .replace("'", "'")
                 .replace("/", "/");
     }
+
+    public static String getUniqueTestNameForScenarioExample(String testCaseName, int exampleNumber) {
+        return testCaseName + (includesBlank(testCaseName) ? " " : "_") + exampleNumber;
+    }
+
+    private static boolean includesBlank(String testCaseName) {
+        return testCaseName.indexOf(' ') != -1;
+    }
+
 }

core/src/main/java/cucumber/runtime/formatter/JUnitFormatter.java

@@ -12,10 +12,9 @@
 import cucumber.api.formatter.Formatter;
 import cucumber.api.formatter.StrictAware;
 import cucumber.runtime.CucumberException;
+import cucumber.runtime.Utils;
 import cucumber.runtime.io.URLOutputStream;
 import cucumber.runtime.io.UTF8OutputStreamWriter;
-import gherkin.GherkinDialect;
-import gherkin.GherkinDialectProvider;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
 import org.w3c.dom.NodeList;
@@ -28,10 +27,8 @@
 import javax.xml.transform.TransformerFactory;
 import javax.xml.transform.dom.DOMSource;
 import javax.xml.transform.stream.StreamResult;
-
 import java.io.Closeable;
 import java.io.IOException;
-import java.io.OutputStream;
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.io.Writer;
@@ -233,18 +230,14 @@ private void writeElement(Document doc, Element tc) {
         private String calculateElementName(cucumber.api.TestCase testCase) {
             String testCaseName = testCase.getName();
             if (testCaseName.equals(previousTestCaseName)) {
-                return testCaseName + (includesBlank(testCaseName) ? " " : "_") + ++exampleNumber;
+                return Utils.getUniqueTestNameForScenarioExample(testCaseName, ++exampleNumber);
             } else {
                 previousTestCaseName = testCase.getName();
                 exampleNumber = 1;
                 return testCaseName;
             }
         }
 
-        private boolean includesBlank(String testCaseName) {
-            return testCaseName.indexOf(' ') != -1;
-        }
-
         public void addTestCaseElement(Document doc, Element tc, Result result) {
             tc.setAttribute("time", calculateTotalDurationString(result));
 

examples/android/android-studio/Cukeulator/README.md

@@ -1,5 +1,5 @@
 ## Cukeulator Example Test
-This is the example test-project for the Cukeulator app for Android Studio 1.0.2.
+This is the example test-project for the Cukeulator app for Android Studio 3.0+
 
 ### Setup
 Features must be placed in `assets/features/`. Subdirectories are allowed.
@@ -67,3 +67,7 @@ adb shell am instrument -w cucumber.cukeulator.test/cucumber.cukeulator.test.Ins
 
 ### Output
 Filter for the logcat tag `cucumber-android` in [DDMS](https://developer.android.com/tools/debugging/ddms.html).
+
+### Using this project with locally built Cucumber-JVM
+See [app/build.gradle](app/build.gradle) under `dependencies`.  
+There is a source-code comment which explains how to use a locally built Cucumber-JVM Android library.