-
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
ARROW-17704: [Java][FlightRPC] Update to Junit 5 #14103
Conversation
Thank you! ARROW-4740 is already resolved, would you mind creating a new Jira? |
@lidavidm Created a new Jira. I plan to convert other modules as well, will split into several PRs |
Great, thank you. I'll try to review this soon. (Note that we generally use a single jira per PR, you can create jira sub-tasks for each PR if you like or just separate jiras) |
|
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.
Thank you! Just two comments
java/pom.xml
Outdated
@@ -398,7 +398,14 @@ | |||
</plugin> | |||
<plugin> | |||
<artifactId>maven-surefire-plugin</artifactId> | |||
<version>3.0.0-M7</version> | |||
<version>2.22.2</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.
Is there any way to avoid downgrading surefire?
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.
@lidavidm
I believe it's the same problem as reported here https://issues.apache.org/jira/browse/SUREFIRE-2033
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.
Hmm. Is it possible to downgrade to only, say, 3.0.0-M4, and add a comment linking to the regression? Or otherwise, is it possible to configure the runners to avoid this?
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.
@lidavidm Done. Fixed by adding extra decencies to surefire, the version remains '3.0.0-M7'
java/flight/flight-sql/src/test/java/org/apache/arrow/flight/TestFlightSql.java
Show resolved
Hide resolved
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.
Slick, thank you for figuring it out.
@davisusanibar or @lwhite1 any comments here before I merge?
Benchmark runs are scheduled for baseline = 5e6da78 and contender = b48d228. b48d228 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
['Python', 'R'] benchmarks have high level of regressions. |
Update flight module to Junit 5 ~~Relevant issue: https://issues.apache.org/jira/browse/ARROW-4740~~ See: https://issues.apache.org/jira/browse/ARROW-17704 Authored-by: andreoss <[email protected]> Signed-off-by: David Li <[email protected]>
Update flight module to Junit 5 ~~Relevant issue: https://issues.apache.org/jira/browse/ARROW-4740~~ See: https://issues.apache.org/jira/browse/ARROW-17704 Authored-by: andreoss <[email protected]> Signed-off-by: David Li <[email protected]>
Update flight module to Junit 5
Relevant issue: https://issues.apache.org/jira/browse/ARROW-4740See: https://issues.apache.org/jira/browse/ARROW-17704