-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[improve][test] Bump TestNG to 7.6.1 #18521
Conversation
/pulsarbot rerun-failure-checks |
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.
Comments inline.
<dependency> | ||
<groupId>org.hamcrest</groupId> | ||
<artifactId>hamcrest</artifactId> | ||
<scope>test</scope> | ||
</dependency> |
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.
Interesting. Why do we need this dependency explicitly now?
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 don't know why ~, maybe the old testNG includes this library?
pom.xml
Outdated
@@ -241,7 +241,7 @@ flexible messaging model and an intuitive client API.</description> | |||
<!-- Set docker-java.version to the version of docker-java used in Testcontainers --> | |||
<docker-java.version>3.2.13</docker-java.version> | |||
<kerby.version>1.1.1</kerby.version> | |||
<testng.version>7.3.0</testng.version> | |||
<testng.version>7.5</testng.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 learn the latest version is 7.6.1. Why do we use 7.5 here? Also, 7.5 is not of a full semantic version format.
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.
7.6.1 requires JDK 11 or higher. The Pulsar-CI have a JDK 8 test.
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.
+1 as long as no regression.
Try to bump TestNG to 7.6.1. |
Signed-off-by: Zixuan Liu <[email protected]>
eb813f3
to
86c60bb
Compare
Codecov Report
@@ Coverage Diff @@
## master #18521 +/- ##
============================================
- Coverage 45.62% 45.28% -0.34%
- Complexity 10075 10693 +618
============================================
Files 697 757 +60
Lines 68024 72781 +4757
Branches 7293 7817 +524
============================================
+ Hits 31033 32961 +1928
- Misses 33413 36154 +2741
- Partials 3578 3666 +88
Flags with carried forward coverage won't be shown. Click here to find out more.
|
All tests passed. Merging... |
Signed-off-by: Zixuan Liu <[email protected]>
Motivation
Bump TestNG to 7.6.1
Modifications
Documentation
doc
doc-required
doc-not-needed
doc-complete
Matching PR in forked repository
PR in forked repository: nodece#14