Skip to content
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

[TEST] Run JUnit tests on beeline module #4969

Closed
wants to merge 1 commit into from

Conversation

bowenliang123
Copy link
Contributor

@bowenliang123 bowenliang123 commented Jun 15, 2023

Why are the changes needed?

  • add maven-surefire-plugin to beeline module for running JUnit tests.

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible

  • Add screenshots for manual tests if appropriate

  • Run test locally before make a pull request

@pan3793
Copy link
Member

pan3793 commented Jun 15, 2023

Does it support JUnit5 ?

@bowenliang123
Copy link
Contributor Author

bowenliang123 commented Jun 15, 2023

No, the scalatest junit plugin only supports Junit 4.x according to the version list at https://www.scalatest.org/plus/junit/versions.

@bowenliang123 bowenliang123 changed the title [WIP] ScalaTest + JUnit [WIP] Run JUnit on beeline Jun 15, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jun 15, 2023

Codecov Report

Merging #4969 (995c83c) into master (bfc6604) will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master   #4969   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files         561     561           
  Lines       30969   30969           
  Branches     4061    4061           
======================================
  Misses      30969   30969           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@turboFei
Copy link
Member

turboFei commented Jun 16, 2023

image seems works now

https://github.com/apache/kyuubi/actions/runs/5279684895/jobs/9550718740?pr=4969#step:8:1937

@bowenliang123 bowenliang123 changed the title [WIP] Run JUnit on beeline Run JUnit tests on beeline module Jun 16, 2023
@bowenliang123 bowenliang123 changed the title Run JUnit tests on beeline module [TEST] Run JUnit tests on beeline module Jun 16, 2023
@bowenliang123
Copy link
Contributor Author

The tests can be skipped with -DskipTests. cc @pan3793

mvn clean install -pl :kyuubi-hive-beeline -DskipTests:
image

@bowenliang123 bowenliang123 marked this pull request as ready for review June 16, 2023 12:06
@turboFei
Copy link
Member

Do we need to check other java modules?

@cfmcgrady
Copy link
Contributor

Do we need to check other java modules?

I have checked. we missed the module kyuubi-hive-beeline only.

@bowenliang123 bowenliang123 self-assigned this Jun 16, 2023
@bowenliang123 bowenliang123 added this to the v1.8.0 milestone Jun 16, 2023
bowenliang123 added a commit that referenced this pull request Jun 16, 2023
### _Why are the changes needed?_

- add `maven-surefire-plugin` to beeline module for running JUnit tests.

### _How was this patch tested?_
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible

- [ ] Add screenshots for manual tests if appropriate

- [ ] [Run test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make a pull request

Closes #4969 from bowenliang123/scalatest-junit.

Closes #4969

995c83c [liangbowen] surefire on beeline

Authored-by: liangbowen <liangbowen@gf.com.cn>
Signed-off-by: liangbowen <liangbowen@gf.com.cn>
(cherry picked from commit de0258b)
Signed-off-by: liangbowen <liangbowen@gf.com.cn>
@bowenliang123 bowenliang123 modified the milestones: v1.8.0, v1.7.2 Jun 16, 2023
@bowenliang123
Copy link
Contributor Author

Thanks for reviewing. Merged to master / 1.7.2.

@bowenliang123 bowenliang123 deleted the scalatest-junit branch June 16, 2023 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants