Skip to content

TEZ-4383: upgrade to mockito 4.3.1 - #190

Merged
abstractdog merged 3 commits into
apache:masterfrom
LA-Toth:a/TEZ-4383
Jun 6, 2022
Merged

TEZ-4383: upgrade to mockito 4.3.1#190
abstractdog merged 3 commits into
apache:masterfrom
LA-Toth:a/TEZ-4383

Conversation

@LA-Toth

Copy link
Copy Markdown
Contributor

No description provided.

@tez-yetus

This comment was marked as outdated.

}

@Test (timeout=1000)
@Test (timeout=3000)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this change needed?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I'm not sure. Initially this test timed out with the original timeout value. I double check this one.

import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.*;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please remove wildcard import, we usually refrain from doing that
by removing it we can stay consistent with other parts of the code

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Cool, OK, consistency is also important for me.
But the tests don't always do that, namely the any() and the other matchers can be imported both as org.mockito.Mockito.any and as org.mockito.ArgumentMatchers.any. I would keep only one of these. The org.mockito.Mockito.any seems to be the better choice.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this looks confusing from mockito side, can you please clarify the difference between them?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

These are the same. Basically the same code (function) is added to both the ArgumentMatchers and the Mockito classes. So in theory there is no real difference.

@tez-yetus

This comment was marked as outdated.

@tez-yetus

This comment was marked as outdated.

@tez-yetus

This comment was marked as outdated.

@tez-yetus

This comment was marked as outdated.

@tez-yetus

This comment was marked as outdated.

@tez-yetus

This comment was marked as outdated.

@tez-yetus

This comment was marked as outdated.

@tez-yetus

This comment was marked as outdated.

@tez-yetus

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec16m 44sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 1sNo case conflicting files found.
+1 💚@author0m 0sThe patch does not contain any @author tags.
+1 💚test4tests0m 0sThe patch appears to include 61 new or modified test files.
_ master Compile Tests _
+0 🆗mvndep6m 33sMaven dependency ordering for branch
+1 💚mvninstall10m 11smaster passed
+1 💚compile11m 58smaster passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚compile11m 8smaster passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚checkstyle9m 23smaster passed
+1 💚javadoc10m 41smaster passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚javadoc9m 44smaster passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+0 🆗spotbugs0m 51sUsed deprecated FindBugs config; considering switching to SpotBugs.
+0 🆗findbugs6m 23sroot in master has 2 extant findbugs warnings.
+0 🆗findbugs0m 50stez-tools/analyzers/job-analyzer in master has 4 extant findbugs warnings.
_ Patch Compile Tests _
+0 🆗mvndep0m 21sMaven dependency ordering for patch
-1 ❌mvninstall0m 20stez-runtime-library in the patch failed.
-1 ❌mvninstall0m 17stez-protobuf-history-plugin in the patch failed.
+1 💚compile11m 57sthe patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚javac11m 57sthe patch passed
+1 💚compile11m 6sthe patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚javac11m 6sthe patch passed
+1 💚checkstyle0m 33sThe patch passed checkstyle in tez-api
+1 💚checkstyle0m 29sThe patch passed checkstyle in tez-common
+1 💚checkstyle0m 31sThe patch passed checkstyle in tez-runtime-internals
+1 💚checkstyle0m 43stez-runtime-library: The patch generated 0 new + 310 unchanged - 23 fixed = 310 total (was 333)
+1 💚checkstyle0m 32sThe patch passed checkstyle in tez-mapreduce
+1 💚checkstyle0m 55stez-dag: The patch generated 0 new + 675 unchanged - 7 fixed = 675 total (was 682)
+1 💚checkstyle0m 29sThe patch passed checkstyle in tez-ext-service-tests
+1 💚checkstyle0m 30sThe patch passed checkstyle in tez-protobuf-history-plugin
+1 💚checkstyle0m 30stez-plugins/tez-yarn-timeline-history: The patch generated 0 new + 38 unchanged - 19 fixed = 38 total (was 57)
+1 💚checkstyle0m 29sThe patch passed checkstyle in tez-yarn-timeline-history-with-acls
+1 💚checkstyle0m 29stez-plugins/tez-yarn-timeline-history-with-fs: The patch generated 0 new + 22 unchanged - 3 fixed = 22 total (was 25)
+1 💚checkstyle0m 29sThe patch passed checkstyle in tez-history-parser
+1 💚checkstyle0m 31sThe patch passed checkstyle in tez-aux-services
+1 💚checkstyle0m 29sThe patch passed checkstyle in job-analyzer
+1 💚checkstyle1m 25sroot: The patch generated 0 new + 1195 unchanged - 52 fixed = 1195 total (was 1247)
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚xml0m 18sThe patch has no ill-formed XML file.
+1 💚javadoc10m 38sthe patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚javadoc9m 43sthe patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚findbugs21m 16sthe patch passed
_ Other Tests _
+1 💚unit2m 32stez-api in the patch passed.
+1 💚unit0m 46stez-common in the patch passed.
+1 💚unit0m 58stez-runtime-internals in the patch passed.
+1 💚unit5m 15stez-runtime-library in the patch passed.
+1 💚unit1m 39stez-mapreduce in the patch passed.
+1 💚unit5m 21stez-dag in the patch passed.
+1 💚unit4m 16stez-ext-service-tests in the patch passed.
+1 💚unit0m 45stez-protobuf-history-plugin in the patch passed.
+1 💚unit1m 54stez-yarn-timeline-history in the patch passed.
+1 💚unit2m 7stez-yarn-timeline-history-with-acls in the patch passed.
+1 💚unit1m 53stez-yarn-timeline-history-with-fs in the patch passed.
+1 💚unit2m 42stez-history-parser in the patch passed.
+1 💚unit3m 2stez-aux-services in the patch passed.
+1 💚unit2m 59sjob-analyzer in the patch passed.
+1 💚unit71m 4sroot in the patch passed.
+1 💚asflicense8m 23sThe patch does not generate ASF License warnings.
310m 54s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-190/10/artifact/out/Dockerfile
GITHUB PR#190
JIRA IssueTEZ-4383
Optional Testsdupname asflicense javac javadoc unit xml compile spotbugs findbugs checkstyle
unameLinux 70720f1c102e 4.15.0-175-generic #184-Ubuntu SMP Thu Mar 24 17:48:36 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitypersonality/tez.sh
git revisionmaster / cf9e3ff
Default JavaPrivate Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Multi-JDK versions/usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
mvninstallhttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-190/10/artifact/out/patch-mvninstall-tez-runtime-library.txt
mvninstallhttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-190/10/artifact/out/patch-mvninstall-tez-plugins_tez-protobuf-history-plugin.txt
Test Resultshttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-190/10/testReport/
Max. process+thread count2089 (vs. ulimit of 5500)
modulesC: tez-api tez-common tez-runtime-internals tez-runtime-library tez-mapreduce tez-dag tez-ext-service-tests tez-plugins/tez-protobuf-history-plugin tez-plugins/tez-yarn-timeline-history tez-plugins/tez-yarn-timeline-history-with-acls tez-plugins/tez-yarn-timeline-history-with-fs tez-plugins/tez-history-parser tez-plugins/tez-aux-services tez-tools/analyzers/job-analyzer . U: .
Console outputhttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-190/10/console
versionsgit=2.25.1 maven=3.6.3 findbugs=3.0.1
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@abstractdog

Copy link
Copy Markdown
Contributor

mvn install failures are not related, patch LGTM, let's merge the change and then we can see if the issue persists

@abstractdog
abstractdog self-requested a review June 6, 2022 19:49
@abstractdog
abstractdog merged commit e5a5578 into apache:masterJun 6, 2022
prabhjyotsingh pushed a commit to acceldata-io/tez that referenced this pull request Nov 12, 2024
…eviewed by Laszlo Bodor)
(cherry picked from commit e5a5578)
prabhjyotsingh added a commit to acceldata-io/tez that referenced this pull request Nov 12, 2024
…eviewed by Laszlo Bodor)
(cherry picked from commit e5a5578)
prabhjyotsingh added a commit to acceldata-io/tez that referenced this pull request Nov 12, 2024
…eviewed by Laszlo Bodor)
(cherry picked from commit e5a5578)
prabhjyotsingh added a commit to acceldata-io/tez that referenced this pull request Nov 12, 2024
…eviewed by Laszlo Bodor)
(cherry picked from commit e5a5578)
prabhjyotsingh added a commit to acceldata-io/tez that referenced this pull request Nov 12, 2024
…eviewed by Laszlo Bodor)
(cherry picked from commit e5a5578)
prabhjyotsingh added a commit to acceldata-io/tez that referenced this pull request Nov 20, 2024
…eviewed by Laszlo Bodor)
(cherry picked from commit e5a5578)
(cherry picked from commit 5a05d43)
prabhjyotsingh added a commit to acceldata-io/tez that referenced this pull request Nov 20, 2024
…ila Toth reviewed by Laszlo Bodor) (#19)
(cherry picked from commit e5a5578)
(cherry picked from commit 5a05d43)
shubhluck pushed a commit to acceldata-io/tez that referenced this pull request Nov 21, 2024
…ila Toth reviewed by Laszlo Bodor) (#19)
(cherry picked from commit e5a5578)
(cherry picked from commit 5a05d43)
Sign up for freeto 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.

3 participants

@LA-Toth@tez-yetus@abstractdog