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

[SPARK-26536][BUILD][TEST] Upgrade Mockito to 2.23.4 #23452

Closed
wants to merge 3 commits into from
Closed

[SPARK-26536][BUILD][TEST] Upgrade Mockito to 2.23.4 #23452

wants to merge 3 commits into from

Conversation

dongjoon-hyun
Copy link
Member

@dongjoon-hyun dongjoon-hyun commented Jan 4, 2019

What changes were proposed in this pull request?

This PR upgrades Mockito from 1.10.19 to 2.23.4. The following changes are required.

  • Replace org.mockito.Matchers with org.mockito.ArgumentMatchers
  • Replace anyObject with any
  • Replace getArgumentAt with getArgument and add type annotation.
  • Use isNull matcher in case of null is invoked.
     saslHandler.channelInactive(null);
-    verify(handler).channelInactive(any(TransportClient.class));
+    verify(handler).channelInactive(isNull());
  • Make and use doReturn wrapper to avoid SI-4775
private def doReturn(value: Any) = org.mockito.Mockito.doReturn(value, Seq.empty: _*)

How was this patch tested?

Pass the Jenkins with the existing tests.

@SparkQA
Copy link

SparkQA commented Jan 4, 2019

Test build #100742 has finished for PR 23452 at commit 9c1119b.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jan 4, 2019

Test build #100749 has finished for PR 23452 at commit d199657.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@@ -79,6 +79,8 @@ public void testRegisterExecutor() {
@SuppressWarnings("unchecked")
@Test
public void testOpenShuffleBlocks() {
when(client.getClientId()).thenReturn("app0");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This UT uses app0 as String appId parameter.

switchCount++;
verify(parser, times(switchCount)).handle(eq(switchNames[0]), same(null));
verify(parser, times(count)).handle(anyString(), any(String.class));
verify(parser, times(nullCount)).handle(anyString(), isNull());
verify(parser, times(count - nullCount)).handle(anyString(), any(String.class));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mockito2 distinguishes null cases and non-null cases. We need to split the verification logics to check both cases.

@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Jan 4, 2019

Hi, @srowen , @vanzin , @dbtsai , @HyukjinKwon .
Could you review this Mockito upgrade PR for JDK11 support?
The last commit fixes the one failure reported at previous Jenkins run. So, Jenkins will pass at this time.

@dongjoon-hyun
Copy link
Member Author

Thank you for review and approval, @srowen .

@dongjoon-hyun
Copy link
Member Author

Thank you for review and approval, @HyukjinKwon .

@SparkQA
Copy link

SparkQA commented Jan 5, 2019

Test build #100760 has finished for PR 23452 at commit 1dc2710.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@dongjoon-hyun
Copy link
Member Author

Thank you all.
Merged to master.

@asfgit asfgit closed this in e15a319 Jan 5, 2019
@dongjoon-hyun dongjoon-hyun deleted the SPARK-26536 branch January 5, 2019 03:54
jackylee-ch pushed a commit to jackylee-ch/spark that referenced this pull request Feb 18, 2019
## What changes were proposed in this pull request?

This PR upgrades Mockito from 1.10.19 to 2.23.4. The following changes are required.

- Replace `org.mockito.Matchers` with `org.mockito.ArgumentMatchers`
- Replace `anyObject` with `any`
- Replace `getArgumentAt` with `getArgument` and add type annotation.
- Use `isNull` matcher in case of `null` is invoked.
```scala
     saslHandler.channelInactive(null);
-    verify(handler).channelInactive(any(TransportClient.class));
+    verify(handler).channelInactive(isNull());
```

- Make and use `doReturn` wrapper to avoid [SI-4775](https://issues.scala-lang.org/browse/SI-4775)
```scala
private def doReturn(value: Any) = org.mockito.Mockito.doReturn(value, Seq.empty: _*)
```

## How was this patch tested?

Pass the Jenkins with the existing tests.

Closes apache#23452 from dongjoon-hyun/SPARK-26536.

Authored-by: Dongjoon Hyun <dongjoon@apache.org>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
Agirish pushed a commit to HPEEzmeral/apache-spark that referenced this pull request May 5, 2022
## What changes were proposed in this pull request?

This PR upgrades Mockito from 1.10.19 to 2.23.4. The following changes are required.

- Replace `org.mockito.Matchers` with `org.mockito.ArgumentMatchers`
- Replace `anyObject` with `any`
- Replace `getArgumentAt` with `getArgument` and add type annotation.
- Use `isNull` matcher in case of `null` is invoked.
```scala
     saslHandler.channelInactive(null);
-    verify(handler).channelInactive(any(TransportClient.class));
+    verify(handler).channelInactive(isNull());
```

- Make and use `doReturn` wrapper to avoid [SI-4775](https://issues.scala-lang.org/browse/SI-4775)
```scala
private def doReturn(value: Any) = org.mockito.Mockito.doReturn(value, Seq.empty: _*)
```

## How was this patch tested?

Pass the Jenkins with the existing tests.

Closes apache#23452 from dongjoon-hyun/SPARK-26536.

Authored-by: Dongjoon Hyun <dongjoon@apache.org>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

(cherry picked from commit e15a319)
udaynpusa pushed a commit to mapr/spark that referenced this pull request Jan 30, 2024
## What changes were proposed in this pull request?

This PR upgrades Mockito from 1.10.19 to 2.23.4. The following changes are required.

- Replace `org.mockito.Matchers` with `org.mockito.ArgumentMatchers`
- Replace `anyObject` with `any`
- Replace `getArgumentAt` with `getArgument` and add type annotation.
- Use `isNull` matcher in case of `null` is invoked.
```scala
     saslHandler.channelInactive(null);
-    verify(handler).channelInactive(any(TransportClient.class));
+    verify(handler).channelInactive(isNull());
```

- Make and use `doReturn` wrapper to avoid [SI-4775](https://issues.scala-lang.org/browse/SI-4775)
```scala
private def doReturn(value: Any) = org.mockito.Mockito.doReturn(value, Seq.empty: _*)
```

## How was this patch tested?

Pass the Jenkins with the existing tests.

Closes apache#23452 from dongjoon-hyun/SPARK-26536.

Authored-by: Dongjoon Hyun <dongjoon@apache.org>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

(cherry picked from commit e15a319)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants