-
Notifications
You must be signed in to change notification settings - Fork 9.2k
HADOOP-19071. Update maven-surefire-plugin from 3.0.0 to 3.2.5. #6664
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
Conversation
|
💔 -1 overall
This message was automatically generated. |
|
(!) A patch to the testing environment has been detected. |
dev-support/bin/hadoop.sh
Outdated
| "${MAVEN}" "${MAVEN_ARGS[@]}" verify -fae --batch-mode -am \ | ||
| "${modules[@]}" \ | ||
| -DskipShade -Dtest=NoUnitTests -Dmaven.javadoc.skip=true -Dcheckstyle.skip=true \ | ||
| -DskipShade -Dsurefire.failIfNoSpecifiedTests=false -Dtest=NoUnitTests -Dmaven.javadoc.skip=true -Dcheckstyle.skip=true \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I carefully readed the build report, and -Dsurefire.failIfNoSpecifiedTests=false is necessary because we have some modules without JUnit tests. We use -Dtest=NoUnitTests to handle this, which can cause an error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we just make this the default in our pom definitions? I disable tests all the time to run failsafe ITests in the integration phase only.
|
💔 -1 overall
This message was automatically generated. |
|
@ayushtkn @steveloughran I still want to upgrade this plugin to a higher version. I'm not sure why the number of running unit tests is decreasing. |
steveloughran
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I'd like not failing if tests aren't provided to be the default
- some surefire releases don't print stack traces by default. We need that...let's make sure that option is set too
dev-support/bin/hadoop.sh
Outdated
| "${MAVEN}" "${MAVEN_ARGS[@]}" verify -fae --batch-mode -am \ | ||
| "${modules[@]}" \ | ||
| -DskipShade -Dtest=NoUnitTests -Dmaven.javadoc.skip=true -Dcheckstyle.skip=true \ | ||
| -DskipShade -Dsurefire.failIfNoSpecifiedTests=false -Dtest=NoUnitTests -Dmaven.javadoc.skip=true -Dcheckstyle.skip=true \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we just make this the default in our pom definitions? I disable tests all the time to run failsafe ITests in the integration phase only.
@steveloughran Thank you for reviewing pr! I will continue to improve this pr. |
|
(!) A patch to the testing environment has been detected. |
|
💔 -1 overall
This message was automatically generated. |
steveloughran
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's no need to force the CLI options down, that's to allow overrides in manual runs. in POM files set the options directly
|
(!) A patch to the testing environment has been detected. |
|
💔 -1 overall
This message was automatically generated. |
|
@steveloughran @ayushtkn The compilation report looks as expected. May I continue with something else? |
|
get a test run on the build, the last yetus report didn't do that. touch something on the root pom & it should do the trick I believe |
|
I'm happy with the last change, lets see what yetus says. I propose getting it into trunk and waiting about a week before backporting, just so it's easier to roll back if there are surprises |
|
(!) A patch to the testing environment has been detected. |
|
💔 -1 overall
This message was automatically generated. |
steveloughran
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
+1
|
@steveloughran @ayushtkn Thanks for reviewing the code! |
|
@slfan1989 did we had a test run in any of the build results? what changed from the previous commit, I doubt we would land up getting the same issues that were present earlier? the last yetus build doesn't have unit tests mentioned only |
|
The build results aren't good A possible reason is VM crashing due to some test & rest all not getting triggered itself, One potential is: This has a OOM & I think lead to VM crash. Debugging the Jenkins results, it has messages like and The file from which I am quoting these are here: Moreover, the tests didn't kick in for all the modules Can check here as well: there is no yarn, no rbf, no aws, no distcp either, a lot of them are missing. Potentially there could be some more memory requirements or some tests crashing the VMs leading to more issues. So, this requires more investigation and efforts |
|
Sorry for not following up on this PR promptly. I didn't run the full suite of unit tests but relied on the original CI results. To ensure the next upgrade goes smoothly, I'll run the complete unit tests on a server, compare the results, and provide a report. |
…5. (#6664)" (#6875) This reverts commit 88ad7db. Signed-off-by: Shilun Fan <[email protected]>
…5. (apache#6664)" (apache#6875) This reverts commit 88ad7db. Signed-off-by: Shilun Fan <[email protected]>
…5. (apache#6664)" (apache#6875) This reverts commit 88ad7db. Signed-off-by: Shilun Fan <[email protected]>
…he#6664) Contributed by Shilun Fan. Reviewed-by: Steve Loughran <[email protected]> Reviewed-by: Ayush Saxena <[email protected]> Signed-off-by: Shilun Fan <[email protected]>
…he#6664) Contributed by Shilun Fan. Reviewed-by: Steve Loughran <[email protected]> Reviewed-by: Ayush Saxena <[email protected]> Signed-off-by: Shilun Fan <[email protected]>
…he#6664) Contributed by Shilun Fan. Reviewed-by: Steve Loughran <[email protected]> Reviewed-by: Ayush Saxena <[email protected]> Signed-off-by: Shilun Fan <[email protected]>
…he#6664) Contributed by Shilun Fan. Reviewed-by: Steve Loughran <[email protected]> Reviewed-by: Ayush Saxena <[email protected]> Signed-off-by: Shilun Fan <[email protected]>
…he#6664) Contributed by Shilun Fan. Reviewed-by: Steve Loughran <[email protected]> Reviewed-by: Ayush Saxena <[email protected]> Signed-off-by: Shilun Fan <[email protected]>
…he#6664) Contributed by Shilun Fan. Reviewed-by: Steve Loughran <[email protected]> Reviewed-by: Ayush Saxena <[email protected]> Signed-off-by: Shilun Fan <[email protected]>
…he#6664) Contributed by Shilun Fan. Reviewed-by: Steve Loughran <[email protected]> Reviewed-by: Ayush Saxena <[email protected]> Signed-off-by: Shilun Fan <[email protected]>
…he#6664) Contributed by Shilun Fan. Reviewed-by: Steve Loughran <[email protected]> Reviewed-by: Ayush Saxena <[email protected]> Signed-off-by: Shilun Fan <[email protected]>


Description of PR
JIRA: HADOOP-19071. Update maven-surefire-plugin from 3.0.0 to 3.2.5.
How was this patch tested?
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?