-
Notifications
You must be signed in to change notification settings - Fork 924
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
Fix pts with testng #5339
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
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. |
build.gradle
Outdated
@@ -127,7 +129,7 @@ allprojects { | |||
} | |||
|
|||
predictiveSelection { | |||
enabled.set(!isCi) | |||
enabled.set(true) |
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.
Q: Is this an intentional change?
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 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 |
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.
TIL. 📝
This reverts commit 07e4e31.
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.
Nice changes! 🙇♂️🙇♂️
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.
👍 👍 👍
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.
Thanks, @jrhee17! 🙇
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
Motivation:
Recently we have enabled PTS which causes an error when trying to run testNg tests.
It has been pointed out that
TestNg
is supported throughjunit-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:
testng
to testing runtimeuseJUnitPlatform()
Result: