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

Handle Termux in InlineDelegateByteBuddyMockMaker.java #3158

Merged
merged 2 commits into from Oct 24, 2023
Merged

Handle Termux in InlineDelegateByteBuddyMockMaker.java #3158

merged 2 commits into from Oct 24, 2023

Conversation

ascopes
Copy link
Contributor

@ascopes ascopes commented Oct 24, 2023 •
edited

Termux on Android can run any JVM, so we cannot rely on the vendor here to determine if we are in Termux. Nonetheless, ByteBuddy can fail to instrument correctly on this platform, so it is worth showing a meaningful error message in this scenario.

Changes

  • Implement checks for Termux in InlineDelegateByteBuddyMockMaker
  • Implement unit tests for the ByteBuddy platform conditions
  • Extract the ByteBuddy platform conditions to a separate utility class
    to enable unit testing them separately. This also enables adding additional
    checks in the future without making the InlineDelegateByteBuddyMockMaker
    much more complicated.
  • Added dependency at test time for junit-jupiter-params to enable writing
    parameterized unit tests.

Existing behaviour:

$ jshell
Oct 24, 2023 7:50:27 AM java.util.prefs.FileSystemPreferences$1 run
INFO: Created user preferences directory.
|  Welcome to JShell -- Version 17-internal
|  For an introduction type: /help intro

jshell> System.getProperty("java.specification.vendor")
$1 ==> "Oracle Corporation"

jshell> System.getProperty("os.name")
$2 ==> "Linux"

jshell> System.getProperty("java.home")
$3 ==> "/data/data/com.termux/files/usr/opt/openjdk"

$ uname -a
Linux localhost 5.4.210-qgki-g809b0fd981e4 #1 SMP PREEMPT Thu Aug 24 22:38:41 CST 2023 aarch64 Android

$ ./mvnw clean test
...
[INFO] Stack trace
[INFO] java.lang.IllegalStateException: Could not initialize plugin: interface org.mockito.plugins.MockMaker (alternate: null)
        at org.mockito.internal.configuration.plugins.PluginLoader$1.invoke(PluginLoader.java:84)
        at jdk.proxy2/jdk.proxy2.$Proxy23.createStaticMock(Unknown Source)
        at org.mockito.internal.util.MockUtil.createStaticMock(MockUtil.java:202)
        at org.mockito.internal.MockitoCore.mockStatic(MockitoCore.java:134)
        at org.mockito.Mockito.mockStatic(Mockito.java:2325)
        at org.mockito.internal.configuration.MockAnnotationProcessor.processAnnotationForMock(MockAnnotationProcessor.java:69)
        at org.mockito.internal.configuration.MockAnnotationProcessor.process(MockAnnotationProcessor.java:28)
        at org.mockito.internal.configuration.MockAnnotationProcessor.process(MockAnnotationProcessor.java:25)
        at org.mockito.internal.configuration.IndependentAnnotationEngine.createMockFor(IndependentAnnotationEngine.java:44)
        at org.mockito.internal.configuration.IndependentAnnotationEngine.process(IndependentAnnotationEngine.java:72)
        at org.mockito.internal.configuration.InjectingAnnotationEngine.processIndependentAnnotations(InjectingAnnotationEngine.java:62)
        at org.mockito.internal.configuration.InjectingAnnotationEngine.process(InjectingAnnotationEngine.java:47)
        at org.mockito.MockitoAnnotations.openMocks(MockitoAnnotations.java:81)
        at org.mockito.internal.framework.DefaultMockitoSession.<init>(DefaultMockitoSession.java:43)
        at org.mockito.internal.session.DefaultMockitoSessionBuilder.startMocking(DefaultMockitoSessionBuilder.java:83)
        at org.mockito.junit.jupiter.MockitoExtension.beforeEach(MockitoExtension.java:159)
        at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
        at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
        Suppressed: java.lang.NullPointerException: Cannot invoke "java.util.Set.forEach(java.util.function.Consumer)" because the return value of "org.junit.jupiter.api.extension.ExtensionContext$Store.remove(Object, java.lang.Class)" is null
                at org.mockito.junit.jupiter.MockitoExtension.afterEach(MockitoExtension.java:190)
                ... 2 more
Caused by: java.lang.IllegalStateException: Internal problem occurred, please report it. Mockito is unable to load the default implementation of class that is a part of Mockito distribution. Failed to load interface org.mockito.plugins.MockMaker
        at org.mockito.internal.configuration.plugins.DefaultMockitoPlugins.create(DefaultMockitoPlugins.java:105)
        at org.mockito.internal.configuration.plugins.DefaultMockitoPlugins.getDefaultPlugin(DefaultMockitoPlugins.java:79)
        at org.mockito.internal.configuration.plugins.PluginLoader.loadPlugin(PluginLoader.java:75)
        at org.mockito.internal.configuration.plugins.PluginLoader.loadPlugin(PluginLoader.java:50)
        at org.mockito.internal.configuration.plugins.PluginRegistry.<init>(PluginRegistry.java:27)
        at org.mockito.internal.configuration.plugins.Plugins.<clinit>(Plugins.java:22)
        at org.mockito.internal.MockitoCore.<clinit>(MockitoCore.java:73)
        at org.mockito.Mockito.<clinit>(Mockito.java:1683)
        at org.mockito.junit.jupiter.MockitoExtension.beforeEach(MockitoExtension.java:155)
        ... 2 more
Caused by: java.lang.reflect.InvocationTargetException
        at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
        at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480)
        at org.mockito.internal.configuration.plugins.DefaultMockitoPlugins.create(DefaultMockitoPlugins.java:103)
        ... 10 more
Caused by: org.mockito.exceptions.base.MockitoInitializationException:
Could not initialize inline Byte Buddy mock maker.

It appears as if your JDK does not supply a working agent attachment mechanism.
Java               : 17
JVM vendor name    : Oracle Corporation
JVM vendor version : 17-internal+0-adhoc..src
JVM name           : OpenJDK 64-Bit Server VM
JVM version        : 17-internal+0-adhoc..src
JVM info           : mixed mode
OS name            : Linux
OS version         : 5.4.210-qgki-g809b0fd981e4

        at org.mockito.internal.creation.bytebuddy.InlineDelegateByteBuddyMockMaker.<init>(InlineDelegateByteBuddyMockMaker.java:244)
        at org.mockito.internal.creation.bytebuddy.InlineByteBuddyMockMaker.<init>(InlineByteBuddyMockMaker.java:23)
        ... 13 more
Caused by: java.lang.IllegalStateException: Could not self-attach to current VM using external process
        at net.bytebuddy.agent.ByteBuddyAgent.installExternal(ByteBuddyAgent.java:706)
        at net.bytebuddy.agent.ByteBuddyAgent.install(ByteBuddyAgent.java:636)
        at net.bytebuddy.agent.ByteBuddyAgent.install(ByteBuddyAgent.java:616)
        at net.bytebuddy.agent.ByteBuddyAgent.install(ByteBuddyAgent.java:568)
        at net.bytebuddy.agent.ByteBuddyAgent.install(ByteBuddyAgent.java:545)
        at org.mockito.internal.creation.bytebuddy.InlineDelegateByteBuddyMockMaker.<clinit>(InlineDelegateByteBuddyMockMaker.java:115)
        ... 14 more

@ascopes
Handle Termux in InlineDelegateByteBuddyMockMaker.java
d8cb1ef 
Termux on Android can run any JVM, so we cannot rely on the vendor
here to determine if we are in Termux. Nonetheless, ByteBuddy can fail to instrument correctly
on this platform, so it is worth showing a meaninful
error message in this scenario.
@codecov-commenter
Copy link

codecov-commenter commented Oct 24, 2023 •
edited

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (1bca5b8) 85.53% compared to head (41df2d3) 85.53%.

Additional details and impacted files 
@@             Coverage Diff              @@
##               main    #3158      +/-   ##
============================================
- Coverage     85.53%   85.53%   -0.01%     
- Complexity     2912     2914       +2     
============================================
  Files           333      334       +1     
  Lines          8856     8861       +5     
  Branches       1095     1096       +1     
============================================
+ Hits           7575     7579       +4     
  Misses          993      993              
- Partials        288      289       +1
Files Coverage Δ
...on/bytebuddy/InlineDelegateByteBuddyMockMaker.java 77.43% <0.00%> (-0.13%) ⬇️
...ito/internal/creation/bytebuddy/PlatformUtils.java 85.71% <85.71%> (ø)

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

@TimvdLippe
Copy link
Contributor

Is it possible to add a regression test for this? We have android-test already, maybe we can extend that.

@ascopes
Copy link
Contributor Author

ascopes commented Oct 24, 2023

@TimvdLippe will look into it; at the very least I will add unit tests for this logic as they don't appear to exist at the moment.

@ascopes
Copy link
Contributor Author

ascopes commented Oct 24, 2023

So unless I am missing something immediately obvious, I think this would be quite involved to set up a regression test for this specific case, since it'd require bundling an Android VM in the test packs (or use an external service like BrowserStack) so that Termux could be installed. Not sure if that would be worth the cost and complexity of implementing this since it would make the test pack potentially system-specific depending on how the subsystem itself would be run.

I will be pushing some unit tests for this in a few minutes though, so hopefully that should improve the coverage for those condition checks and provide a set of contracts for expected/unexpected cases that could trigger this condition.

ascopes
ascopes commented Oct 24, 2023
View reviewed changes
build.gradle Show resolved Hide resolved
@ascopes ascopes marked this pull request as draft October 24, 2023 09:02
@ascopes
Enable testing platform conditions
41df2d3 
- Implement unit tests for the ByteBuddy platform conditions
- Extract the ByteBuddy platform conditions to a separate utility class
  to enable unit testing them separately. This also enables adding additional
  checks in the future without making the InlineDelegateByteBuddyMockMaker
  much more complicated.
- Added dependency at test time for junit-jupiter-params to enable writing
  parameterized unit tests.
@ascopes ascopes marked this pull request as ready for review October 24, 2023 09:13
@TimvdLippe
Copy link
Contributor

Thanks! These regression tests look good to me, thanks for adding them.

@TimvdLippe TimvdLippe merged commit a8adbf5 into mockito:main Oct 24, 2023
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ascopes @codecov-commenter @TimvdLippe