Details

    • Type: Sub-task
    • Status: Open
    • Priority: Major
    • Resolution: Unresolved
    • Affects Version/s: 2.9.0
    • Fix Version/s: None
    • Component/s: test
    • Labels:
      None

      Description

      I don't know when Hadoop picked up Mockito, but it has been frozen at 1.8.5 since the switch to maven in 2011.

      Mockito is now at version 2.1, with lots of Java 8 support. That' s not just defining actions as closures, but in supporting Optional types, mocking methods in interfaces, etc.

      It's only used for testing, and, provided there aren't regressions, cost of upgrade is low. The good news: test tools usually come with good test coverage. The bad: mockito does go deep into java bytecodes.

        Issue Links

          Activity

          Hide
          stevel@apache.org Steve Loughran added a comment -

          Not an easy update, at least from one review https://asolntsev.github.io/en/2016/10/11/mockito-2.1/

          No just from API changes, but a stricter tool finds problems in existing mock declarations. And, given how the outcome on failed mocks is usually a stack trace of hard-to-diagnose issues, that could take time.

          The change long makes it clear that for full java 8 support, the update is needed. But: as its not a published dependency of any production artifact, just the test ones, it is a JAR which can be upgraded with less risk than others

          Show
          stevel@apache.org Steve Loughran added a comment - Not an easy update, at least from one review https://asolntsev.github.io/en/2016/10/11/mockito-2.1/ No just from API changes, but a stricter tool finds problems in existing mock declarations. And, given how the outcome on failed mocks is usually a stack trace of hard-to-diagnose issues, that could take time. The change long makes it clear that for full java 8 support, the update is needed. But: as its not a published dependency of any production artifact, just the test ones, it is a JAR which can be upgraded with less risk than others
          Hide
          anujjain Anuj Jain added a comment -

          Can I take up this task.I am new to hadoop project and I am looking for a task to start contributing

          Show
          anujjain Anuj Jain added a comment - Can I take up this task.I am new to hadoop project and I am looking for a task to start contributing
          Hide
          stevel@apache.org Steve Loughran added a comment - - edited

          Anuj, I would advise you to tread carefully with this one, as it's a bigger patch than it looks

          • updating POMs have dramatic consequences downstream
          • if the imports change, it will span more of the code
          • If it finds bugs in tests, the tests need to be changed, which means that the patch will need to work out the real intent of the test, otherwise the fix could actually hide problems.

          I think we'd be scared of any big patch for this. A small one, fine. But big: it'll span the hadoop modules, need review, and so likely to stutter along.

          Show
          stevel@apache.org Steve Loughran added a comment - - edited Anuj, I would advise you to tread carefully with this one, as it's a bigger patch than it looks updating POMs have dramatic consequences downstream if the imports change, it will span more of the code If it finds bugs in tests, the tests need to be changed, which means that the patch will need to work out the real intent of the test, otherwise the fix could actually hide problems. I think we'd be scared of any big patch for this. A small one, fine. But big: it'll span the hadoop modules, need review, and so likely to stutter along.
          Hide
          anujjain Anuj Jain added a comment -

          Steve, I understand what you mean.Can you please help me choosing a smaller task to start.

          Show
          anujjain Anuj Jain added a comment - Steve, I understand what you mean.Can you please help me choosing a smaller task to start.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Why not get on the common developer list and ask for advice. Right now, this week, one where help would be appreciated is probably just in downloading, testing the Hadoop 2.8.0 RC that's been put up

          Show
          stevel@apache.org Steve Loughran added a comment - Why not get on the common developer list and ask for advice. Right now, this week, one where help would be appreciated is probably just in downloading, testing the Hadoop 2.8.0 RC that's been put up
          Hide
          ajisakaa Akira Ajisaka added a comment -

          The upgrade will be very big change, so I'd like to split this into some sub-tasks to reduce the cost of review. The first sub-task is to remove the usage of Whitebox, which was removed in Mockito 2.1. I'll add sub-tasks later if needed.

          Show
          ajisakaa Akira Ajisaka added a comment - The upgrade will be very big change, so I'd like to split this into some sub-tasks to reduce the cost of review. The first sub-task is to remove the usage of Whitebox, which was removed in Mockito 2.1. I'll add sub-tasks later if needed.
          Hide
          ajisakaa Akira Ajisaka added a comment -
          Show
          ajisakaa Akira Ajisaka added a comment - Filed HADOOP-14188
          Hide
          ajisakaa Akira Ajisaka added a comment -

          Filed HADOOP-14245 for replacing Mockito.stub with Mockito.when.

          For ArgumentMatcher, we need to fix the code when upgrading Mockito.
          https://github.com/mockito/mockito/blob/v2.7.19/src/main/java/org/mockito/ArgumentMatcher.java#L88

           * All existing custom implementations of <code>ArgumentMatcher</code> will no longer compile.
           * All locations where hamcrest matchers are passed to <code>argThat()</code> will no longer compile.
           * There are 2 approaches to fix the problems:
           * <ul>
           * <li>a) Refactor the hamcrest matcher to Mockito matcher:
           * Use "implements ArgumentMatcher" instead of "extends ArgumentMatcher".
           * Then refactor <code>describeTo()</code> method into <code>toString()</code> method.
           * </li>
           * <li>
           * b) Use <code>org.mockito.hamcrest.MockitoHamcrest.argThat()</code> instead of <code>Mockito.argThat()</code>.
           * Ensure that there is <a href="http://hamcrest.org/JavaHamcrest/">hamcrest</a> dependency on classpath
           * (Mockito does not depend on hamcrest any more).
          

          I prefer option a) because it does not depend on hamcrest.

          Show
          ajisakaa Akira Ajisaka added a comment - Filed HADOOP-14245 for replacing Mockito.stub with Mockito.when. For ArgumentMatcher, we need to fix the code when upgrading Mockito. https://github.com/mockito/mockito/blob/v2.7.19/src/main/java/org/mockito/ArgumentMatcher.java#L88 * All existing custom implementations of <code>ArgumentMatcher</code> will no longer compile. * All locations where hamcrest matchers are passed to <code>argThat()</code> will no longer compile. * There are 2 approaches to fix the problems: * <ul> * <li>a) Refactor the hamcrest matcher to Mockito matcher: * Use " implements ArgumentMatcher" instead of " extends ArgumentMatcher" . * Then refactor <code>describeTo()</code> method into <code>toString()</code> method. * </li> * <li> * b) Use <code>org.mockito.hamcrest.MockitoHamcrest.argThat()</code> instead of <code>Mockito.argThat()</code>. * Ensure that there is <a href= "http: //hamcrest.org/JavaHamcrest/" >hamcrest</a> dependency on classpath * (Mockito does not depend on hamcrest any more). I prefer option a) because it does not depend on hamcrest.
          Hide
          ajisakaa Akira Ajisaka added a comment -

          FYI: This blocks HADOOP-11123 since Mockito 1.x does not support Java9.

          Show
          ajisakaa Akira Ajisaka added a comment - FYI: This blocks HADOOP-11123 since Mockito 1.x does not support Java9.

            People

            • Assignee:
              ajisakaa Akira Ajisaka
              Reporter:
              stevel@apache.org Steve Loughran
            • Votes:
              2 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:

                Development