Skip to content

Support primitives in GenericArrayReturnType.#3753

Merged
TimvdLippe merged 2 commits intomockito:mainfrom
jselbo:main
Oct 22, 2025
Merged

Support primitives in GenericArrayReturnType.#3753
TimvdLippe merged 2 commits intomockito:mainfrom
jselbo:main

Conversation

@jselbo
Copy link
Collaborator

@jselbo jselbo commented Oct 21, 2025

Fixes #3752

In my testing, for a method returning byte[], Method#getGenericReturnType() on a normal JVM returns a Class, the same as byte[].class.

On Android, the implementation is completely different than OpenJDK's. Code ref:
https://cs.android.com/android/platform/superproject/main/+/main:libcore/ojluni/src/main/java/java/lang/reflect/Method.java;l=175-177;bpv=0;bpt=1

It uses Android-specific reflection APIs under the hood, and this returns a GenericArrayType with component type byte.class.

This is my attempt to handle this scenario, by adding primitive support to GenericArrayReturnType.

Checklist

  • [ X] Read the contributing guide
  • [ X] PR should be motivated, i.e. what does it fix, why, and if relevant how
  • [ X] 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
  • [ X] Avoid other runtime dependencies
  • [ X] 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
  • [ X] The pull request follows coding style (run ./gradlew spotlessApply for auto-formatting)
  • [ X] Mention Fixes #<issue number> in the description if relevant
  • [ X] At least one commit should end with Fixes #<issue number> if relevant

@jselbo
Copy link
Collaborator Author

jselbo commented Oct 21, 2025

Sorry, I did not think of a good way to add tests. The new Primitives logic is trivial and maybe doesn't need a test. The GenericArrayReturnType class is a private implementation detail. I suppose I could make it package-private and construct it directly in the test to verify its logic?

Copy link
Contributor

@TimvdLippe TimvdLippe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can add some tests here: https://github.com/mockito/mockito/tree/main/mockito-integration-tests/android-tests/src/test/java/org/mockitousage/androidtest We prefer to use full integration tests rather than unit tests here. Therefore, you can copy the example as mentioned in your issue and put that in the test suite

@jselbo
Copy link
Collaborator Author

jselbo commented Oct 22, 2025

Thanks, added an integration test. I confirmed the test fails without the changes in GenericMetadataSupport and passes with them.

@jselbo jselbo requested a review from TimvdLippe October 22, 2025 15:40
@codecov-commenter
Copy link

codecov-commenter commented Oct 22, 2025

Codecov Report

❌ Patch coverage is 92.85714% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.46%. Comparing base (bf3a809) to head (6bf612d).

Files with missing lines Patch % Lines
...ternal/util/reflection/GenericMetadataSupport.java 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #3753      +/-   ##
============================================
+ Coverage     86.44%   86.46%   +0.01%     
- Complexity     2987     2988       +1     
============================================
  Files           341      341              
  Lines          9032     9041       +9     
  Branches       1112     1113       +1     
============================================
+ Hits           7808     7817       +9     
+ Misses          943      942       -1     
- Partials        281      282       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@TimvdLippe TimvdLippe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix!

@TimvdLippe TimvdLippe merged commit 8564b43 into mockito:main Oct 22, 2025
19 checks passed
dongjoon-hyun added a commit to apache/spark-kubernetes-operator that referenced this pull request Dec 10, 2025
### What changes were proposed in this pull request?

This PR aims to upgrade `mockito` to 5.21.0 for Apache Spark K8s Operator `v0.7`.

### Why are the changes needed?

To bring the latest features and bug fixes.
- https://github.com/mockito/mockito/releases/tag/v5.21.0
  - mockito/mockito#3758
  - mockito/mockito#3752
  - mockito/mockito#3753
  - mockito/mockito#3759
  - mockito/mockito#3731

### Does this PR introduce _any_ user-facing change?

No behavior change.

### How was this patch tested?

Pass the CIs.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes #425 from dongjoon-hyun/SPARK-54672.

Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
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.

ClassNotFoundException when stubbing array of primitive type on Android

3 participants