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

Fixes #2007 : Downgrade objenesis version for mockito-android #2024

Merged
merged 8 commits into from Sep 1, 2020

Conversation

@kozaxinan
Copy link
Contributor

@kozaxinan kozaxinan commented Aug 26, 2020

check list

  • 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

Fixes #2007

After building, mockito android pom includes objenesis 2.6 while mockito-core has objenesis 3.1.

Locally build version is Tested with our internal project to verify fix.

  androidTestImplementation('com.nhaarman.mockitokotlin2:mockito-kotlin:x.y.z') {
    exclude group: 'org.mockito', module: 'mockito-core'
  }

pom of mockito-android.

<dependencies>
    <dependency>
      <groupId>org.mockito</groupId>
      <artifactId>mockito-core</artifactId>
      <version>3.5.8</version>
      <scope>compile</scope>
      <exclusions>
        <exclusion>
          <artifactId>objenesis</artifactId>
          <groupId>org.objenesis</groupId>
        </exclusion>
      </exclusions>
    </dependency>
    <dependency>
      <groupId>net.bytebuddy</groupId>
      <artifactId>byte-buddy-android</artifactId>
      <version>1.10.13</version>
      <scope>compile</scope>
    </dependency>
    <dependency>
      <groupId>org.objenesis</groupId>
      <artifactId>objenesis</artifactId>
      <version>2.6</version>
      <scope>compile</scope>
    </dependency>
  </dependencies>

Side note: I had problem with animal sniffer on release/3.x branch. I had to disable android api 24 signature check to compile the app.

org.mockito.internal.creation.bytebuddy.InlineByteBuddyMockMaker:507  Undefined reference (android-api-level-24-7.0_r2): java.lang.instrument.Instrumentation
org.mockito.internal.creation.bytebuddy.InlineByteBuddyMockMaker:507  Undefined reference (android-api-level-24-7.0_r2): boolean java.lang.instrument.Instrumentation.isModifiableClass(Class)

Breaking changes solutions for android modules

Solution 1

If you have motkito-kotlin with mockito-android, exclude mockito-core from mockito-kotlin.

        implementation("com.nhaarman.mockitokotlin2:mockito-kotlin:<version>") {
            exlcude group: "org.mockito", module: "mockito-core"
        }

Solution 2

Don't add mockito-core with mockito-android

Solution 3

Use dependency resolution

in root build.gradle

subprojects {
  configurations.configureEach {
    resolutionStrategy.eachDependency { details ->
      if (details.requested.group == 'org.objenesis') { 
        details.useVersion '2.6'
      }
    }
  }
}
@codecov-commenter
Copy link

@codecov-commenter codecov-commenter commented Aug 26, 2020

Codecov Report

Merging #2024 into release/3.x will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff               @@
##             release/3.x    #2024   +/-   ##
==============================================
  Coverage          84.88%   84.88%           
  Complexity          2703     2703           
==============================================
  Files                325      325           
  Lines               8198     8198           
  Branches             979      979           
==============================================
  Hits                6959     6959           
  Misses               968      968           
  Partials             271      271           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8007df7...8918202. Read the comment docs.

@kozaxinan
Copy link
Contributor Author

@kozaxinan kozaxinan commented Aug 27, 2020

Objenesis 2.6 version is now strictly defined in mockito-android. When mockito-core or mockito-kotlin is added as dependency, there will be version conflict for objenesis 2.6 and 3.1. For helping developer, because reason is added for strict version with suggestion for how to continue.

Could not resolve org.objenesis:objenesis:3.1.
     Required by:
         project :app > org.mockito:mockito-android:3.5.8 > org.mockito:mockito-core:3.5.8
      > Cannot find a version of 'org.objenesis:objenesis' that satisfies the version constraints:
           Dependency path 'is24-android:app:unspecified' --> 'org.mockito:mockito-android:3.5.8' --> 'org.objenesis:objenesis:{strictly 2.6}' because of the following reason: 
        --------------------------------------\     /-------------------------------------
        ---------------------------------------\   /--------------------------------------
        ----------------------------------------\ /---------------------------------------
        -----------------------------------------V----------------------------------------
        Mockito core uses Objenesis 3.x and Objenesis 3.x does not work with android api 25 and below.
        If you have mockito-core dependency with mockito-android, remove mockito-core.
        If you have mockito-kotlin, exclude mockito-core.
        implementation("com.nhaarman.mockitokotlin2:mockito-kotlin") {
            exclude group: "org.mockito", module: "mockito-core"
        }
        For more information please check;
            https://github.com/mockito/mockito/pull/2024
            https://github.com/mockito/mockito/pull/2007
        -----------------------------------------A----------------------------------------
        ----------------------------------------/ \---------------------------------------
        ---------------------------------------/   \--------------------------------------
        --------------------------------------/     \-------------------------------------
        
           Dependency path 'is24-android:app:unspecified' --> 'org.mockito:mockito-android:3.5.8' --> 'org.mockito:mockito-core:3.5.8' --> 'org.objenesis:objenesis:3.1'
Copy link
Contributor

@TimvdLippe TimvdLippe left a comment

This would work for me. @raphw WDYT?

subprojects/android/android.gradle Outdated Show resolved Hide resolved
subprojects/android/android.gradle Outdated Show resolved Hide resolved
@kozaxinan kozaxinan marked this pull request as ready for review Aug 27, 2020
kozaxinan added 2 commits Aug 27, 2020
@kozaxinan kozaxinan changed the title Fixes #2007 : Downgrade objenesis version for mockito-android (WIP) Fixes #2007 : Downgrade objenesis version for mockito-android Aug 28, 2020
@raphw raphw self-requested a review Sep 1, 2020
@raphw
Copy link
Member

@raphw raphw commented Sep 1, 2020

Looks good to me, should we maybe still avoid the ASCII art? This can be difficult to read in some CI environments, I think the message by itself should be sufficient?

@raphw
raphw approved these changes Sep 1, 2020
@kozaxinan
Copy link
Contributor Author

@kozaxinan kozaxinan commented Sep 1, 2020

I added asci art because when you have multiple conflicts, explanation text is between gradle messages. It might not be clear enough for inexperienced dev. That is just an assumption.

Please feel free to adjust if it is needed.

Copy link
Contributor

@TimvdLippe TimvdLippe left a comment

WDYT about this?

subprojects/android/android.gradle Outdated Show resolved Hide resolved
Co-authored-by: Tim van der Lippe <TimvdLippe@users.noreply.github.com>
@TimvdLippe TimvdLippe merged commit 626d457 into mockito:release/3.x Sep 1, 2020
4 checks passed
4 checks passed
@github-actions
Validation
Details
codecov/patch Coverage not affected when comparing 8007df7...8918202
Details
codecov/project 84.88% (+0.00%) compared to 8007df7
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@kozaxinan kozaxinan mentioned this pull request Sep 24, 2020
7 of 8 tasks complete
@madisoncfallin
Copy link

@madisoncfallin madisoncfallin commented Oct 13, 2020

Hey folks! I hate to beat a merged horse, but I'm having a ton of trouble building locally and it seems to be related to this issue. Can anyone offer me some pointers?

https://stackoverflow.com/questions/64341381/trouble-setting-up-mockito-for-local-development

PaulKlauser added a commit to PaulKlauser/mockito that referenced this pull request Mar 31, 2021
TimvdLippe pushed a commit that referenced this pull request Mar 31, 2021
* Revert "Fixes #2007 : Downgrade objenesis version for mockito-android (#2024)"

This reverts commit 626d457.

* Fixes #2007 : Update objenesis version to 3.2 to fix Android incompatibility
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

5 participants