[ 
https://issues.apache.org/jira/browse/HADOOP-14178?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16754648#comment-16754648
 ] 

Masatake Iwasaki commented on HADOOP-14178:
-------------------------------------------

Thanks for the update, [~ajisakaa].

> For those who want to review this patch, the patch does mainly two things:
 > 
 > * Replace Matcher.xxx with ArgumentMatcher.xxx to fix deprecation warnings
 > * Use ArgumentMatcher.any() instead of any(xxx.class)/anyYYY() when the 
 > tests want to match null

I've walked through the diffs of 032 side-by-side to see if this stands. LGTM 
with just 1 nit in TestKillAMPreemptionPolicy. I also checked results of tests 
having relatively large update on my local without seeing any problems. While 
we could have missed runtime errors in the tests disabled by default (such fs 
contract tests), the number of relevant files is so small. We should fix that 
in follow-up JIRAs.

I'm +1 if the below has rationale.

Is changing argument of times relevant to Mockit compatibility? The test passed 
with the patch applied. TestKillAMPreemptionPolicy.java:
{code:java}
@@ -17,7 +17,7 @@
  */
 package org.apache.hadoop.mapreduce.v2.app;
 
-import static org.mockito.Matchers.any;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
@@ -85,9 +85,9 @@ public void testKillAMPreemptPolicy() {
     policy.init(mActxt);
     pM = getPreemptionMessage(true, false, container);
     policy.preempt(mPctxt, pM);
-    verify(mActxt.getEventHandler(), times(2)).handle(
+    verify(mActxt.getEventHandler(), times(1)).handle(
         any(TaskAttemptEvent.class));
-    verify(mActxt.getEventHandler(), times(2)).handle(
+    verify(mActxt.getEventHandler(), times(1)).handle(
         any(JobCounterUpdateEvent.class));
 
     // strictContract is null & contract is not null
@@ -95,9 +95,9 @@ public void testKillAMPreemptPolicy() {
     policy.init(mActxt);
     pM = getPreemptionMessage(false, true, container);
     policy.preempt(mPctxt, pM);
-    verify(mActxt.getEventHandler(), times(2)).handle(
+    verify(mActxt.getEventHandler(), times(1)).handle(
         any(TaskAttemptEvent.class));
-    verify(mActxt.getEventHandler(), times(2)).handle(
+    verify(mActxt.getEventHandler(), times(1)).handle(
         any(JobCounterUpdateEvent.class));
 
     // strictContract is not null & contract is not null
@@ -105,9 +105,9 @@ public void testKillAMPreemptPolicy() {
     policy.init(mActxt);
     pM = getPreemptionMessage(true, true, container);
     policy.preempt(mPctxt, pM);
-    verify(mActxt.getEventHandler(), times(4)).handle(
+    verify(mActxt.getEventHandler(), times(2)).handle(
         any(TaskAttemptEvent.class));
-    verify(mActxt.getEventHandler(), times(4)).handle(
+    verify(mActxt.getEventHandler(), times(2)).handle(
         any(JobCounterUpdateEvent.class));
   }
{code}

> Move Mockito up to version 2.x
> ------------------------------
>
>                 Key: HADOOP-14178
>                 URL: https://issues.apache.org/jira/browse/HADOOP-14178
>             Project: Hadoop Common
>          Issue Type: Sub-task
>          Components: test
>    Affects Versions: 2.9.0
>            Reporter: Steve Loughran
>            Assignee: Akira Ajisaka
>            Priority: Major
>         Attachments: HADOOP-14178.001.patch, HADOOP-14178.002.patch, 
> HADOOP-14178.003.patch, HADOOP-14178.004.patch, HADOOP-14178.005-wip.patch, 
> HADOOP-14178.005-wip2.patch, HADOOP-14178.005-wip3.patch, 
> HADOOP-14178.005-wip4.patch, HADOOP-14178.005-wip5.patch, 
> HADOOP-14178.005-wip6.patch, HADOOP-14178.005.patch, HADOOP-14178.006.patch, 
> HADOOP-14178.007.patch, HADOOP-14178.008.patch, HADOOP-14178.009.patch, 
> HADOOP-14178.010.patch, HADOOP-14178.011.patch, HADOOP-14178.012.patch, 
> HADOOP-14178.013.patch, HADOOP-14178.014.patch, HADOOP-14178.015.patch, 
> HADOOP-14178.016.patch, HADOOP-14178.017.patch, HADOOP-14178.018.patch, 
> HADOOP-14178.019.patch, HADOOP-14178.020.patch, HADOOP-14178.021.patch, 
> HADOOP-14178.022.patch, HADOOP-14178.023.patch, HADOOP-14178.024.patch, 
> HADOOP-14178.025.patch, HADOOP-14178.026.patch, HADOOP-14178.027.patch, 
> HADOOP-14178.028.patch, HADOOP-14178.029.patch, HADOOP-14178.030.patch, 
> HADOOP-14178.031.patch, HADOOP-14178.032.patch
>
>
> 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|https://github.com/mockito/mockito/wiki/What%27s-new-in-Mockito-2]. 
> 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.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

Reply via email to