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

Use TestContextManager in cucumber-spring #1470

Closed
mpkorstanje opened this issue Sep 26, 2018 · 20 comments
Closed

Use TestContextManager in cucumber-spring #1470

mpkorstanje opened this issue Sep 26, 2018 · 20 comments
Labels
good first issue Good for newcomers 🙏 help wanted Help wanted - not prioritized by core team 🧷 pinned Tells Stalebot not to close this issue ⚡ enhancement Request for new functionality
Milestone

Comments

@mpkorstanje
Copy link
Contributor

mpkorstanje commented Sep 26, 2018

Summary

Support TestContextManager in cucumber-spring.

Expected Behavior

When using cucumber-spring @MockBean (and other annotations) should just work.

public class Example {

    @MockBean
    private User object;

    @Given("there is a User")
    public void there_is_a_User() {
        when(object.toString()).thenReturn("I've been mocked");
    }

    @Then("the User is mocked")
    public void the_user_is_mocked() throws Exception {
        assertThat(object.toString(), equalTo("I've been mocked"));
    }
}

Current Behavior

It is currently possible to use @Autowired e.g.:

public class Example {

    @Autowired
    private User object;

    @Then("the user is real")
    public void the_user_is_mocked() throws Exception {
        assertThat(object.toString(), equalTo("I am real"));
    }
}

But fields annotated with@MockBean are not mocked, the example fails with a null pointer exception.

Possible Solution

The causes of failure are two fold:

  1. There is no spring context declared (ContextConfiguration, BootstrapWith, ect) so the SpringFactory falls back to some minimal default. This can be resolved by adding @SpringBootTest to the step definition.

  2. The SpringFactory does not call

  • TestContextManager.beforeTestClass()
  • TestContextManager.prepareTestInstance().
  • TestContextManager.beforeTestMethod()
  • TestContextManager.beforeTestExecution()
  • TestContextManager.afterTestExecution()
  • TestContextManager.afterTestMethod()
  • TestContextManager.afterTestClass()

Instead step definitions are registered as beans in the Spring context. This had the advantage that step definitions can be auto wired into each other and constructor dependency injection just works. However it also puts the step definitions into the spring context which complicates things.

Given that step definitions calling each other is not a best practice I would not mind step definitions were no longer part of the spring context and instead treated as test instances (see SpringJUnit4ClassRunner.createTest). Any dependency injection would have to be done via @Autowired.

So to implement this properly the following would have to be done:

  • Require that all step definitions created have an empty constructor.
  • Map the life-cycle methods of the TestContextManager to that of the ObjectFactory.
  • Fail when there is no Spring context declared instead of creating a default.

Your Environment

  • Version used: cucumber-spring:4.0.0
@mpkorstanje mpkorstanje added ⚡ enhancement Request for new functionality 🙏 help wanted Help wanted - not prioritized by core team good first issue Good for newcomers Spring 5 labels Sep 26, 2018
@mpkorstanje mpkorstanje changed the title Support @MockBean annotations in cucumber-spring Support @MockBean in cucumber-spring Sep 26, 2018
@mlvandijk
Copy link
Member

Another Spring 5 issue: #1315 (was Closed due to inactivity)

@stale
Copy link

stale bot commented Dec 16, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs.

@stale stale bot added the ⌛ stale Will soon be closed by stalebot unless there is activity label Dec 16, 2018
@stale
Copy link

stale bot commented Dec 24, 2018

This issue has been automatically closed because of inactivity. You can support the Cucumber core team on opencollective.

@stale stale bot closed this as completed Dec 24, 2018
@mpkorstanje mpkorstanje reopened this Dec 30, 2018
@stale stale bot removed the ⌛ stale Will soon be closed by stalebot unless there is activity label Dec 30, 2018
@mpkorstanje
Copy link
Contributor Author

mpkorstanje commented Dec 30, 2018

StaleBot take a break.

@mpkorstanje mpkorstanje added the 🧷 pinned Tells Stalebot not to close this issue label Dec 30, 2018
@mpkorstanje
Copy link
Contributor Author

mpkorstanje commented Feb 2, 2019

Another consideration. Currently a dummy step is required in the context configuration. This is kinda silly.

@SpringBootTest(classes=Application.class)
@AutoConfigureMockMvc
@TestPropertySource("classpath:test.properties")
public class CucumberContextConfiguration  {

    @Value("${key}")
    private String value;

    @Before
    public void setup_cucumber_spring_context(){
        // Dummy method so cucumber will recognize this class as glue
        // and use its context configuration.
    }
}

In Cucumber v5 object factories will be shared between backends. So it should be possible to make a spring backend that scans the glue path for context configuration and adds it to the object factory.

@mlvandijk mlvandijk modified the milestone: 5.0.0 Feb 3, 2019
@mpkorstanje mpkorstanje changed the title Support @MockBean in cucumber-spring Use TestContextManager in cucumber-spring Apr 4, 2019
mpkorstanje added a commit that referenced this issue Apr 7, 2019
In preparation of #1470 constructor dependency injection is deprecated
in favour of field injection with @Autowired.
@mpkorstanje
Copy link
Contributor Author

So #1604 contains a pretty good spike. I've tested it with a a few TestExecutionListeners but more tests are probably needed.

@mpkorstanje mpkorstanje modified the milestones: 5.0.0, 5.x.x Apr 12, 2019
@Dzieciak
Copy link
Contributor

What more is needed to have it ready? I updated spring-deprecate-constructor-dependency-injection to build with the current master here:
https://github.com/Dzieciak/cucumber-jvm/tree/spring-deprecate-constructor-dependency-injection-v5
It seems to work.

Is it a must to have no arguments constructor in classes with cucumber annotations? With constructor dependency injection things are easier to test. Also, no argument constructor requirement concerns not only step definitions, but all classes with Cucumber annotations.

@mpkorstanje
Copy link
Contributor Author

mpkorstanje commented Nov 22, 2019

Heya, thanks for looking into and updating the branch with master!

The main problem I ran into is that the TestContextManager is designed for Unit tests. Unit tests typically have a test context that consists of a single class. Cucumber uses a test context that consists of multiple classes (all classes with step definitions).

So for Cucumber this is a typical test context:

public class UserSteps {

    @MockBean
    private User object;

    @Given("there is a User")
    public void there_is_a_User() {
        when(object.toString()).thenReturn("I've been mocked");
    }

    @Then("the User is mocked")
    public void the_user_is_mocked() throws Exception {
        assertThat(object.toString(), equalTo("I've been mocked"));
    }
}

public class SystemSteps {

    @MockBean
    private System object;

    @Given("there is a System")
    public void there_is_a_System() {
        when(object.toString()).thenReturn("I've been mocked");
    }

    @Then("the System is mocked")
    public void the_user_is_mocked() throws Exception {
        assertThat(object.toString(), equalTo("I've been mocked"));
    }
}

In the the CucumberTestContextManager I am dealing with this by multiplexing all operations of the TestContextManager to each step definition class which is wrapped in a CucumberTestContext class.

However this multiplexing is intentionally not complete. Some CucumberTestContext operations multiplex, others delegate to the TestContext of the stepClassWithSpringContext. For example CucumberTestContext.getAttribute will read and write all attributes from the delegate.

This attribute store contains state and is because of that a potential problem. For example the ServletTestExecutionListener will mutate this state and act accordingly.

	@Override
	public void afterTestMethod(TestContext testContext) throws Exception {
		if (Boolean.TRUE.equals(testContext.getAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE))) {
			if (logger.isDebugEnabled()) {
				logger.debug(String.format("Resetting RequestContextHolder for test context %s.", testContext));
			}
			RequestContextHolder.resetRequestAttributes();
			testContext.setAttribute(DependencyInjectionTestExecutionListener.REINJECT_DEPENDENCIES_ATTRIBUTE,
				Boolean.TRUE);
		}
		testContext.removeAttribute(POPULATED_REQUEST_CONTEXT_HOLDER_ATTRIBUTE);
		testContext.removeAttribute(RESET_REQUEST_CONTEXT_HOLDER_ATTRIBUTE);
	}

Fortunately this example has been properly implemented. Multiple executions of afterTestMethod are idempotent. However there currently are 13 TestExecutionListener implementations that I am aware of. Each of these would have be idempotent:

DependencyInjectionTestExecutionListener
DirtiesContextBeforeModesTestExecutionListener
DirtiesContextTestExecutionListener
MockMvcPrintOnlyOnFailureTestExecutionListener
MockRestServiceServerResetTestExecutionListener
MockitoTestExecutionListener
ResetMocksTestExecutionListener
RestDocsTestExecutionListener
ServletTestExecutionListener
SpringBootDependencyInjectionTestExecutionListener
SqlScriptsTestExecutionListener
TransactionalTestExecutionListener
WebDriverTestExecutionListener

To further complicate this DependencyInjectionTestExecutionListener would appear not to be idempotent, but is in effectivly idempotent because we never re-inject dependencies.

Is it a must to have no arguments constructor in classes with cucumber annotations?

Yes. Unfortunately so. As you can see in SpringFactory.start we must have an instance of each step definition class before we can call testContextManager.prepareTestInstance(instances). The actual DI is then done by the DependencyInjectionTestExecutionListener which is limited to field injection.

However you can still inject dependencies by annotating fields with @Autowired. But because the step definition classes are no longer part of the application contex you can't @Autowire a step definition class into another class. This does come at the advantage of being able to share the application context between threads and concurrent executions.

What more is needed to have it ready?

  1. We need to check if all TestExecutionListener are idempotent or effectively idempotent.
  2. We need a way to share state between steps.
  3. We need to document how the new way of writing step definitions with Cucumber Spring.
  4. We need to document the conceptual difference between the spring application context and the cucumber test context and how these two relate.

For #2 a possible solution would be the introduction of a @CucumberGlue annotation that acts like @Autowired but instead injects classes from the step context.:

public class UserSteps {

    @MockBean
    private User object;

    @CucumberGlue
    private Shared shared;
}

public class SystemSteps {

    @MockBean
    private System object;

    @CucumberGlue
    private Shared shared;
}

public class SharedSteps {

     @AutoWired
      private final SomeComponent component

}

The challenge here is that classes containing @CucumberGlue annotated fields are now also glue classes. But they don't need to have any methods annotated with @Given, @When, ect. This annotation must also live in the cucumber-spring module. So the cucumber-spring module would need it's own Backend implementation to scan for this annotation and add it to the SpringFactory.

This wasn't possible in Cucumber v4.x. It should be possible in v5.x.

I haven't had the time to revisit this yet. You would be most welcome to do so.

@mpkorstanje
Copy link
Contributor Author

mpkorstanje commented Nov 22, 2019

Btw. If you are unable or not interested in spending time on this, I will also accept an PR against spring-deprecate-constructor-dependency-injection rather then master with your updated branch. Everything helps.

@Dzieciak
Copy link
Contributor

Thanks for explanation. I have a broader view on this now. I would definitely like to and will spend more time on this. For now I'll send a PR against spring-deprecate-constructor-dependency-injection

@mpkorstanje
Copy link
Contributor Author

mpkorstanje commented Dec 21, 2019

I'm giving up on the approach used in #1604. The solution to #1848 solves a significant part of the problems I was trying to solve. It however doesn't use make MockBeans possible yet. We still have to invoke the other methods of the TestContextManager.

@mpkorstanje
Copy link
Contributor Author

I don't see a reason to invoke anything but TestContextManager.beforeTestClass() and TestContextManager.afterTestClass(). All other methods require a test instance and test method which cucumber doesn't have.

Reviewing the code in the test execution listeners below I also don't believe anything useful would happen if we were to provide a dummy test instance and method. It's the annotations on the method that drive most of the functionality.

AbstractDirtiesContextTestExecutionListener
AbstractTestExecutionListener
DependencyInjectionTestExecutionListener
DirtiesContextBeforeModesTestExecutionListener
DirtiesContextTestExecutionListener
EventPublishingTestExecutionListener
MockMvcPrintOnlyOnFailureTestExecutionListener
MockRestServiceServerResetTestExecutionListener
MockitoTestExecutionListener
ResetMocksTestExecutionListener
RestDocsTestExecutionListener
ServletTestExecutionListener
SpringBootDependencyInjectionTestExecutionListener
SqlScriptsTestExecutionListener
TransactionalTestExecutionListener
WebDriverTestExecutionListener

@ddsultan
Copy link

ddsultan commented Jun 2, 2020

Is there any workarounf for this in the meantime?

@mpkorstanje
Copy link
Contributor Author

If you're using 6.0.0-RC2 you could use something like this:

@CucumberContextConfiguration
@SpringBootTest(classes = TestConfig.class)
public class CucumberSpringConfiguration {

  @MockBean
  public MyService service;

}

The class annotated with @CucumberContextConfiguration is passed to springs TestContextManager and subject to the same processing the class would be in JUnit. So this should result in the application context being created with a mocked MyService bean.

You'll have to figure out the edgecase yourself.

@ddsultan
Copy link

ddsultan commented Jun 4, 2020

If you're using 6.0.0-RC2 you could use something like this:

@CucumberContextConfiguration
@SpringBootTest(classes = TestConfig.class)
public class CucumberSpringConfiguration {

  @MockBean
  public MyService service;

}

The class annotated with @CucumberContextConfiguration is passed to springs TestContextManager and subject to the same processing the class would be in JUnit. So this should result in the application context being created with a mocked MyService bean.

You'll have to figure out the edgecase yourself.

Thank you @mpkorstanje,

when will the 6.x version be GA?

@pprotas
Copy link

pprotas commented Dec 16, 2021

Is there an update on this? I've been using the workaround mentioned above, but i would like to use @MockBean in my stepdef classes for better readability of the code.

@Yutsa
Copy link

Yutsa commented Sep 12, 2022

I've been using a workaround where I put fields annotated with @MockBean in my @CucumberContextConfiguration and then I @Autowired them in my step definition.

But this is not the best thing to do because I have a lot of mocks in my single @CucumberContextConfiguration. And if I have a test where I need a mock and one where I don't need one it can be problematic.

Is there still no way to use @MockBean inside a step definition ?

@mpkorstanje
Copy link
Contributor Author

Unlike JUnit where in one execution tests from multiple classes can be ran, Cucumber runs scenarios from multiple feature files.

This means that step definitions must be globally available. There is no way to scope them to a specific feature file and thus no way to use different sets of mock beans.

Your best option right now is to either reconsider your motivations for mocking. Instead consider stubbing your external services. Or you may want to consider multiple executions of Cucumber with different context configurations for Spring.

mpkorstanje added a commit that referenced this issue Dec 11, 2022
To make writing tests with Spring easier Spring provides a
`TestContextManager`. This classes provides call backs for various
`TestExecutionListeners`.

These are then used by various extensions such as
the `MockitoTestExecutionListener` which injects `@MockBeans` into test
instances. When all methods are not invoked this leads to problems such as
(#2654,#2655,#2656)

While this was initially (#1470) not a problem, it appears that various
listener implementations have started to assume that all methods would be
invoked.

Closes: #2655
Fixes: #2654, #2572
mpkorstanje added a commit that referenced this issue Dec 11, 2022
To make writing tests with Spring easier Spring provides a
`TestContextManager`. This classes provides call backs for various
`TestExecutionListeners`.

These are then used by various extensions such as
the `MockitoTestExecutionListener` which injects `@MockBeans` into test
instances. When all methods are not invoked this leads to problems such as
(#2654,#2655,#2656)

While this was initially (#1470) not a problem, it appears that various
listener implementations have started to assume that all methods would be
invoked.

Closes: #2655
Fixes: #2654, #2572
mpkorstanje added a commit that referenced this issue Dec 11, 2022
To make writing tests with Spring easier Spring provides a
`TestContextManager`. This classes provides call backs for various
`TestExecutionListeners`.

These are then used by various extensions such as
the `MockitoTestExecutionListener` which injects `@MockBeans` into test
instances. When all methods are not invoked this leads to problems such as
(#2654,#2655,#2656)

While this was initially (#1470) not a problem, it appears that various
listener implementations have started to assume that all methods would be
invoked.

Closes: #2655
Fixes: #2654, #2572
mpkorstanje added a commit that referenced this issue Dec 11, 2022
To make writing tests with Spring easier Spring provides a
`TestContextManager`. This classes provides call backs for various
`TestExecutionListeners`.

These are then used by various extensions such as
the `MockitoTestExecutionListener` which injects `@MockBeans` into test
instances. When all methods are not invoked this leads to problems such as
(#2654,#2655,#2656)

While this was initially (#1470) not a problem, it appears that various
listener implementations have started to assume that all methods would be
invoked.

Closes: #2655
Fixes: #2654, #2572
mpkorstanje added a commit that referenced this issue Dec 11, 2022
To make writing tests with Spring easier Spring provides a
`TestContextManager`. This classes provides call backs for various
`TestExecutionListeners`.

These are then used by various extensions such as
the `MockitoTestExecutionListener` which injects `@MockBeans` into test
instances. When all methods are not invoked this leads to problems such as
(#2654,#2655,#2656)

While this was initially (#1470) not a problem, it appears that various
listener implementations have started to assume that all methods would be
invoked.

Closes: #2655
Fixes: #2654, #2572
@J3riie
Copy link

J3riie commented Apr 2, 2024

Hi !
I've been using the same workaround for the @MockBean fields, and I'm fine with that. The issue I am now encountering is when I want to @SpyBean a field. I cannot make it work, neither when using it in the step definition nor in the same way I am using @MockBean. Any idea on how it should be done ?

Thanks a lot

@mpkorstanje
Copy link
Contributor Author

Please create a new issue. It would also help if you can create a minimal reproducer. And even better if you can analyze the problem by putting breakpoints in all these classes:

https://docs.spring.io/spring-boot/docs/current/api/org/springframework/boot/test/mock/mockito/package-summary.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers 🙏 help wanted Help wanted - not prioritized by core team 🧷 pinned Tells Stalebot not to close this issue ⚡ enhancement Request for new functionality
Projects
None yet
Development

No branches or pull requests

7 participants