Skip to content

StreamTracer should be mockable #4658

Open
@ejona86

Description

@ejona86

#3305 put DoNotMock on StreamTracer saying that the mocks are not thread-safe. But mockito claims they are, except for stubbing and verify methods. We need to figure out where the discrepancy is and either file a bug against Mockito or let the StreamTracer be mocked.

I discovered this as part of removing DoNotMock for #4452. For my changes, I left things as-is, but I found the documentation I wrote to be questionable.

CC @carl-mastrangelo

Activity

added this to the Next milestone on Jul 19, 2018
vinodhabib

vinodhabib commented on Dec 26, 2024

@vinodhabib
Contributor

@shivaspeaks After analysis I understand like this issue does not have any action as i can see @DoNotMock annotation was removed as part of PR #4659 from the StreamStracer class which was added/introduced as part of PR #3305 earlier.
Also @DoNotMock was removed from error_prone_annotations in 2.1.3 version upgrade as per PR #4659 details later it was added as gradle dependency as part of grpc-core.

For the below highlighted description point :

We need to figure out where the discrepancy is and either file a bug against Mockito or let the StreamTracer be mocked as per the below comment https://github.com/grpc/grpc-java/pull/4659/files#r203902174 can see as,
See #4658. I'm purposefully not trying to "change" anything; I'm trying to keep the status-quo. Yes, I see the tester is in internal. There's relatively few users of this, so I'm not too worried about them using something internal (especially inside google where it is easy to track who's using it). But this is also the state the class was already in.

image

By referring to the above comments and other PR details, I hope we don't have anything pending here. Kindly confirm and suggest.

As per my understanding this issue is fixed as part of PR #4659

ejona86

ejona86 commented on Dec 26, 2024

@ejona86
MemberAuthor

"We need to figure out where the discrepancy is and either file a bug against Mockito or let the StreamTracer be mocked" seems unresolved. I don't see how that comment says anything; that is me just saying, "I'm keeping the status quo" and was written the day after I made this issue.

I'm willing to resolve the last part of this by saying #3305 is mistaken and we remove the "DO NOT MOCK" from StreamTracer. At this point, the problem could have been long-resolved bugs in mockito.

vinodhabib

vinodhabib commented on Dec 27, 2024

@vinodhabib
Contributor

"We need to figure out where the discrepancy is and either file a bug against Mockito or let the StreamTracer be mocked" seems unresolved. I don't see how that comment says anything; that is me just saying, "I'm keeping the status quo" and was written the day after I made this issue.

I'm willing to resolve the last part of this by saying #3305 is mistaken and we remove the "DO NOT MOCK" from StreamTracer. At this point, the problem could have been long-resolved bugs in mockito.

@ejona86 @shivaspeaks Request you to elaborate more on the expected/pending changes of this issue? still its not clear on what changes are required here? also not clear on which discrepancy we are discussing/talking about here (as per my previous understanding i thought the discrepancy is all about using @DoNotMock annotation or not).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @ejona86@vinodhabib

        Issue actions

          StreamTracer should be mockable · Issue #4658 · grpc/grpc-java