-
Notifications
You must be signed in to change notification settings - Fork 543
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
[SUREFIRE-2024] Replace testng-junit5 by testng-engine #500
Conversation
<scope>test</scope> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.junit.support</groupId> |
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.
You are missing the engine exclusion in this dependency.
In the previous revision, see the POM, the Jupiter version was 5.5.0, therefore we excluded it and the engine and api was removed as well. Adding it in the provider would mean that a new version of the engine and api would be consistent with another version.
Overriding the version of API in the profile junit5-api
would mean that the original engine stays. That's not consistent.
So it's better to remove engine as a transitive dependency. The profile's responsibility would be to specify junit artifact.
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.
If engine should be not as a transitive dependency form testng-engin
we simply should report a issue for testng-engin
not introduce workaround here.
You point me to archived project https://github.com/testng-team/testng-junit5
Now maintenanced is:
https://github.com/junit-team/testng-engine
60bde3e
to
1c73dc5
Compare
@slawekjaranowski |
Another radical proposition, drop this it tests at all, So mention in documentation such possibility even without configuration examples and point to product page should be enough. Our product is provider for junit-platform so we should take care about it not for every extension for junit-platform. |
@slawekjaranowski |
Ok, It is only improvement and can be postpone to a later release. |
@slawekjaranowski |
@slawekjaranowski |
1c73dc5
to
9a88797
Compare
@Tibor17 I think that exclusions in most of time it is workaround for problem in target project. As we see on the project page https://github.com/junit-team/testng-engine there are no exclusions in examples. And next https://repo1.maven.org/maven2/org/junit/support/testng-engine/1.0.1/testng-engine-1.0.1.pom |
In the old revision of |
They would appear in our test classpath. |
Now with newer version of So I prefer to not propagate workaround with exclusion to users, whose simply copy and paste it. Is it exclusion is still needed and reasonable? I hope that JUnit team upgrade WDYT? |
@slawekjaranowski |
Ok, thanks. |
Following this checklist to help us incorporate your
contribution quickly and easily:
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
[SUREFIRE-XXX] - Fixes bug in ApproximateQuantiles
,where you replace
SUREFIRE-XXX
with the appropriate JIRA issue. Best practiceis to use the JIRA issue title in the pull request title and in the first line of the
commit message.
mvn clean install
to make sure basic checks pass. A more thorough check willbe performed on your pull request automatically.
mvn -Prun-its clean install
).If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.
I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004
In any other case, please file an Apache Individual Contributor License Agreement.