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

Spies are not reset after test execution when using @SpyBean #33830

Closed
feliksik opened this issue Jan 13, 2023 · 3 comments
Closed

Spies are not reset after test execution when using @SpyBean #33830

feliksik opened this issue Jan 13, 2023 · 3 comments
Labels
type: bug A general bug
Milestone

Comments

@feliksik
Copy link

feliksik commented Jan 13, 2023

I experience the same problem as in #7374: The @SpyBean on the class is not reset between test cases.

I create a small (minimal?) version to reproduce, as suggested in the mentioned issue.

See the git repo on https://github.com/feliksik/bug-reports/tree/master/spybean-spring.

For convenience, the test inline:

@DataJpaTest
@ContextConfiguration(classes = SpyBeanJpa.MyConfig.class)
@SpyBean(classes = EntityJpaRepo.class)
class SpyBeanJpa {

    @Configuration("TestService")
    @EnableJpaRepositories({"nl.feliksik.bugs.spybean.jpa"})
    @EntityScan({"nl.feliksik.bugs.spybean.jpa"})
    @ComponentScan({
            "nl.feliksik.bugs.spybean",
    })
    public static class MyConfig {
    }

    @Autowired
    private MyService systemUnderTest;

    @Autowired
    private EntityJpaRepo jpaRepo;

    @BeforeEach
    void setUp() {
        // workaround for https://github.com/spring-projects/spring-boot/issues/33830
        //Mockito.reset(jpaRepo);
    }

    @Test
    void testA_break() {
        RuntimeException runtimeException = new RuntimeException("We simulate a failure");

        // Note that if you use the syntax that starts with when(), the real method
        // is called with null param. This is an unrelated issue I don't understand.
        doThrow(runtimeException).when(jpaRepo).saveAndFlush(any());

        try {
            systemUnderTest.upsert("someId");

            // should not reach this
            assertThat(true).isFalse();
        } catch (RuntimeException e) {
            // ok
            assertThat(e).isEqualTo(runtimeException);
        }
    }


    @Test
    void testB_ok() {
        // should be ok -- but it FAILS!
        systemUnderTest.upsert("someId");
    }
}

The first test mocks an exception (the test expects it, and succeeds), the second test suffers from this: it fails, as it does not expect an exception. (If I run them separately or change the alphabetic order of cases, all is fine).

The workaround I discovered is the reset() in the setup method.

Note that I could not reproduce this without the Jpa repo; but I want to simulate a database failure.

  1. Possibly there is some of the Jpa/Mocking magic that conflicts here?
  2. Is this a bug?
  3. If someone can point out a better way to test for database failures, that would be appreciated.
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 13, 2023
@wilkinsona wilkinsona self-assigned this Jan 18, 2023
@wilkinsona
Copy link
Member

wilkinsona commented Jan 18, 2023

Thanks for the report and sample. I've reproduced the problem. The spy isn't being reset due to #7271 and the problem that we anticipated to some extent in this comment:

Changing from getBean(name) to getSingleton(name) (as proposed in #7271) avoids the problem as we end up working with the FactoryBean rather than the bean that it will produce. That generally feels ok to me, but there is an edge case where it'll break: if someone has a FactoryBean in some test configuration that produces a mock, the mock will no longer be reset.

This is what's happening in your sample (although the FactoryBean isn't declared in test configuration) and we end up checking if a JpaRepositoryFactoryBean instance is a mock (it isn't) rather than checking if the repository that it produced is a mock (it would have been).

I'm not sure how we can fix this without regressing 7271 so I'd like to discuss it with the rest of the team. In the meantime, your workaround is a good one. Alternatively, you may want to perform this sort of failure testing in a pure unit test that doesn't involve the application context.

@wilkinsona wilkinsona removed their assignment Jan 18, 2023
@wilkinsona wilkinsona added the for: team-meeting An issue we'd like to discuss as a team to make progress label Jan 18, 2023
@feliksik
Copy link
Author

Thank you for your comment, I actually found a workaround so I can live on :-)

Aside this bug, w.r.t. the testing strategy: I would indeed like to do unit testing instead of loading the entire application context, but I don't believe it's nice or wise to mock all methods of a JPA repo for the unit test. Does the spring/JPA philosophy like mocking so much that it would actually always mock this in the tests?

I know there is some controversy about whether to use mocks always or not; but am I correct that there is no way to obtain an in-memory implementation of a JPA repo, so you can unittest with that dependency, and not having to mock the methods in the test? (i.e.. just observe the behavior from your unit under test)?

@philwebb philwebb added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged for: team-meeting An issue we'd like to discuss as a team to make progress labels Feb 1, 2023
@philwebb philwebb added this to the 2.7.x milestone Feb 1, 2023
@philwebb
Copy link
Member

philwebb commented Feb 1, 2023

We're going to try calling getBean(...) first and if that fails we'll call getSingleton(...).

@wilkinsona wilkinsona modified the milestones: 2.7.x, 2.7.9 Feb 1, 2023
krenson pushed a commit to krenson/test-push that referenced this issue Mar 15, 2023
…ot-starter-parent from 2.7.8 to 2.7.9 (patch)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [org.springframework.boot:spring-boot-starter-parent](https://spring.io/projects/spring-boot) ([source](https://github.com/spring-projects/spring-boot)) | parent | patch | `2.7.8` -> `2.7.9` |

---

### Release Notes

<details>
<summary>spring-projects/spring-boot</summary>

### [`v2.7.9`](https://github.com/spring-projects/spring-boot/releases/tag/v2.7.9)

[Compare Source](spring-projects/spring-boot@v2.7.8...v2.7.9)

#### 🐞 Bug Fixes

-   Maven Plugin's PropertiesMergingResourceTransformer closes InputStream when it should not do so [#&#8203;34063](spring-projects/spring-boot#34063)
-   Actuator Health web endpoint broken with Gson and Java 17 [#&#8203;34030](spring-projects/spring-boot#34030)
-   Dependency management for Mongo's Java Driver is incomplete [#&#8203;33941](spring-projects/spring-boot#33941)
-   Using devtools with Reactive application results in slower restarts [#&#8203;33855](spring-projects/spring-boot#33855)
-   Spies are not reset after test execution when using `@SpyBean` [#&#8203;33830](spring-projects/spring-boot#33830)
-   Properties Migrator does not detect properties of Map type that are marked as deprecated [#&#8203;27854](spring-projects/spring-boot#27854)

#### 📔 Documentation

-   Updated documentation for `@ConfigurationProperties` bean naming rules [#&#8203;34029](spring-projects/spring-boot#34029)
-   Restore "Use Jedis Instead of Lettuce" how-to documentation [#&#8203;33994](spring-projects/spring-boot#33994)
-   Add Redis application properties example [#&#8203;33965](spring-projects/spring-boot#33965)
-   Use Maven Central for release downloads in CLI installation documentation [#&#8203;33962](spring-projects/spring-boot#33962)
-   Actuator section is missing from documentation overview [#&#8203;33932](spring-projects/spring-boot#33932)
-   Add Javadoc since to OperationParameter.getAnnotation() [#&#8203;33914](spring-projects/spring-boot#33914)
-   Document additional configuration that is required for spring.mvc.throw-exception-if-no-handler-found=true to be effective [#&#8203;31660](spring-projects/spring-boot#31660)

#### 🔨 Dependency Upgrades

-   Upgrade to ActiveMQ 5.16.6 [#&#8203;34238](spring-projects/spring-boot#34238)
-   Upgrade to Byte Buddy 1.12.23 [#&#8203;34239](spring-projects/spring-boot#34239)
-   Upgrade to Dropwizard Metrics 4.2.16 [#&#8203;34240](spring-projects/spring-boot#34240)
-   Upgrade to Elasticsearch 7.17.9 [#&#8...
krenson pushed a commit to krenson/test-push that referenced this issue Mar 15, 2023
…ot-starter-parent from 2.7.8 to 2.7.9 (patch)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [org.springframework.boot:spring-boot-starter-parent](https://spring.io/projects/spring-boot) ([source](https://github.com/spring-projects/spring-boot)) | parent | patch | `2.7.8` -> `2.7.9` |

---

### Release Notes

<details>
<summary>spring-projects/spring-boot</summary>

### [`v2.7.9`](https://github.com/spring-projects/spring-boot/releases/tag/v2.7.9)

[Compare Source](spring-projects/spring-boot@v2.7.8...v2.7.9)

#### 🐞 Bug Fixes

-   Maven Plugin's PropertiesMergingResourceTransformer closes InputStream when it should not do so [#&#8203;34063](spring-projects/spring-boot#34063)
-   Actuator Health web endpoint broken with Gson and Java 17 [#&#8203;34030](spring-projects/spring-boot#34030)
-   Dependency management for Mongo's Java Driver is incomplete [#&#8203;33941](spring-projects/spring-boot#33941)
-   Using devtools with Reactive application results in slower restarts [#&#8203;33855](spring-projects/spring-boot#33855)
-   Spies are not reset after test execution when using `@SpyBean` [#&#8203;33830](spring-projects/spring-boot#33830)
-   Properties Migrator does not detect properties of Map type that are marked as deprecated [#&#8203;27854](spring-projects/spring-boot#27854)

#### 📔 Documentation

-   Updated documentation for `@ConfigurationProperties` bean naming rules [#&#8203;34029](spring-projects/spring-boot#34029)
-   Restore "Use Jedis Instead of Lettuce" how-to documentation [#&#8203;33994](spring-projects/spring-boot#33994)
-   Add Redis application properties example [#&#8203;33965](spring-projects/spring-boot#33965)
-   Use Maven Central for release downloads in CLI installation documentation [#&#8203;33962](spring-projects/spring-boot#33962)
-   Actuator section is missing from documentation overview [#&#8203;33932](spring-projects/spring-boot#33932)
-   Add Javadoc since to OperationParameter.getAnnotation() [#&#8203;33914](spring-projects/spring-boot#33914)
-   Document additional configuration that is required for spring.mvc.throw-exception-if-no-handler-found=true to be effective [#&#8203;31660](spring-projects/spring-boot#31660)

#### 🔨 Dependency Upgrades

-   Upgrade to ActiveMQ 5.16.6 [#&#8203;34238](spring-projects/spring-boot#34238)
-   Upgrade to Byte Buddy 1.12.23 [#&#8203;34239](spring-projects/spring-boot#34239)
-   Upgrade to Dropwizard Metrics 4.2.16 [#&#8203;34240](spring-projects/spring-boot#34240)
-   Upgrade to Elasticsearch 7.17.9 [#&#8...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants