Merge pull request #271 from ovenal/fix_268_issue_step_title_and_para…

…meter_with_comma #268 Fix issue with custom step name when a method parameter contains with comma
serenity-bdd · Jan 22, 2016 · bfa0e6b · bfa0e6b
2 parents 778d78d + adc66f3
commit bfa0e6b
Show file tree

Hide file tree

Showing 8 changed files with 99 additions and 26 deletions.
diff --git a/serenity-core/src/main/java/net/thucydides/core/steps/AnnotatedStepDescription.java b/serenity-core/src/main/java/net/thucydides/core/steps/AnnotatedStepDescription.java
@@ -260,21 +260,15 @@ private String stepName() {
     private String annotatedStepNameWithParameters(String annotatedStepTemplate) {
         String annotatedStepName = annotatedStepTemplate;
 
-        Iterable<String> parameters = getParamatersFrom(description.getName());
         int counter = 0;
-        for(String parameter : parameters) {
+        for(String parameter : description.getArguments()) {
             String token = "{" + counter++ + "}";
             annotatedStepName = StringUtils.replace(annotatedStepName, token, parameter);
 
         }
         return annotatedStepName;
     }
 
-    private Iterable<String> getParamatersFrom(String name) {
-        String parameters = StringUtils.substringAfter(name,":");
-        return Splitter.on(",").trimResults().split(parameters);
-    }
-
     public boolean isAGroup() {
 
         Method testMethod = getTestMethodIfPresent();

diff --git a/serenity-core/src/main/java/net/thucydides/core/steps/ExecutedStepDescription.java b/serenity-core/src/main/java/net/thucydides/core/steps/ExecutedStepDescription.java
@@ -1,9 +1,11 @@
 package net.thucydides.core.steps;
 
+import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import net.thucydides.core.reflection.MethodFinder;
 
 import java.lang.reflect.Method;
+import java.util.List;
 import java.util.Map;
 
 import static net.thucydides.core.util.NameConverter.humanize;
@@ -17,38 +19,42 @@ public class ExecutedStepDescription implements Cloneable {
 
     private final Class<? extends Object> stepsClass;
     private final String name;
+    private final List<String> argumentsList;
     private final Map<String, Object> displayedFields;
     private boolean isAGroup;
 
     private final static Map<String,Object> NO_FIELDS = Maps.newHashMap();
+    private final static List<String> NO_ARGUMENTS = Lists.newArrayList();
 
     protected ExecutedStepDescription(final Class<? extends Object> stepsClass,
                                       final String name,
+                                      List<String> argumentsList,
                                       Map<String, Object> displayedFields,
                                       final boolean isAGroup) {
         this.stepsClass = stepsClass;
         this.name = name;
+        this.argumentsList = argumentsList;
         this.displayedFields = displayedFields;
         this.isAGroup = isAGroup;
     }
 
     protected ExecutedStepDescription(final Class<? extends Object> stepsClass,
                                       final String name,
                                       final boolean isAGroup) {
-        this(stepsClass, name, NO_FIELDS, isAGroup);
+        this(stepsClass, name, NO_ARGUMENTS, NO_FIELDS, isAGroup);
     }
 
     protected ExecutedStepDescription(final Class<? extends Object> stepsClass,
                                       final String name) {
-        this(stepsClass, name, NO_FIELDS, false);
+        this(stepsClass, name, NO_ARGUMENTS, NO_FIELDS, false);
     }
 
     protected ExecutedStepDescription(final String name) {
-        this(null, name, NO_FIELDS, false);
+        this(null, name, NO_ARGUMENTS, NO_FIELDS, false);
     }
 
     public ExecutedStepDescription clone() {
-        return new ExecutedStepDescription(stepsClass, name, displayedFields, isAGroup);
+        return new ExecutedStepDescription(stepsClass, name, argumentsList, displayedFields, isAGroup);
     }
 
     /**
@@ -58,25 +64,42 @@ public Class<? extends Object> getStepClass() {
         return stepsClass;
     }
 
-
     public String getName() {
         return name;
     }
 
+    public List<String> getArguments() {
+        return argumentsList;
+    }
+
     public ExecutedStepDescription withName(String newName) {
         return new ExecutedStepDescription(this.stepsClass, newName, isAGroup);
     }
 
     public ExecutedStepDescription withDisplayedFields(Map<String, Object> displayedFields) {
-        return new ExecutedStepDescription(this.stepsClass, this.name, displayedFields, isAGroup);
+        return new ExecutedStepDescription(this.stepsClass, this.name, this.argumentsList, displayedFields, isAGroup);
     }
 
     /**
      * We might not have the test class provided (e.g. at the end of a test).
      */
     public static ExecutedStepDescription of(final Class<? extends Object> stepsClass,
                                              final String name) {
-        return new ExecutedStepDescription(stepsClass, name, NO_FIELDS, false);
+        return new ExecutedStepDescription(stepsClass, name, NO_ARGUMENTS, NO_FIELDS, false);
+    }
+
+    public static ExecutedStepDescription of(final Class<? extends Object> stepsClass,
+                                             final String name,
+                                             final Object[] arguments) {
+        return new ExecutedStepDescription(stepsClass, name, convertArguments(arguments), NO_FIELDS, false);
+    }
+
+    private static List<String> convertArguments(Object[] arguments) {
+        List<String> listResult = Lists.newArrayList();
+        for (Object argument : arguments) {
+            listResult.add(StepArgumentWriter.readableFormOf(argument));
+        }
+        return listResult;
     }
 
     public static ExecutedStepDescription withTitle(final String name) {

diff --git a/serenity-core/src/main/java/net/thucydides/core/steps/StepInterceptor.java b/serenity-core/src/main/java/net/thucydides/core/steps/StepInterceptor.java
@@ -418,7 +418,7 @@ private void notifyStepSkippedFor(final Method method, final Object[] args)
 
     private void notifyOfStepFailure(final Object object, final Method method, final Object[] args,
                                      final Throwable cause) throws Throwable {
-        ExecutedStepDescription description = ExecutedStepDescription.of(testStepClass, getTestNameFrom(method, args))
+        ExecutedStepDescription description = ExecutedStepDescription.of(testStepClass, getTestNameFrom(method, args), args)
                 .withDisplayedFields(fieldValuesIn(object));
 
         StepFailure failure = new StepFailure(description, cause);
@@ -433,7 +433,7 @@ private boolean shouldThrowExceptionImmediately() {
     }
 
     private void notifyStepStarted(final Object object, final Method method, final Object[] args) {
-        ExecutedStepDescription description = ExecutedStepDescription.of(testStepClass, getTestNameFrom(method, args))
+        ExecutedStepDescription description = ExecutedStepDescription.of(testStepClass, getTestNameFrom(method, args), args)
                 .withDisplayedFields(fieldValuesIn(object));
         StepEventBus.getEventBus().stepStarted(description);
     }
@@ -444,7 +444,7 @@ private Map<String, Object> fieldValuesIn(Object object) {
 
     private void notifySkippedStepStarted(final Object object, final Method method, final Object[] args) {
 
-        ExecutedStepDescription description = ExecutedStepDescription.of(testStepClass, getTestNameFrom(method, args))
+        ExecutedStepDescription description = ExecutedStepDescription.of(testStepClass, getTestNameFrom(method, args), args)
                 .withDisplayedFields(fieldValuesIn(object));
         StepEventBus.getEventBus().skippedStepStarted(description);
     }

diff --git a/serenity-core/src/test/java/net/thucydides/core/steps/WhenDescribingSteps.java b/serenity-core/src/test/java/net/thucydides/core/steps/WhenDescribingSteps.java
@@ -6,6 +6,7 @@
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.nullValue;
 
 
@@ -112,6 +113,25 @@ public void a_description_can_be_cloned() {
         assertThat(clone.getName(), is(description.getName()));
         assertThat(clone.getTestMethod(), is(description.getTestMethod()));
         assertThat(clone.isAGroup(), is(description.isAGroup()));
+        assertThat(clone.getArguments(), is(description.getArguments()));
     }
 
+    @Test
+    public void a_description_can_be_created_without_arguments_list() {
+        ExecutedStepDescription description = new ExecutedStepDescription(SampleTestSteps.class, "a_step");
+
+        assertThat(description.getTestMethod().getName(), is("a_step"));
+        assertThat(description.getArguments(), notNullValue());
+        assertThat(description.getArguments().size(), is(0));
+    }
+
+    @Test
+    public void a_description_can_be_created_with_arguments_list() {
+        ExecutedStepDescription description = ExecutedStepDescription.of(SampleTestSteps.class, "a_step_with_parameters", new Object[] {"Joe"});
+
+        assertThat(description.getTestMethod().getName(), is("a_step_with_parameters"));
+        assertThat(description.getArguments(), notNullValue());
+        assertThat(description.getArguments().size(), is(1));
+        assertThat(description.getArguments().get(0), is("Joe"));
+    }
 }
diff --git a/...ity-core/src/test/java/net/thucydides/core/steps/WhenDescribingStepsUsingAnnotations.java b/...ity-core/src/test/java/net/thucydides/core/steps/WhenDescribingStepsUsingAnnotations.java
@@ -132,7 +132,9 @@ public void a_step_group_can_be_annotated_to_provide_a_more_readable_name() {
 
     @Test
     public void a_step_can_be_annotated_to_provide_a_more_readable_name_including_a_parameter() {
-        ExecutedStepDescription description = new ExecutedStepDescription(SampleTestSteps.class, "a_customized_step_with_parameters: Joe");
+        ExecutedStepDescription description = ExecutedStepDescription.of(SampleTestSteps.class,
+                "a_customized_step_with_parameters: Joe",
+                new Object[]{"Joe"});
 
         AnnotatedStepDescription annotatedStepDescription = AnnotatedStepDescription.from(description);
 
@@ -142,9 +144,10 @@ public void a_step_can_be_annotated_to_provide_a_more_readable_name_including_a_
     @Test
     public void a_step_can_be_annotated_to_provide_a_more_readable_name_including_a_parameter_and_a_field_variable() {
 
-        ExecutedStepDescription description = new ExecutedStepDescription(SampleTestSteps.class,
-                                                                          "a_customized_step_with_parameters_and_fields: Joe")
-                                                                          .withDisplayedFields(ImmutableMap.of("color",(Object)"red"));
+        ExecutedStepDescription description = ExecutedStepDescription.of(SampleTestSteps.class,
+                "a_customized_step_with_parameters_and_fields",
+                new Object[]{"Joe"})
+                .withDisplayedFields(ImmutableMap.of("color", (Object) "red"));
 
         AnnotatedStepDescription annotatedStepDescription = AnnotatedStepDescription.from(description);
 
@@ -155,8 +158,9 @@ public void a_step_can_be_annotated_to_provide_a_more_readable_name_including_a_
     @Test(expected = AssertionError.class)
     public void a_step_can_be_annotated_to_provide_a_more_readable_name_including_a_parameter_and_an_empty_field_variable() {
 
-        ExecutedStepDescription description = new ExecutedStepDescription(SampleTestSteps.class,
-                "a_customized_step_with_parameters_and_empty_field_value: Joe")
+        ExecutedStepDescription description = ExecutedStepDescription.of(SampleTestSteps.class,
+                "a_customized_step_with_parameters_and_empty_field_value",
+                new Object[] {"Joe"})
                 .withDisplayedFields(ImmutableMap.of("color",(Object)"red", "emptyField", Fields.FieldValue.UNDEFINED));
 
         AnnotatedStepDescription annotatedStepDescription = AnnotatedStepDescription.from(description);
@@ -165,13 +169,26 @@ public void a_step_can_be_annotated_to_provide_a_more_readable_name_including_a_
 
     @Test
     public void a_step_can_be_annotated_to_provide_a_more_readable_name_including_several_parameters() {
-        ExecutedStepDescription description = new ExecutedStepDescription(SampleTestSteps.class, "a_customized_step_with_two_parameters: Joe,20");
+        ExecutedStepDescription description = ExecutedStepDescription.of(SampleTestSteps.class,
+                "a_customized_step_with_two_parameters",
+                new Object[]{"Joe", "20"});
 
         AnnotatedStepDescription annotatedStepDescription = AnnotatedStepDescription.from(description);
 
         assertThat(annotatedStepDescription.getName(), is("a step about a person called Joe, aged 20"));
     }
 
+    @Test
+    public void a_step_can_be_annotated_to_provide_a_more_readable_name_including_several_parameters_with_comma() {
+        ExecutedStepDescription description = ExecutedStepDescription.of(SampleTestSteps.class,
+                "a_customized_step_with_two_parameters",
+                new Object[] {"Smith, Joe", "20"});
+
+        AnnotatedStepDescription annotatedStepDescription = AnnotatedStepDescription.from(description);
+
+        assertThat(annotatedStepDescription.getName(), is("a step about a person called Smith, Joe, aged 20"));
+    }
+
     @Test
     public void should_identify_pending_steps() {
         ExecutedStepDescription description = new ExecutedStepDescription(SampleTestSteps.class, "a_pending_step");

diff --git a/...c/test/java/net/serenitybdd/junit/runners/integration/WhenRunningANonWebTestScenario.java b/...c/test/java/net/serenitybdd/junit/runners/integration/WhenRunningANonWebTestScenario.java
@@ -295,7 +295,20 @@ public void should_report_nested_class_parameters_correctly() throws Initializat
 
         TestOutcome testOutcome = testOutcomeWithTitle("Should handle nested object parameters", executedScenarios);
         TestStep firstStep = testOutcome.getTestSteps().get(0);
-        assertThat(firstStep.getDescription(), is("a step with an object parameter called <span class='step-parameter'>$100.00</span>"));
+        assertThat(firstStep.getDescription(), is("a step with an object parameter called $100.00"));
+    }
+
+    @Test
+    public void should_report_correctly_customized_title_for_parameter_with_comma() throws InitializationError {
+
+        SerenityRunner runner = new SerenityRunner(NonWebTestScenarioWithParameterizedSteps.class);
+        runner.run(new RunNotifier());
+
+        List<TestOutcome> executedScenarios = runner.getTestOutcomes();
+
+        TestOutcome testOutcome = testOutcomeWithTitle("Should be correct customized title for parameter with comma", executedScenarios);
+        TestStep firstStep = testOutcome.getTestSteps().get(0);
+        assertThat(firstStep.getDescription(), is("a step with two object parameters called 'Joe, Smith' and '20'"));
     }
 
     private TestOutcome testOutcomeWithTitle(String title, List<TestOutcome> testOutcomes) {

diff --git a/...-junit/src/test/java/net/thucydides/samples/NonWebTestScenarioWithParameterizedSteps.java b/...-junit/src/test/java/net/thucydides/samples/NonWebTestScenarioWithParameterizedSteps.java
@@ -38,5 +38,8 @@ public void should_handle_nested_object_parameters() {
         steps.a_customized_step_with_object_parameters(new SampleNonWebSteps.CurrencyIn$(100));
     }
 
-
+    @Test
+    public void should_be_correct_customized_title_for_parameter_with_comma() {
+        steps.a_customized_step_with_two_parameters("Joe, Smith", "20");
+    }
 }
diff --git a/serenity-junit/src/test/java/net/thucydides/samples/SampleNonWebSteps.java b/serenity-junit/src/test/java/net/thucydides/samples/SampleNonWebSteps.java
@@ -96,4 +96,7 @@ public String toString() {
     @Step("a step with an object parameter called {0}")
     public void a_customized_step_with_object_parameters(CurrencyIn$ currency) {}
 
+    @Step("a step with two object parameters called '{0}' and '{1}'")
+    public void a_customized_step_with_two_parameters(String param1, String param2) {}
+
 }