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.

Sign up for GitHub

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

Jump to bottom

[Spring] Invoke all TestContextManager methods #2661

Merged
merged 1 commit into from
Dec 11, 2022
Merged

[Spring] Invoke all TestContextManager methods #2661

merged 1 commit into from
Dec 11, 2022

Conversation

mpkorstanje
Copy link
Contributor

@mpkorstanje mpkorstanje commented Dec 11, 2022 •
edited

⚡️ What's your motivation?

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

🏷️ What kind of change is this?

  • 🐛 Bug fix (non-breaking change which fixes a defect)
  • ⚡ New feature (non-breaking change which adds new behaviour)

📋 Checklist:

  • I agree to respect and uphold the Cucumber Community Code of Conduct
  • I've changed the behaviour of the code
    • I have added/updated tests to cover my changes.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • Users should know about my change
    • I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

@codecov
Copy link

codecov bot commented Dec 11, 2022 •
edited

Codecov Report

Merging #2661 (aa55c97) into main (9de55cf) will increase coverage by 0.03%.
The diff coverage is 91.66%.

@@             Coverage Diff              @@
##               main    #2661      +/-   ##
============================================
+ Coverage     84.77%   84.81%   +0.03%     
- Complexity     2686     2693       +7     
============================================
  Files           322      322              
  Lines          9532     9570      +38     
  Branches        903      905       +2     
============================================
+ Hits           8081     8117      +36     
- Misses         1120     1121       +1     
- Partials        331      332       +1
Impacted Files Coverage Δ
...in/java/io/cucumber/spring/TestContextAdaptor.java 93.63% <91.66%> (+0.58%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@mpkorstanje mpkorstanje force-pushed the spring-invoke-all-test-context-manager-phases branch from 74efc89 to 9370efa Compare December 11, 2022 15:38
@mpkorstanje mpkorstanje changed the title [Spring] Invoke all test context manager lifecycle phases [Spring] Invoke all TestContextManager methods Dec 11, 2022
@mpkorstanje mpkorstanje force-pushed the spring-invoke-all-test-context-manager-phases branch 2 times, most recently from 6c1e4d6 to 6ee38e0 Compare December 11, 2022 15:43
@mpkorstanje
[Spring] Invoke all TestContextManager methods
aa55c97 
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 mpkorstanje force-pushed the spring-invoke-all-test-context-manager-phases branch from 6ee38e0 to aa55c97 Compare December 11, 2022 15:46
@mpkorstanje mpkorstanje marked this pull request as ready for review December 11, 2022 15:55
@mpkorstanje mpkorstanje merged commit df2042b into main Dec 11, 2022
@mpkorstanje mpkorstanje deleted the spring-invoke-all-test-context-manager-phases branch December 11, 2022 15:55
mpkorstanje added a commit that referenced this pull request Dec 16, 2022
@mpkorstanje
[Spring] Support constructor dependency @CucumberContextConfiguration…
4b8d712 
… classes

In #2661 classes annotated with `@CucumberContextConfiguration` were created as
beans directly from the bean factory. This allowed them to be prepared as test
instances by JUnit. However, we did not tell the factory that it should
autowire constructor dependencies resulting in #2663.

Additionally, because beans were created directly from the bean factory, they
were not added to the application context. This meant that while dependencies
could be injected into them, they could not be injected into other objects.

Fixes: #2663
@mpkorstanje mpkorstanje mentioned this pull request Dec 16, 2022
Merged
7 tasks
mpkorstanje added a commit that referenced this pull request Dec 16, 2022
@mpkorstanje
[Spring] Inject CucumberContextConfiguration constructor dependencies
13f319f 
In #2661 classes annotated with `@CucumberContextConfiguration` were created as
beans directly from the bean factory. This allowed them to be prepared as test
instances by JUnit. However, we did not tell the factory that it should
autowire constructor dependencies resulting in #2663.

Additionally, because beans were created directly from the bean factory, they
were not added to the application context. This meant that while dependencies
could be injected into them, they could not be injected into other objects.

Fixes: #2663
mpkorstanje added a commit that referenced this pull request Dec 16, 2022
@mpkorstanje
[Spring] Inject CucumberContextConfiguration constructor dependencies
1a55e41 
In #2661 classes annotated with `@CucumberContextConfiguration` were created as
beans directly from the bean factory. This allowed them to be prepared as test
instances by JUnit. However, we did not tell the factory that it should
autowire constructor dependencies resulting in #2663.

Additionally, because beans were created directly from the bean factory, they
were not added to the application context. This meant that while dependencies
could be injected into them, they could not be injected into other objects.

Fixes: #2663
mpkorstanje added a commit that referenced this pull request Dec 16, 2022
@mpkorstanje
[Spring] Inject CucumberContextConfiguration constructor dependencies (…
0f2b83c 
…#2664)

In #2661 classes annotated with `@CucumberContextConfiguration` were created as
beans directly from the bean factory. This allowed them to be prepared as test
instances by JUnit. However, we did not tell the factory that it should
autowire constructor dependencies resulting in #2663.

Additionally, because beans were created directly from the bean factory, they
were not added to the application context. This meant that while dependencies
could be injected into them, they could not be injected into other objects.

Fixes: #2663
mpkorstanje added a commit that referenced this pull request May 24, 2024
@mpkorstanje
[Spring] Document CucumberContextConfiguration semantics
e39f05e 
Cucumber uses Spring Test Context Manager framework. This framework was
written for JUnit and assumes that there is a "test instance".
Cucumber however uses multiple step definition classes and so it has
multiple test instances.

Originally `@CucumberContextConfiguration` was added to signal to Spring
which class should be used to configure the application context from.
But as people also expected mock beans and other features provided by
Springs test execution listeners to work (#2661) the annotated instance
was only instantiated but never initialized by Spring.

This changed the semantics somewhat as now features that depend on the
bean being initialized stopped working (#2886). Unfortunately, there is
little that can be done here. Spring expects that the instance provided
to the Test Context Manager to be an uninitialized bean. The solution
for this is to put the context configuration and step definitions in
different classes.

Cleaning up the examples to follow this pattern should avoid this
problem somewhat in the future. Though I won't go as far as recommending
people do this. Putting everything in one class looks quite nice. And
generally still works.

Closes: #2886
@mpkorstanje mpkorstanje mentioned this pull request May 24, 2024
Open
7 tasks
mpkorstanje added a commit that referenced this pull request May 24, 2024
@mpkorstanje
[Spring] Document CucumberContextConfiguration semantics
148c2f8 
Cucumber uses Spring Test Context Manager framework. This framework was
written for JUnit and assumes that there is a "test instance".
Cucumber however uses multiple step definition classes and so it has
multiple test instances.

Originally `@CucumberContextConfiguration` was added to signal to Spring
which class should be used to configure the application context from.
But as people also expected mock beans and other features provided by
Springs test execution listeners to work (#2661) the annotated instance
was only instantiated but never initialized by Spring.

This changed the semantics somewhat as now features that depend on the
bean being initialized stopped working (#2886). Unfortunately, there is
little that can be done here. Spring expects that the instance provided
to the Test Context Manager to be an uninitialized bean. The solution
for this is to put the context configuration and step definitions in
different classes.

Cleaning up the examples to follow this pattern should avoid this
problem somewhat in the future. Though I won't go as far as recommending
people do this. Putting everything in one class looks quite nice. And
generally still works.

Closes: #2886
@mpkorstanje mpkorstanje mentioned this pull request May 24, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

@MockBean not Working with SpringBootTest and Junit5
1 participant
@mpkorstanje