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

Spying objects not working properly since 5.3.0 #2972

Closed
mmaeller opened this issue Apr 13, 2023 · 14 comments
Closed

Spying objects not working properly since 5.3.0 #2972

mmaeller opened this issue Apr 13, 2023 · 14 comments

Comments

@mmaeller
Copy link

Hi.

starting with 5.3.0 we are unable to spy ObjectMapper with a registered JavaTimeModule.
Could this be dependent on the changes for #2877? It looks like the module has been removed after creating the spy.

With 5.2.0 everything is working fine. Please see the minimal example below.

import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.Mockito.spy;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
import java.time.LocalDateTime;
import java.util.concurrent.atomic.AtomicReference;
import org.junit.jupiter.api.Test;

public class ObjectMapperTest {

  @Test
  void spyObjectMapperWithJavaTimeModule() {
    var objectMapper = spy(new ObjectMapper().registerModule(new JavaTimeModule()));
    var reference = new AtomicReference<Dummy>();
    assertDoesNotThrow(
        () ->
            reference.set(
                objectMapper.readValue(
                    """
          {
          "timestamp": "2023-04-13T09:00:00"
          }
          """,
                    Dummy.class)));
    assertEquals(LocalDateTime.of(2023, 4, 13, 9, 0), reference.get().timestamp());
  }

  record Dummy(LocalDateTime timestamp) {}
}
@cmuchinsky
Copy link

cmuchinsky commented Apr 13, 2023 •
edited
Loading

Seeing similar problems with spy mocks, in my case it seems like the spy wrapper isn't calling the real methods under the covers properly

@mmaeller mmaeller changed the title Unable to spy ObjectMapper with a registered JavaTimeModule Spying objects not working properly since 5.3.0 Apr 13, 2023
@mmaeller
Copy link
Author

My first thought was the issue could be the JavaTimeModule being final.

@TimvdLippe
Copy link
Contributor

Which version of ByteBuddy are you using? Can you downgrade to the version of ByteBuddy that Mockito 5.2.0 used and let us know if that solves the issue?

@cmuchinsky
Copy link

Which version of ByteBuddy are you using? Can you downgrade to the version of ByteBuddy that Mockito 5.2.0 used and let us know if that solves the issue?

FWIW, I'm using ByteBuddy 1.14.4 (latest) with Mockito 5.2.0 without any issues, its only when I upgrade to Mockito 5.3.0 that I started having problems

@mmaeller
Copy link
Author

Hi @TimvdLippe, thanks for your response.
5.3.0 with byte-buddy 1.14.1 and 1.14.4 not working.
5.2.0 with byte-buddy 1.14.1 and 1.14.4 working.

@TimvdLippe
Copy link
Contributor

@raphw I think this is #2948 don't you think?

@raphw
Copy link
Member

raphw commented Apr 13, 2023

I'd be surprised by that. Could you build Mockito but revert this one commit to see if it solves the issue?

@ratoaq2
Copy link

ratoaq2 commented Apr 14, 2023

I'm also having intermittent issues spying instances with mockito 5.3.0

@raphw
Copy link
Member

raphw commented Apr 14, 2023

We have a bunch of tests on this, so I am a bit confused. I will have a look once I find the time.

Could someone try to create a reproduction of the test without external dependencies? This would ease my work a lot, and I can use it as a regression test.

@mmaeller
Copy link
Author

@raphw
https://github.com/mmaeller/mockito-2972

@rzanner
Copy link

rzanner commented Apr 19, 2023

We have a similar issue spying on objects.

Fields of the real object that are initialized in the default constructor call are not properly set in the spy and therefore causing a different behavior at runtime. This could be solved by using MockSettings.useConstructor(), thankfully.

Another issue is more serious: also method calls are not redirected to the spy anymore - instead, the method on the mock is called. :-(
So, although the actual instance that was "spy"ied from via spiedInstance(actual).defaultAnswer(CALLS_REAL_METHODS) would yield the correct result, the method call result is the one from the additionally created mock instance.
I could solve this only by also instrumenting the methods of the spy (which should not be needed, when using "CALLS_REAL_METHODS", at least from my understanding).

@salander85 salander85 mentioned this issue Apr 20, 2023
Merged
1 task
@TimvdLippe
Copy link
Contributor

Can anybody who is affected bisect the commits between Mockito 5.2.0 and 5.3.0 and let us know which commit breaks the behavior? I am still stumbled by this regression and I can't piece it together with the commits that were in this minor version release.

@raphw
Copy link
Member

raphw commented Apr 21, 2023

I tried it now and it is certainly related to the change of the visibility. I am however not yet sure how this relates, looking into.

@raphw
Copy link
Member

raphw commented Apr 21, 2023

Found out and fixed: #2983

@asolntsev asolntsev mentioned this issue Apr 21, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@rzanner @raphw @TimvdLippe @ratoaq2 @mmaeller @cmuchinsky