Should work for cucumber scenario outlines

[alexmsmartins]

Problem

Cucumber tests with Data Tables will pass with failing examples as long as the first example passes.

This happens because all the examples in have the same fully qualified name that comes from the scenario outline description.

The changes

This Pr simply allows for tests with equal names to pass through from the JUnit RunNotifier to the SBT EventHandler without repeats being filtered out.

The first commit in this test is a passing build that should have failed and the second commit is the fix.

[alexmsmartins]

I already signed the CLA.

[benmccann]

Hey @alexmsmartins, I was the primary maintainer of this repo for the past several years. I no longer use SBT much because my company was acquired by a company that uses Gradle. We should find someone at Lightbend to take over this repo and review your PR. I'll let you drive that since you're probably more motivated to get this change in

A couple places you might try:

• https://groups.google.com/forum/#!forum/sbt-dev
• https://gitter.im/sbt/sbt-contrib

@dwijnand @jsuereth would you be able to help find a new maintainer for this repo?

[alexmsmartins]

Hi @benmccann , @dwijnand and @jsuereth

Is it possible to just get merge permissions?

That should get things going because, in my team, we keep getting bitten by this problem where scenarios with the same name that pass basically stop failing scenarios with the same name in other files from failing the build. And the fix in this PR does solve that problem.

While you don't get a new maintainer, I could just push things forward a bit.

[benmccann]

@szeiger perhaps you could help review this PR?

@alexmsmartins do you mind rebasing this PR? sorry I know it's quite old

[SethTisue]

I'll add one more possible name on here: @eed3si9n

[eed3si9n]

I'd be happy to review and get this to finish line.

[eed3si9n]

This seems problematic. If we're doing Lightbend CLA the copy right needs to transfer to Lightbend.

[eed3si9n]

This happens because all the examples in have the same fully qualified name that comes from the scenario outline description.

I don't know anything about Cucumber, but this sounds like their bug? I feel uneasy compromising the overall contract of testframework for all junit-interface users just to workaround Cucumber bug/oddity.

[gaeljw]

This happens because all the examples in have the same fully qualified name that comes from the scenario outline description.

I don't know anything about Cucumber, but this sounds like their bug? I feel uneasy compromising the overall contract of testframework for all junit-interface users just to workaround Cucumber bug/oddity.

IMHO It doesn't feel like a bug in Cucumber, the same case works absolutely fine in Scala+Maven.
You can see the issue I just created this morning and two project samples (one with sbt, one with Maven). cucumber/cucumber-jvm-scala#22

[mpkorstanje]

@eed3si9n

I don't know anything about Cucumber, but this sounds like their bug? I feel uneasy compromising the overall contract of testframework for all junit-interface users just to workaround Cucumber bug/oddity.

JUnit uses the Description object to determine the identity of tests. Internally Description implements equals and hashcode by delegating to Serializable uniqueId.

Cucumber is currently using Description.createTestDescription(String className, String name, Serializable uniqueId) to create descriptions for each example. While the class name and name may not be unique, the unique id is.

So rather then depending on the class and method name to determine test identity the Description.equals() and Description.hashcode() method should be be used.

[mpkorstanje]

This is a pom dependency and not required.

[mpkorstanje]

This is a transitive dependency of cucumber-scala and not required.

[mpkorstanje]

This is a rather old version. Latest is 4.7.1

https://search.maven.org/search?q=g:%22io.cucumber%22%20AND%20a:%22cucumber-scala_2.12%22

[mpkorstanje]

reported now goes unused.

[eed3si9n]

AbstractEvent should probably expose Description, and reported should use the description's identity to track whether the event has been posted or not.

[eed3si9n]

JUnit uses the Description object to determine the identity of tests.

Thanks for the explanation. I guess the impedance mismatch here is that junit-interface (and sbt test frameworks in general) tries to lookup tests using a mechanism called fingerprint and it's either SubclassFingerprint or AnnotatedFingerprint. In case of the example used in this PR, I'm guessing that org.junit.runner.RunWith annotation fingerprint would picked up cucumber.CucumberApplicationSpec:

package cucumber

import cucumber.api.CucumberOptions
import cucumber.api.junit.Cucumber
import org.junit.runner.RunWith

@RunWith(classOf[Cucumber])
@CucumberOptions(
  features = Array("classpath:features"),
  glue = Array("com.waioeka.sbt"),
  strict = true,
)
class CucumberApplicationSpec

sbt would then send back the task definition for cucumber.CucumberApplicationSpec back to the runner exposed by junit-interface. sbt on the other end would be listening to the events for it.

So rather than depending on the class and method name to determine test identity the Description.equals() and Description.hashcode() method should be be used.

That makes sense. Currently it's testing for if(reported.add(e.fullyQualifiedName())), but reported should keep track of Description's identity instead.

[gaeljw]

✔️ For anyone coming here, please note that starting from version 6.2.2 cucumber-junit now adds a unique number to workaround this issue (see cucumber/cucumber-jvm#2045).