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-24711][K8S] Fix tags for integration tests #21697
Conversation
@ssuchter @foxish @felixcheung pls review. |
Kubernetes integration test starting |
Kubernetes integration test status success |
Kubernetes integration test starting |
Kubernetes integration test status success |
<plugin> | ||
<groupId>org.apache.maven.plugins</groupId> | ||
<artifactId>maven-surefire-plugin</artifactId> | ||
<version>2.22.0</version> |
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 think we declare the version in the parent POM? (or should). OK to update it while you're at it to the latest version.
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.
Ok sure will do.
Test build #92525 has finished for PR 21697 at commit
|
This is good! Same comment that @srowen had on moving the version up to the parent POM and referencing it here. |
Thanks @foxish will update the PR. |
Kubernetes integration test starting |
Kubernetes integration test status success |
Kubernetes integration test starting |
Kubernetes integration test status success |
pom.xml
Outdated
@@ -114,6 +114,7 @@ | |||
<maven.compiler.source>${java.version}</maven.compiler.source> | |||
<maven.compiler.target>${java.version}</maven.compiler.target> | |||
<maven.version>3.3.9</maven.version> | |||
<maven.surefire.plugin.version>2.22.0</maven.surefire.plugin.version> |
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.
The parent POM here already declares the surefire plugin, but version 2.20.1. That would need to be changed. If you do, you don't need a version in the child POM, nor do you really need a property to control the version and the extra indirection.
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.
Yes but I thought you wanted me to add that version at the parent pom as a property. Ok, I will update the version only in the parent and remove the version from the child pom.
Test build #92622 has finished for PR 21697 at commit
|
Test build #92624 has finished for PR 21697 at commit
|
@srowen fixed it, ready for final review/merge. |
Kubernetes integration test starting |
Kubernetes integration test status success |
Merged to master |
Test build #92646 has finished for PR 21697 at commit
|
What changes were proposed in this pull request?
How was this patch tested?
Manually by adding tags.