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

Add new artifact mockito-subclass (to use mock-maker-subclass MockMaker) #2821

Merged
merged 1 commit into from Dec 14, 2022

Conversation

reta
Copy link
Contributor

@reta reta commented Dec 12, 2022

Signed-off-by: Andriy Redko andriy.redko@aiven.io
Related to #2589 (comment)

Checklist

  • Read the contributing guide
  • PR should be motivated, i.e. what does it fix, why, and if relevant how
  • If possible / relevant include an example in the description, that could help all readers
    including project members to get a better picture of the change
  • Avoid other runtime dependencies
  • Meaningful commit history ; intention is important please rebase your commit history so that each
    commit is meaningful and help the people that will explore a change in 2 years
  • The pull request follows coding style
  • Mention Fixes #<issue number> in the description if relevant
  • At least one commit should mention Fixes #<issue number> if relevant

@reta
Copy link
Contributor Author

reta commented Dec 12, 2022

@TimvdLippe could you please take a look if I correctly implemented your idea, thank you

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
@TimvdLippe
Copy link
Contributor

In the CI logs of the Java 8 job with the mock-maker-subclass I see a lot of exceptions coming from MockUtil. Can you check whether it is configured correctly and why it is throwing these? I am a bit confused, as the code looks good and I don't see any obvious reason why this would start failing. And why the CI job itself is green anyways.

@reta
Copy link
Contributor Author

reta commented Dec 13, 2022

In the CI logs of the Java 8 job with the mock-maker-subclass I see a lot of exceptions coming from MockUtil. Can you check whether it is configured correctly and why it is throwing these? I am a bit confused, as the code looks good and I don't see any obvious reason why this would start failing. And why the CI job itself is green anyways.

@TimvdLippe just to confirm we are on the same page, I see this assertion:

2022-12-13T08:23:29.2135998Z     java.lang.AssertionError
2022-12-13T08:23:29.2136388Z         at org.mockito.internal.util.MockUtil.getMockHandlerOrNull(MockUtil.java:160)
2022-12-13T08:23:29.2136849Z         at org.mockito.internal.util.MockUtil.isMock(MockUtil.java:147)
2022-12-13T08:23:29.2137322Z         at org.mockito.internal.util.DefaultMockingDetails.isMock(DefaultMockingDetails.java:32)

If that's the one you have in mind - it is all over the place (all builds / mock makers I mean), the assertions is gone on retry

@TimvdLippe
Copy link
Contributor

Yes that's the one I spotted. I don't think we have these assertions on master, do we? If it is on master we can safely ignore it for this PR, but we should fix it in a separate PR.

@reta
Copy link
Contributor Author

reta commented Dec 13, 2022

Yeah, I saw it on local builds (master) and CIs, we could probably see that on #2804 (needs your approval please for workflow run), btw here it the latest run on master [1]

> Task :androidTest:compressReleaseAssets
> Task :androidTest:checkReleaseDuplicateClasses
> Task :androidTest:dexBuilderRelease
> Task :androidTest:desugarReleaseFileDependencies

> Task :test

org.mockito.StaticMockingExperimentTest > stubbing_new FAILED
    java.lang.AssertionError
        at org.mockito.internal.util.MockUtil.getMockHandlerOrNull(MockUtil.java:160)
        at org.mockito.internal.util.MockUtil.isMock(MockUtil.java:147)
        at org.mockito.internal.util.DefaultMockingDetails.isMock(DefaultMockingDetails.java:32)
        at org.mockito.internal.util.DefaultMockingDetails.assertGoodMock(DefaultMockingDetails.java:85)
        at org.mockito.internal.util.DefaultMockingDetails.mockHandler(DefaultMockingDetails.java:77)
        at org.mockito.internal.util.DefaultMockingDetails.getMockHandler(DefaultMockingDetails.java:68)
        at org.mockito.StaticMockingExperimentTest.<init>(StaticMockingExperimentTest.java:44)

[1] https://github.com/mockito/mockito/actions/runs/3612965923/jobs/6088416494#logs

@TimvdLippe
Copy link
Contributor

Ugh yeah you are correct. Filed #2823 for this. This PR LGTM, thanks!

@TimvdLippe TimvdLippe merged commit a4e2e48 into mockito:main Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants