Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transformer doesn't convert "[empty]" values in the table to empty string ("") #2262

Closed
manish7-thakur opened this issue Mar 11, 2021 · 10 comments
Milestone

Comments

@manish7-thakur
Copy link

Describe the bug
Updated to version 6.10.0 recently and went through the documentation https://github.com/cucumber/cucumber-jvm-scala/blob/main/docs/transformers.md but seems like none of the registered transformers for empty values in being used during the transformation. Not sure at what stage the transformer is called or if I'm doing something wrong.

Registered the transformer defined as:

  DefaultDataTableCellTransformer("[empty]")((s: String, t: java.lang.reflect.Type) => {
    new StringBuilder().append(s).append("-").append(t)
  })

and can see it's being registered (see attached screenshot) with glue but after transformation the DataTable values are still coming out to be [empty].

To Reproduce
Steps to reproduce the behavior.

  Scenario: Test case 2, hotel with special condition
    Given Hotels 419 checkin 2020-10-01 los 2
    And BookingDate 2020-09-19 01:30:00
    And GMTOffset +7
    When The user search
    Then The cancellation policy from search result should match following expected value
      | RoomId     | Promotion | Description | DefaultDescription | FreeCancellationDate          | Title               | Symbol            |
      | 1199923528 | -         |  [empty]    |       [empty]      | 2020-09-19T00:00:00.000+07:00 | Cancellation policy | special-condition |

Step is defined as:

  Then("""^The search result should match following expected value$""") { (dt: DataTable) =>
    val result = parseResult(response, dataExtractors)
    validateResult(dt, result)
  }

Expected behavior
I expected the cells in features marked with [empty would be converted to "" against which result can be validated.

Versions:

  • Cucumber Scala: [e.g. 6.10.0]
  • Scala: [e.g. 2.12]

Additional context
Add any other context about the problem here.

Screen Shot 2021-03-11 at 14 26 04

@gaeljw
Copy link
Member

gaeljw commented Mar 11, 2021

Hi @manish7-thakur, can you provide us the validateResult method or at least how you use the dt: DataTable parameter?

One thing to note though is that DefaultDataTableCellTransformer is used to convert from a String to a type t, in your implementation you are always returning a StringBuilder. This kinda works but is not the intended usage.

TBH I've rarely see any use of the DefaultDataTableCellTransformer, most of the time DataTableType fits better but I would need to see your DataTable usage to understand what you are trying to do in the end.

@gaeljw gaeljw self-assigned this Mar 11, 2021
@mpkorstanje
Copy link
Contributor

One thing to note though is that DefaultDataTableCellTransformer is used to convert from a String to a type t, in your implementation you are always returning a StringBuilder. This kinda works but is not the intended usage.

For the [empty] replacement to work as expected the return type should be String.

@gaeljw
Copy link
Member

gaeljw commented Mar 11, 2021

For the [empty] replacement to work as expected the return type should be String.

Right, didn't think about that! Thanks @mpkorstanje

@manish7-thakur
Copy link
Author

manish7-thakur commented Mar 12, 2021

Hi @manish7-thakur, can you provide us the validateResult method or at least how you use the dt: DataTable parameter?

One thing to note though is that DefaultDataTableCellTransformer is used to convert from a String to a type t, in your implementation you are always returning a StringBuilder. This kinda works but is not the intended usage.

TBH I've rarely see any use of the DefaultDataTableCellTransformer, most of the time DataTableType fits better but I would need to see your DataTable usage to understand what you are trying to do in the end.

@gaeljw validateResult is just used to find table diff after prepending header to actual response. I expect that at this point the expected datable's [empty] cells would be replaced with string values.

  def validateResult(expected: DataTable, actual: Seq[String], sameSorting: Boolean = false): Unit = {
    val rows = actual.map(d => d.tail.split("\\|"))
    val header = if (!expected.cells().isEmpty) expected.cells().get(0).asScala.toArray else Array.empty[String]
    val data = if (!header.isEmpty || rows.nonEmpty) DataTable.create((header :: rows.toList).map(_.toList.asJava).asJava) else ValidationResult.EmptyExpectedResult

    try if (sameSorting) expected.diff(data) else expected.unorderedDiff(data)
    catch {
      // To avoid stack trace on the HTML report
      case tde: TableDiffException =>
        throw CustomDiffException(tde.toString)
      case e: Exception => throw e
    }
  }

@manish7-thakur
Copy link
Author

manish7-thakur commented Mar 12, 2021

For the [empty] replacement to work as expected the return type should be String.

Right, didn't think about that! Thanks @mpkorstanje

Well I have tried String and other response types. Even tried the JacksonDefaultDataTableEntryTransformer.
https://github.com/cucumber/cucumber-jvm-scala/blob/73c33a0acc3185af74a5a34e55436acc64310a4a/cucumber-scala/src/main/scala/io/cucumber/scala/JacksonDefaultDataTableEntryTransformer.scala

But nothing seems to work out. If you say that return type should be String, then how come this test works:
https://github.com/cucumber/cucumber-jvm-scala/blob/e584a27ffb090ac562ba6e6a7a19e8bba6e31ea3/cucumber-scala/src/test/scala/io/cucumber/scala/ScalaDslDefaultDataTableEntryTransformerTest.scala#L78

At the moment I have to work around this issue with:

  private def replaceWithEmptyString(expected: DataTable) = {
    val enrichedCells = expected.cells().asScala.map(_.asScala.map {
      case "[empty]" => ""
      case str => str
    }.asJava).asJava
    DataTable.create(enrichedCells)
  }

@gaeljw
Copy link
Member

gaeljw commented Mar 12, 2021

I would need to check the code of Cucumber core to be 100% sure but I believe transformers are not applied when you access the DataTable directly with the cells() method.

Transformers are applied when you use for instance:

dataTable.asLists[String](classOf[String]) // java.util.List[java.util.List[String]]
// Or
dataTable.asScalaRawLists[String] // Seq[Seq[String]]

@manish7-thakur
Copy link
Author

I would need to check the code of Cucumber core to be 100% sure but I believe transformers are not applied when you access the DataTable directly with the cells() method.

Transformers are applied when you use for instance:

dataTable.asLists[String](classOf[String]) // java.util.List[java.util.List[String]]
// Or
dataTable.asScalaRawLists[String] // Seq[Seq[String]]

But I think transformation is applied while creating the DataTable instance from the feature file. So when we get the instance of DataTable I expect the [empty] cells to have been replaced with empty("") string already. I'm accessing cells() of expected as a workaround to replace [empty] cells manually with a helper method replaceWithEmptyString in the validateResult to move forward for now.

@mpkorstanje
Copy link
Contributor

mpkorstanje commented Mar 12, 2021

The transformers are only applied when the data table is converted into something else. The idea being that you use a list of transformed objects rather then a data table.

  Then("""^The search result should match following expected value$""") { 
      (cancelationPolicyResults: Seq[CancelationPolicyResult]) => ...

  }

Or if you use it on a list of strings, that you use a list of strings instead.

  Then("""^The search result should match following expected value$""") { 
      (cancelationPolicyResults: Seq[Seq[String]]) => ...

  }

@mpkorstanje
Copy link
Contributor

It would make sense for all .asX methods to invoke the transformer though, currently the difference between .asList(String.class) and .asList() is somewhat unexpected. We can do this on the next major version of Cucumber JVM.

@mpkorstanje mpkorstanje transferred this issue from cucumber/cucumber-jvm-scala Mar 12, 2021
@mpkorstanje mpkorstanje modified the milestones: v8.0.0, v7.0.0 Mar 12, 2021
@manish7-thakur
Copy link
Author

The transformers are only applied when the data table is converted into something else. The idea being that you use a list of transformed objects rather then a data table.

  Then("""^The search result should match following expected value$""") { 
      (cancelationPolicyResults: Seq[CancelationPolicyResult]) => ...

  }

Or if you use it on a list of strings, that you use a list of strings instead.

  Then("""^The search result should match following expected value$""") { 
      (cancelationPolicyResults: Seq[Seq[String]]) => ...

  }

@mpkorstanje @gaeljw Thanks for clarifying this point. I was under the impression that transformers are applied while creating the DataTable instance where it's just used while converting DataTable to something else.

cukebot pushed a commit to cucumber-attic/datatable-java that referenced this issue Apr 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants