HADOOP-19243. Upgrade Mockito version to 4.11.0 #6968
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Hi @muskan1012, thanks for working on this area, as CI workflow is not ready for Java 17 yet, please describe how you tested it locally in detail, e.g.
such information would be much helpful for reviewers to verify the change. |
|
💔 -1 overall
This message was automatically generated. |
|
yeah, I expected mockito upgrades to break things. |
Yes @steveloughran, expected same. Did some changes and pushed those. Will check Yetus output and on the basis of that will do the necessary changes. |
|
💔 -1 overall
This message was automatically generated. |
...ct/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestCacheDirectives.java
Show resolved
Hide resolved
steveloughran
requested changes
Sep 5, 2024
| @@ -173,8 +173,8 @@ | |||
| </dependency> | |||
| <dependency> | |||
| <groupId>org.powermock</groupId> | |||
| <artifactId>powermock-api-mockito</artifactId> | |||
| <version>1.7.4</version> | |||
| <artifactId>powermock-api-mockito2</artifactId> | |||
There was a problem hiding this comment.
move the declaration of this jar into the hadoop-project pom with this module importing it, same for the one below. There's an existing powermock.version they can use once all projects are in sync
|
|
||
| import static org.junit.Assert.assertFalse; | ||
| import static org.junit.Assert.assertTrue; | ||
| import static org.mockito.ArgumentMatchers.*; |
There was a problem hiding this comment.
use single imports over .*
| @@ -97,6 +101,16 @@ public void testUpdateQuotaAndCollectBlocks() { | |||
| Assert.assertEquals(-BLOCK_SIZE, counts.getTypeSpaces().get(SSD)); | |||
| } | |||
|
|
|||
| private INodeFile createMockFile(short replication) { | |||
There was a problem hiding this comment.
add a short javadoc saying why this is needed
| @@ -28,9 +28,9 @@ | |||
| import org.junit.Before; | |||
| import org.junit.Test; | |||
| import org.junit.runner.RunWith; | |||
| import org.mockito.ArgumentMatchers; | |||
| import static org.mockito.Mockito.mockStatic; | |||
There was a problem hiding this comment.
move into the static import block
| .thenReturn(asyncDocumentClient); | ||
| Configuration conf = Mockito.mock(Configuration.class); | ||
| mockStatic(DocumentStoreUtils.class); | ||
| when(DocumentStoreUtils.getCosmosDBDatabaseName(conf)).thenReturn("FooBar"); |
There was a problem hiding this comment.
nit: can you keep the thenReturn() clauses on separate lines}
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
muskan1012
force-pushed
the
HADOOP-19243
branch
from
8026713 to
94bc7c2
Compare
Sure @steveloughran, tried to check all import related issues and fixed those, hope it's in good state now. |
steveloughran
requested changes
Oct 18, 2024
Sure @steveloughran, addressed checkstyle issues. Thank you for the review! |
|
💔 -1 overall
This message was automatically generated. |
|
I don't know what happened there but it was fairly traumatic. Can you do a merge commit with trunk to see if things go away on a Yetus build after merge. +1 pending Yetus being happy |
|
I see a couple of PR's like 7126 and 7127 failing the compile job with same errors. apparently the local compile using jdk8 passes after reverting pom.xml changes made in HADOOP-19298. |
|
Looks like the compile failure is tracked in HADOOP-19318 |
|
💔 -1 overall
This message was automatically generated. |
|
The two tests which fail both use Mockito. TestCapacityOverTimePolicy and TestDFSAdmin I think we should assume that these are related to the upgrade, unless it can be shown otherwise. Please can you investigate. Thank you! |
@steveloughran, These tests classes passed locally, indicating the issue isn't related to my changes. |
|
Okay, +1 to the merge but if things play up you'll need a followup PR. Let's give it a few days before a backport to branch-3.4, but you can put that backport PR up right now |
muskan1012
added a commit
to muskan1012/hadoop
that referenced
this pull request
Nov 11, 2024
Mockito is now at a JDK-17 compatible version. Contributed by Muskan Mishra
muskan1012
added a commit
to muskan1012/hadoop
that referenced
this pull request
Nov 15, 2024
Mockito is now at a JDK-17 compatible version. Contributed by Muskan Mishra
muskan1012
added a commit
to muskan1012/hadoop
that referenced
this pull request
Nov 18, 2024
Mockito is now at a JDK-17 compatible version. Contributed by Muskan Mishra
|
Hi @muskan1012, @steveloughran, This PR is causing the build to fail on Windows - Could you please fix this? |
GauthamBanasandra
added a commit
to GauthamBanasandra/hadoop
that referenced
this pull request
Feb 27, 2025
* There was a change to upgrade Mockito to 4.11.0 in apache#6968. * This broke the build on Windows in the hadoop-yarn-ui module - apache#6968 (comment). * Thus, we need to revert back to the previous stable version (2.18.0) of mockito for hadoop-yarn-ui.
This was referenced Feb 27, 2025
GauthamBanasandra
added a commit
that referenced
this pull request
Mar 4, 2025
* There was a change to upgrade Mockito to 4.11.0 in HADOOP-19243. Github PR #6968. * This broke the build on Windows in the hadoop-yarn-ui module. * This PR upgrades the version wro4j to 1.8.0 to get rid of its dependency on mockito, in order to fix the build failure of hadoop-yarn-ui module.
|
This was fixed in #7457. |
YanivKunda
pushed a commit
to YanivKunda/hadoop
that referenced
this pull request
Mar 23, 2025
* There was a change to upgrade Mockito to 4.11.0 in HADOOP-19243. Github PR apache#6968. * This broke the build on Windows in the hadoop-yarn-ui module. * This PR upgrades the version wro4j to 1.8.0 to get rid of its dependency on mockito, in order to fix the build failure of hadoop-yarn-ui module.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of PR
Upgraded the mockito-core and mockito-inline version to make it compatible with JDK17 and to resolve errors related to Mockito: "mockito cannot mock this class."
How was this patch tested?
Tested by running locally.