Open
Description
#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.
Metadata
Metadata
Assignees
Type
Projects
Milestone
Relationships
Development
No branches or pull requests
Activity
vinodhabib commentedon Dec 26, 2024
@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.
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 commentedon Dec 26, 2024
"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 commentedon Dec 27, 2024
@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).