Uploaded image for project: 'Solr'
  1. Solr
  2. SOLR-9966

Convert/migrate tests using EasyMock to Mockito

    Details

    • Type: Task
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 6.5, 7.0
    • Component/s: Tests
    • Security Level: Public (Default Security Level. Issues are Public)
    • Labels:

      Description

      In SOLR-9893 I disabled all tests on Java 9 that use EasyMock, because Easymock is not compatible with Java 9 (it uses outdated cglib version that does not work with Jigsaw module system). To me the project seems dead (no releases since more than 2 years).

      Mockito latest version is compatible to Java 9 because it no longer uses cglib and the more modern and powerful Byte-Buddy lib; SOLR-9893 updated to it.

      I found this about more or less "automated rewrite" of EasyMock tests to Mockito:

      It is not many tests, so this would be a great cleanup:

      • core/src/test/org/apache/solr/cloud/ClusterStateTest.java
      • core/src/test/org/apache/solr/cloud/OverseerCollectionConfigSetProcessorTest.java
      • core/src/test/org/apache/solr/core/BlobRepositoryMockingTest.java
      • core/src/test/org/apache/solr/core/CoreSorterTest.java
      • core/src/test/org/apache/solr/security/TestPKIAuthenticationPlugin.java
      • core/src/test/org/apache/solr/servlet/SolrRequestParserTest.java
      • solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientCacheTest.java

      There is one special case:

      • contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestJdbcDataSource.java

      I am not sure how to convert this one, because it uses some strange system properties and a handler that intercepts some EasyMock stuff. I may need help to convert that one!

      After this is resolved we can remove the following dependencies from Solr:

      • cglib-nodep
      • easymock
      1. SOLR-9966.patch
        86 kB
        Cao Manh Dat

        Issue Links

          Activity

          Hide
          henri Henri Tremblay added a comment -

          Hi. EasyMock is not dead. I still maintain it. And sadly, this issue has never been raised. That would have been helpful. I've fixed Objenesis for Java 9 last week. EasyMock will follow now that I'm aware of a problem.

          I'm expecting a release soon. I also don't think migrating worth the effort. You won't get much benefit.

          That said, EasyMock is a nice framework and easy to dive in. It's perfect for someone you wants to get involved in a technical open source framework. And we are looking for help. Because indeed, right now, there is about one release a year (the last one was in September 2015. So not 2 years)

          Show
          henri Henri Tremblay added a comment - Hi. EasyMock is not dead. I still maintain it. And sadly, this issue has never been raised. That would have been helpful. I've fixed Objenesis for Java 9 last week. EasyMock will follow now that I'm aware of a problem. I'm expecting a release soon. I also don't think migrating worth the effort. You won't get much benefit. That said, EasyMock is a nice framework and easy to dive in. It's perfect for someone you wants to get involved in a technical open source framework. And we are looking for help. Because indeed, right now, there is about one release a year (the last one was in September 2015. So not 2 years)
          Hide
          thetaphi Uwe Schindler added a comment -

          Hi Henri,
          the problem is that Solr tests are using 2 different frameworks for mocking and a cleanup would be fine. So based on statistics on Solr's Code Mockito is more used and EasyMock is only used with some legacy tests.
          To fix the Java 9 problem, you have to first wait for a release of CGLIB.
          Uwe

          Show
          thetaphi Uwe Schindler added a comment - Hi Henri, the problem is that Solr tests are using 2 different frameworks for mocking and a cleanup would be fine. So based on statistics on Solr's Code Mockito is more used and EasyMock is only used with some legacy tests. To fix the Java 9 problem, you have to first wait for a release of CGLIB. Uwe
          Hide
          thetaphi Uwe Schindler added a comment -

          FYI, the CGLIB issue is: https://github.com/cglib/cglib/issues/93
          It is fixed in Git, but as far as I know, no new release.

          Show
          thetaphi Uwe Schindler added a comment - FYI, the CGLIB issue is: https://github.com/cglib/cglib/issues/93 It is fixed in Git, but as far as I know, no new release.
          Hide
          thetaphi Uwe Schindler added a comment -
          Show
          thetaphi Uwe Schindler added a comment - Easymock issue: https://github.com/easymock/easymock/issues/193
          Hide
          caomanhdat Cao Manh Dat added a comment -

          A patch for this ticket. If no one have problem with this patch, I will commit it shortly.

          Show
          caomanhdat Cao Manh Dat added a comment - A patch for this ticket. If no one have problem with this patch, I will commit it shortly.
          Hide
          thetaphi Uwe Schindler added a comment - - edited

          Yeah, commit it! Thanks. I just looked over it, but my mocking framework knowledge is minimal... So no deep checks!

          I started with converting it, but as I am no specialist, I gave up after some time. But one thing I can confirm: My patch for the InitialContextFactory for the JDBC tests in dataimporthandler looked identical - including the lambda

          You should also remove the cglib dependency completely - its useless. Please also remove the license files of easymock and cglib!

          Show
          thetaphi Uwe Schindler added a comment - - edited Yeah, commit it! Thanks. I just looked over it, but my mocking framework knowledge is minimal... So no deep checks! I started with converting it, but as I am no specialist, I gave up after some time. But one thing I can confirm: My patch for the InitialContextFactory for the JDBC tests in dataimporthandler looked identical - including the lambda You should also remove the cglib dependency completely - its useless. Please also remove the license files of easymock and cglib!
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 46ef9256b44d78fb4ade339652795255d97078d5 in lucene-solr's branch refs/heads/master from Cao Manh Dat
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=46ef925 ]

          SOLR-9966: Convert/migrate tests using EasyMock to Mockito

          Show
          jira-bot ASF subversion and git services added a comment - Commit 46ef9256b44d78fb4ade339652795255d97078d5 in lucene-solr's branch refs/heads/master from Cao Manh Dat [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=46ef925 ] SOLR-9966 : Convert/migrate tests using EasyMock to Mockito
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 33845f73721c6090163ff869a669557350b8a233 in lucene-solr's branch refs/heads/branch_6x from Cao Manh Dat
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=33845f7 ]

          SOLR-9966: Convert/migrate tests using EasyMock to Mockito

          Show
          jira-bot ASF subversion and git services added a comment - Commit 33845f73721c6090163ff869a669557350b8a233 in lucene-solr's branch refs/heads/branch_6x from Cao Manh Dat [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=33845f7 ] SOLR-9966 : Convert/migrate tests using EasyMock to Mockito
          Hide
          caomanhdat Cao Manh Dat added a comment -
          Show
          caomanhdat Cao Manh Dat added a comment - Thanks Uwe Schindler
          Hide
          ichattopadhyaya Ishan Chattopadhyaya added a comment -

          Seems like there is a reproducible failure on jenkins due to these changes, affecting the TestApiFramework suite.
          https://jenkins.thetaphi.de/job/Lucene-Solr-6.x-Linux/2889/
          Cao Manh Dat, Uwe Schindler, can you please take a look?

          Show
          ichattopadhyaya Ishan Chattopadhyaya added a comment - Seems like there is a reproducible failure on jenkins due to these changes, affecting the TestApiFramework suite. https://jenkins.thetaphi.de/job/Lucene-Solr-6.x-Linux/2889/ Cao Manh Dat , Uwe Schindler , can you please take a look?
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 54303260aa3b1c539fadb2bcaa5db04b46bd9a9d in lucene-solr's branch refs/heads/master from Cao Manh Dat
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=5430326 ]

          SOLR-9966: Fix previous commit bug

          Show
          jira-bot ASF subversion and git services added a comment - Commit 54303260aa3b1c539fadb2bcaa5db04b46bd9a9d in lucene-solr's branch refs/heads/master from Cao Manh Dat [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=5430326 ] SOLR-9966 : Fix previous commit bug
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 76ca4f07a17387f1839e42be9e8e581c94988c80 in lucene-solr's branch refs/heads/branch_6x from Cao Manh Dat
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=76ca4f0 ]

          SOLR-9966: Fix previous commit bug

          Show
          jira-bot ASF subversion and git services added a comment - Commit 76ca4f07a17387f1839e42be9e8e581c94988c80 in lucene-solr's branch refs/heads/branch_6x from Cao Manh Dat [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=76ca4f0 ] SOLR-9966 : Fix previous commit bug
          Hide
          caomanhdat Cao Manh Dat added a comment -

          Uwe Schindler : I found that, in JDK9 Class.forname(<class of mock object>) will throw a ClassNotFoundException. But in JDK8 that exception was not thrown. I temporary ignore TestJDBCDataSource#testRetrieveFromDriverManager on JDK9. Do you have any idea about this bug?

          Show
          caomanhdat Cao Manh Dat added a comment - Uwe Schindler : I found that, in JDK9 Class.forname(<class of mock object>) will throw a ClassNotFoundException. But in JDK8 that exception was not thrown. I temporary ignore TestJDBCDataSource#testRetrieveFromDriverManager on JDK9. Do you have any idea about this bug?
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 6af020399bd144ab8f1a9ae97af85f07d8c1258d in lucene-solr's branch refs/heads/master from Cao Manh Dat
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=6af0203 ]

          SOLR-9966: Fix ant precommit by removing cglib

          Show
          jira-bot ASF subversion and git services added a comment - Commit 6af020399bd144ab8f1a9ae97af85f07d8c1258d in lucene-solr's branch refs/heads/master from Cao Manh Dat [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=6af0203 ] SOLR-9966 : Fix ant precommit by removing cglib
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 31211d088f9666439017f9a8cdcc5be7d132f751 in lucene-solr's branch refs/heads/branch_6x from Cao Manh Dat
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=31211d0 ]

          SOLR-9966: Fix ant precommit by removing cglib

          Show
          jira-bot ASF subversion and git services added a comment - Commit 31211d088f9666439017f9a8cdcc5be7d132f751 in lucene-solr's branch refs/heads/branch_6x from Cao Manh Dat [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=31211d0 ] SOLR-9966 : Fix ant precommit by removing cglib
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit eb0be0574c3281ff5218dc4b5f3f5e59220acbe8 in lucene-solr's branch refs/heads/master from Uwe Schindler
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=eb0be05 ]

          SOLR-9966: Fix check-licenses precommit

          Show
          jira-bot ASF subversion and git services added a comment - Commit eb0be0574c3281ff5218dc4b5f3f5e59220acbe8 in lucene-solr's branch refs/heads/master from Uwe Schindler [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=eb0be05 ] SOLR-9966 : Fix check-licenses precommit
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 8a965def6619ab57cdb9418e4158e69479020428 in lucene-solr's branch refs/heads/branch_6x from Uwe Schindler
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=8a965de ]

          SOLR-9966: Fix check-licenses precommit

          Show
          jira-bot ASF subversion and git services added a comment - Commit 8a965def6619ab57cdb9418e4158e69479020428 in lucene-solr's branch refs/heads/branch_6x from Uwe Schindler [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=8a965de ] SOLR-9966 : Fix check-licenses precommit
          Hide
          thetaphi Uwe Schindler added a comment -

          I just fixed another precommit.

          Show
          thetaphi Uwe Schindler added a comment - I just fixed another precommit.
          Hide
          thetaphi Uwe Schindler added a comment -

          Hi,

          I will try to figure out the call Class#forName does not work with a mock class (I have a strong feeling why it is like this). I would create the ContextFactory as a standard non-mock class and assign that one to the factory property. This may also be a bug in the JDBC code of Java 9 (if it does not work with a non-mock class, too).

          Nevertheless, disabling a test should be done with assumeFalse("description why it does not work", Constants.JRE_IS_MINIMUM_JAVA9);

          Show
          thetaphi Uwe Schindler added a comment - Hi, I will try to figure out the call Class#forName does not work with a mock class (I have a strong feeling why it is like this). I would create the ContextFactory as a standard non-mock class and assign that one to the factory property. This may also be a bug in the JDBC code of Java 9 (if it does not work with a non-mock class, too). Nevertheless, disabling a test should be done with assumeFalse("description why it does not work", Constants.JRE_IS_MINIMUM_JAVA9);
          Hide
          thetaphi Uwe Schindler added a comment -

          Sorry, misunderstood your comment: The problem is with passing "driver" to theDriverManager, not the InitialContextFactory.

          On Java 9 with module system, Class.forName() cannot work, as the class is injected into the JVM using special sun.misc.Unsafe tricks as anonymous class (without any public class name, you only see "codegen.java.sql.Driver$MockitoMock$...randomunicodechars..."). The class name is just there for display purposes, but you cannot load it. In fact this could also break with Java 8, if Mockito would implement injecting of mock classes different there (e.g. on IBM's JVM).

          So this test should be removed or rewritten to not use class names and instead pass instances. The class names of Mock classes can by default not used to call Class#forName.

          Show
          thetaphi Uwe Schindler added a comment - Sorry, misunderstood your comment: The problem is with passing "driver" to theDriverManager, not the InitialContextFactory. On Java 9 with module system, Class.forName() cannot work, as the class is injected into the JVM using special sun.misc.Unsafe tricks as anonymous class (without any public class name, you only see "codegen.java.sql.Driver$MockitoMock$...randomunicodechars..."). The class name is just there for display purposes, but you cannot load it. In fact this could also break with Java 8, if Mockito would implement injecting of mock classes different there (e.g. on IBM's JVM). So this test should be removed or rewritten to not use class names and instead pass instances. The class names of Mock classes can by default not used to call Class#forName.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 7e30fe147193105ec36265b03527142e48623a18 in lucene-solr's branch refs/heads/master from Uwe Schindler
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=7e30fe1 ]

          SOLR-9966: Do test-ignore properly

          Show
          jira-bot ASF subversion and git services added a comment - Commit 7e30fe147193105ec36265b03527142e48623a18 in lucene-solr's branch refs/heads/master from Uwe Schindler [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=7e30fe1 ] SOLR-9966 : Do test-ignore properly
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit ccd9733fa24d9c783b544e2cb4a0278ba064e3fc in lucene-solr's branch refs/heads/branch_6x from Uwe Schindler
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=ccd9733 ]

          SOLR-9966: Do test-ignore properly

          Show
          jira-bot ASF subversion and git services added a comment - Commit ccd9733fa24d9c783b544e2cb4a0278ba064e3fc in lucene-solr's branch refs/heads/branch_6x from Uwe Schindler [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=ccd9733 ] SOLR-9966 : Do test-ignore properly
          Hide
          caomanhdat Cao Manh Dat added a comment - - edited
          Show
          caomanhdat Cao Manh Dat added a comment - - edited Thanks Uwe Schindler , Ishan Chattopadhyaya !

            People

            • Assignee:
              caomanhdat Cao Manh Dat
              Reporter:
              thetaphi Uwe Schindler
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development