DuplicateStepDefinition error for abstract method implementation.

[sanflg]

Problem:

Whenever we have a class with Step or Hook definition/annotation, this class cannot be inherited by another but can be the last class of an hierarchy. The problem arises when we have common methods definition across this classes and no abstract method can be implemented to enforce behaviors (abstract class nor interface). i.e:

abstract class Parent {
    public abstract void method();
}

class Child1 extends Parent {
    @Given("Child 1")
    public void method(){
        System.out.println("1");
    }
}

class Child2 extends Parent {
    @Given("Child 2")
    public void method(){
        System.out.println("2");
    }
}

There is any chance to made cucumber recognize abstract definitions to avoid DuplicateStepDefinition error on implementation and use POO needed implementations such as abstraction and behavior ruling?

More dependencies context: Maven

<dependency>
    <groupId>io.cucumber</groupId>
    <artifactId>cucumber-java</artifactId>
    <version>7.12.0</version>
</dependency>

<dependency>
    <groupId>io.cucumber</groupId>
    <artifactId>cucumber-testng</artifactId>
    <version>7.12.0</version>
</dependency>

<dependency>
    <groupId>org.testng</groupId>
    <artifactId>testng</artifactId>
    <version>7.7.1</version>
</dependency>

Class definitions in my scenario:

BasePageBehaviors interface

public interface BasePageBehaviors<T extends BasePage<T>> {
     public T goTo();
}

BasePage abstract class

public abstract class BasePage<T extends BasePage<T>> implements BasePageBehaviors {
   public void goTo(String url){
   driver.get(url);
   }
}

GoogleHomePage class

public class GoogleHomePage extends BasePage<GoogleHomePage> {
    @Given("User is in search page")
    public GoogleHomePage goTo() {
        goTo(url.toString());
        return this;
    }
}

Error:

[ERROR] Failures: 
[ERROR] org.seleniumTest.cucumber.CucumberRunnerTests.runScenario(org.seleniumTest.cucumber.CucumberRunnerTests)
[ERROR]   Run 1: CucumberRunnerTests>AbstractTestNGCucumberTests.runScenario:35 » DuplicateStepDefinition

[ERROR] runScenario(org.seleniumTest.cucumber.CucumberRunnerTests)  Time elapsed: 0.003 s  <<< FAILURE!
io.cucumber.core.runner.DuplicateStepDefinitionException: Duplicate step definitions in org.seleniumTest.pageObjects.GoogleHomePage.goTo() and org.seleniumTest.pageObjects.GoogleHomePage.goTo()

[mpkorstanje]

I'm having some trouble understanding your problem.

I can not produce a duplicate DuplicateStepDefinitionException from the code you have provided. Could you perhaps provide an MCVE in a Github Repo? You can use the skeleton project to get started https://github.com/cucumber/cucumber-java-skeleton

[sanflg]

Hi @mpkorstanje
Generated an specific branch where this issue is reproducible: https://github.com/sanflg/SeleniumTest/tree/DuplicateStepDefinition

Glue objects hierarchy: src/main/java/org/seleniumTest/pageObjects
Features: src/test/resources/features/search.feature
CucumberRunner: src/test/java/org/seleniumTest/cucumber/CucumberRunnerTests.java

Running with cucumber.xml file I'm getting:

io.cucumber.core.runner.DuplicateStepDefinitionException: Duplicate step definitions in org.seleniumTest.pageObjects.GoogleHomePage.goTo() and org.seleniumTest.pageObjects.GoogleHomePage.goTo()

	at io.cucumber.core.runner.CachingGlue.lambda$prepareGlue$3(CachingGlue.java:278)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1541)
	at io.cucumber.core.runner.CachingGlue.prepareGlue(CachingGlue.java:272)
	at io.cucumber.core.runner.Runner.runPickle(Runner.java:72)
	at io.cucumber.testng.TestNGCucumberRunner.lambda$runScenario$1(TestNGCucumberRunner.java:132)
	at io.cucumber.core.runtime.CucumberExecutionContext.lambda$runTestCase$5(CucumberExecutionContext.java:137)
	at io.cucumber.core.runtime.RethrowingThrowableCollector.executeAndThrow(RethrowingThrowableCollector.java:23)
	at io.cucumber.core.runtime.CucumberExecutionContext.runTestCase(CucumberExecutionContext.java:137)
	at io.cucumber.testng.TestNGCucumberRunner.runScenario(TestNGCucumberRunner.java:129)
	at io.cucumber.testng.AbstractTestNGCucumberTests.runScenario(AbstractTestNGCucumberTests.java:35)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.testng.internal.invokers.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:139)
	at org.testng.internal.invokers.TestInvoker.invokeMethod(TestInvoker.java:677)
	at org.testng.internal.invokers.TestInvoker.invokeTestMethod(TestInvoker.java:221)
	at org.testng.internal.invokers.MethodRunner.runInSequence(MethodRunner.java:50)
	at org.testng.internal.invokers.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:969)
	at org.testng.internal.invokers.TestInvoker.invokeTestMethods(TestInvoker.java:194)
	at org.testng.internal.invokers.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:148)
	at org.testng.internal.invokers.TestMethodWorker.run(TestMethodWorker.java:128)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1541)
	at org.testng.TestRunner.privateRun(TestRunner.java:829)
	at org.testng.TestRunner.run(TestRunner.java:602)
	at org.testng.SuiteRunner.runTest(SuiteRunner.java:437)
	at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:431)
	at org.testng.SuiteRunner.privateRun(SuiteRunner.java:391)
	at org.testng.SuiteRunner.run(SuiteRunner.java:330)
	at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
	at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:95)
	at org.testng.TestNG.runSuitesSequentially(TestNG.java:1256)
	at org.testng.TestNG.runSuitesLocally(TestNG.java:1176)
	at org.testng.TestNG.runSuites(TestNG.java:1099)
	at org.testng.TestNG.run(TestNG.java:1067)
	at com.intellij.rt.testng.IDEARemoteTestNG.run(IDEARemoteTestNG.java:66)
	at com.intellij.rt.testng.RemoteTestNGStarter.main(RemoteTestNGStarter.java:109)

And with maven command "clean test -Ddriver=chrome -Dmaximize=false -DsuiteXmlFile=cucumber":

	Suppressed: io.cucumber.core.runner.DuplicateStepDefinitionException: Duplicate step definitions in org.seleniumTest.pageObjects.GoogleHomePage.goTo() and org.seleniumTest.pageObjects.GoogleHomePage.goTo()
		at io.cucumber.core.runner.CachingGlue.lambda$prepareGlue$3(CachingGlue.java:278)
		at java.base/java.util.ArrayList.forEach(ArrayList.java:1541)
		at io.cucumber.core.runner.CachingGlue.prepareGlue(CachingGlue.java:272)
		at io.cucumber.core.runner.Runner.runPickle(Runner.java:72)
		at io.cucumber.testng.TestNGCucumberRunner.lambda$runScenario$1(TestNGCucumberRunner.java:132)
		at io.cucumber.core.runtime.CucumberExecutionContext.lambda$runTestCase$5(CucumberExecutionContext.java:137)
		at io.cucumber.core.runtime.RethrowingThrowableCollector.executeAndThrow(RethrowingThrowableCollector.java:23)
		at io.cucumber.core.runtime.CucumberExecutionContext.runTestCase(CucumberExecutionContext.java:137)
		at io.cucumber.testng.TestNGCucumberRunner.runScenario(TestNGCucumberRunner.java:129)
		at io.cucumber.testng.AbstractTestNGCucumberTests.runScenario(AbstractTestNGCucumberTests.java:35)
		at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
		at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
		at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
		at java.base/java.lang.reflect.Method.invoke(Method.java:566)
		at org.testng.internal.invokers.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:139)
		at org.testng.internal.invokers.TestInvoker.invokeMethod(TestInvoker.java:677)
		at org.testng.internal.invokers.TestInvoker.invokeTestMethod(TestInvoker.java:221)
		at org.testng.internal.invokers.MethodRunner.runInSequence(MethodRunner.java:50)
		at org.testng.internal.invokers.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:969)
		at org.testng.internal.invokers.TestInvoker.invokeTestMethods(TestInvoker.java:194)
		at org.testng.internal.invokers.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:148)
		at org.testng.internal.invokers.TestMethodWorker.run(TestMethodWorker.java:128)
		... 20 more

[jkronegg]

I can not produce a duplicate DuplicateStepDefinitionException from the code you have provided.

I could reproduce the issue by adding the classes BasePageBehaviors, BasePage and GoogleHomePage to one of my project which uses Cucumber 7.12.0.

This is due to the fact that GoogleHomePage.class.getDeclaredMethods() has two methods:

0 = {Method@2523} "public mypackage.GoogleHomePage mypackage.GoogleHomePage.goTo()"
1 = {Method@2524} "public mypackage.BasePage mypackage.GoogleHomePage.goTo()"

That's interesting, I've never realized that such object-oriented construction will lead to two methods, one being an "alias" of the other. As the common Java developer, I learned this is not possible, see https://stackoverflow.com/questions/5561436/can-two-java-methods-have-same-name-with-different-return-type . But it seems I was wrong, see https://stackoverflow.com/a/5561736/698168

I see two solution variants:

Ignore alias method based on Field.slot

We could use the Field.slot to differentiate them, e.g. https://stackoverflow.com/questions/47529744/what-is-a-java-lang-reflect-fieldslot (the method which returns a GoogleHomePage has slot 1 and the one which returns BasePage has slot 2). In this case we could ignore the Method with the same name/parameters that has the highest slot.

I see some drawbacks with this variant:

• we will need global information (a kind of "bag" with the two methods).
• detecting an "alias" is complex (we need to determine whether two methods are the same)
• relying on the undocumented Field.slot is not very secure (the behavior could depend on the JVM implementation)

Ignore alias method based on Method.modifiers

We could also use the Method.modifiers, because they are different. Modifier.toString(method.clazz.getDeclaredMethods()[i].getModifiers()) gives:

• GoogleHomePage -> "public"
• BasePage -> "public volatile"

In this case, we could ignore the methods with volatile modifier (Modifier.isVolatile(method.getModifiers())).

This variant has the advantage to use only local information (we can decide just based on the Method without requiring information about other potential "alias" Methods).

This could be done in JavaBackend.loadGlue:

@Override
public void loadGlue(Glue glue, List<URI> gluePaths) {
    GlueAdaptor glueAdaptor = new GlueAdaptor(lookup, glue);

    gluePaths.stream()
            .filter(gluePath -> CLASSPATH_SCHEME.equals(gluePath.getScheme()))
            .map(ClasspathSupport::packageName)
            .map(classFinder::scanForClassesInPackage)
            .flatMap(Collection::stream)
            .distinct()
            .forEach(aGlueClass -> scan(aGlueClass, (method, annotation) -> {
                if (!Modifier.isVolatile(method.getModifiers()) {    // <----------------- added condition
                    container.addClass(method.getDeclaringClass());
                    glueAdaptor.addDefinition(method, annotation);
                }
            }));
}

Note: I tested the above code on my project and it works without throwing the DuplicateStepDefinitionException.

[sanflg]

TL&DR: Motivation on why would need abstraction on Step definitions, this is not related to any technical aspect but rather to work methodologies and what I consider good practices.

@jkronegg @mpkorstanje followed the discussion in the PR and may not be able to contribute technically to the solution but the whole motivation in making abstract implementations for "Step definitions" classes relies in the following problem, more related to a workin methodology and good practices rather than a real code problem:

For common verbal expressions (used in Gherkin), where there may not exist syntactical or conceptual difference, depending on the scenario, each of this expressions in Step Definitions may change a lot in the code implementation. Usually we have the acceptance criteria to check that all the requirements are implemented but some times we can miss some points or do half implementations.

Because of this, abstraction is the way that we have to enforce behaviors in OOP projects, in order to achieve this, we can use interfaces and abstract classes to enforce child classes to implement this general required behaviors, but adding the enforcement on step definition classes to also implement this behaviors, adds one layer more of compromise that this features will be applied, clearly this doesn't means that the feature files will be also enforced to implement this missing function part for common required behaviors/tests but this can be supplied with a custom plugging and even with some sort of automated script to generate feature files where we consume the already defined step definitions.

Clearly if we have step definitions with the same signature, cucumber will arise the DuplicateStepDefinition exception, but we can control this using DDD sort of solution with package visibility and order (Check this blog post for more info) and this way, with package control, we could have parent classes enforcing behaviors and holding the common methods for all this classes in a parent directory.

This whole discussion only makes sense in really big enterprise projects where we loss track of each one of the tickets/pages/implementations and this way we have one more tool to remember to don't miss them.

[jkronegg]

Thanks for these clarifications.

Usually we have the acceptance criteria to check that all the requirements are implemented but some times we can miss some points or do half implementations.

I totally agree with you. I got the same experience on some my projects based on Cucumber: your tests are passing just because the Steps definitions are wrongly implemented. I consider steps definitions as code : they should be tested as well. Ideally, you should have a kind of test harness/framework which helps the developer. Currently, I have custom code for this. But my dream would be that Cucumber do it for you, either built-in or through a plugin (I have no idea on how to do it, but IMHO the first-in-class framework which does this is Mockito: it tells you when one of your mock is useless and guide you to write better test code).

If you have ideas on how to integrate such automatic checks in Cucumber, do not hesitate to discuss the point on the Slack channel or to open a new issue on Github.

solution with package visibility and order (Check this blog post for more info)

I would have done the same as proposed in the blog you mention. But without correcting the current issue, there will still be DuplicateStepDefinition errors.

BTW, which JVM do you use ? (as this issue seems to be JVM specific)

[sanflg]

Currently for this project I'm using corretto-11.0.16.1. May I update it to another distribution/newer version?.

It would be nice to start checking plugging for IDE or a check for feature implementation, will start doing this investigation on my free time. The most similar thing that comes to my mind for a line by line usage check is Jacoco more usually implemented by unitary tests due to the 1-1 matching nature of unitary testing but would not be entirely wrong to do some similar kind of implementation for stepDefinitions method usage.

[jkronegg]

May I update it to another distribution/newer version?

No, that's not required. The issue should be resolved on Cucumber side (Cucumber should work the same way on all JVMs).

[jkronegg]

@sanflg this issue is solved in cucumber 7.12.1