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

Fix pts with testng #5339

Merged
merged 4 commits into from
Jan 2, 2024
Merged

Fix pts with testng #5339

merged 4 commits into from
Jan 2, 2024

Conversation

jrhee17
Copy link
Contributor

@jrhee17 jrhee17 commented Dec 8, 2023

Motivation:

Recently we have enabled PTS which causes an error when trying to run testNg tests.

Execution failed for task ':core:testNg'.
> Predictive Test Selection only supports JUnit Platform.

It has been pointed out that TestNg is supported through junit-platform.
ref: https://docs.gradle.com/enterprise/predictive-test-selection/#_frameworks_and_languages

I see no reason we shouldn't just use junit-platform overall as our default test framework launcher.

ref: https://gradle.slack.com/archives/C05975D6V7H/p1701969512592219

Note that we are unable to add the testNg globally by default due to the following issue: junit-team/testng-engine#87

Modifications:

  • Added a test engine testng to testing runtime
  • Modified to always use useJUnitPlatform()

Result:

  • We can run testNg tests with PTS enabled

Copy link

codecov bot commented Dec 8, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (02cbc34) 73.95% compared to head (8688fd2) 73.66%.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #5339      +/-   ##
============================================
- Coverage     73.95%   73.66%   -0.29%     
+ Complexity    20150    20102      -48     
============================================
  Files          1741     1741              
  Lines         74353    74353              
  Branches       9481     9481              
============================================
- Hits          54988    54775     -213     
- Misses        14876    15041     +165     
- Partials       4489     4537      +48     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jrhee17 jrhee17 added the cleanup label Dec 8, 2023
@jrhee17 jrhee17 marked this pull request as ready for review December 8, 2023 17:57
build.gradle Outdated
@@ -127,7 +129,7 @@ allprojects {
}

predictiveSelection {
enabled.set(!isCi)
enabled.set(true)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: Is this an intentional change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commit was added to verify that the build passes with pts enabled. I've reverted this commit 🙇

@@ -225,6 +224,9 @@ if (rootProject.findProperty('flakyTests') != 'true') {
}
}
}
dependencies {
runtimeOnly libs.junit.testng.engine
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL. 📝

Copy link
Contributor

@ikhoon ikhoon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice changes! 🙇‍♂️🙇‍♂️

Copy link
Contributor

@minwoox minwoox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 👍 👍

Copy link
Member

@trustin trustin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @jrhee17! 🙇

@ikhoon ikhoon merged commit ca31809 into line:main Jan 2, 2024
14 of 15 checks passed
eottabom pushed a commit to eottabom/armeria that referenced this pull request Jan 18, 2024
Motivation:

Recently we have enabled PTS which causes an error when trying to run testNg tests.
```
Execution failed for task ':core:testNg'.
> Predictive Test Selection only supports JUnit Platform.
```

It has been pointed out that `TestNg` is supported through `junit-platform`.
ref: https://docs.gradle.com/enterprise/predictive-test-selection/#_frameworks_and_languages

I see no reason we shouldn't just use `junit-platform` overall as our default test framework launcher.

ref: https://gradle.slack.com/archives/C05975D6V7H/p1701969512592219

Note that we are unable to add the testNg globally by default due to the following issue: junit-team/testng-engine#87

Modifications:

- Added a test engine `testng` to testing runtime
- Modified to always use `useJUnitPlatform()`

Result:

- We can run testNg tests with PTS enabled
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants