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

[SUREFIRE-2119] - Sanitize failIfNoSpecifiedTests prefix in failsafe #570

Merged
merged 1 commit into from Feb 21, 2023

Conversation

liry
Copy link
Contributor

@liry liry commented Oct 11, 2022

Surefire is using surefire.failIfNoSpecifiedTests, but failsafe used it.failIfNoSpecifiedTests.
Error msg is then pointed to nonexistent property: No tests matching pattern "..." were executed! (Set -Dfailsafe.failIfNoSpecifiedTests=false to ignore this error.)

Following this checklist to help us incorporate your
contribution quickly and easily:

  • Make sure there is a JIRA issue filed
    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.
  • Each commit in the pull request should have a meaningful subject line and body.
  • Format the pull request title like [SUREFIRE-XXX] - Fixes bug in ApproximateQuantiles,
    where you replace SUREFIRE-XXX with the appropriate JIRA issue. Best practice
    is to use the JIRA issue title in the pull request title and in the first line of the
    commit message.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Run mvn clean install to make sure basic checks pass. A more thorough check will
    be performed on your pull request automatically.
  • You have run the integration tests successfully (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.

@liry
Copy link
Contributor Author

liry commented Oct 11, 2022

Relates to https://issues.apache.org/jira/browse/SUREFIRE-1910, fixes https://issues.apache.org/jira/browse/SUREFIRE-2119.
Not sure if the parameter deprecation can work this way.

@liry liry force-pushed the SUREFIRE-2119 branch 2 times, most recently from c9ac8f9 to 24b53db Compare October 11, 2022 14:03
Comment on lines 144 to 146
* Set this to "true" to cause a failure if none of the tests specified in -Dit.test=... are run. Defaults to
* "true".
* Replacing "it.failIfNoSpecifiedTests" to be consistent with surefire plugin.
Copy link
Member

Choose a reason for hiding this comment

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

as true is the default value, I would start documentation from something like

Set this to false to prevent a failure if none ....

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with your suggestion, but @liry did not change the wording here.

Since there's no response from @liry, can the PR be merged anyway? In my fork I've rebased the branch to master. Builds were successful: https://github.com/andpab/maven-surefire/actions/runs/4198008135

After the merge of this one I could make a new PR that adjusts the wording according to your suggestion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I somehow missed the first comment. Wording changed.

Surefire is using `surefire.failIfNoSpecifiedTests`,
but failsafe used `it.failIfNoSpecifiedTests`.
Error msg is then pointed to nonexistent property:
`No tests matching pattern "..." were executed! (Set
-Dfailsafe.failIfNoSpecifiedTests=false to ignore this error.)`
Comment on lines +919 to +920
getConsoleLogger().warning( "Use " + getPluginName()
+ ".failIfNoSpecifiedTests property instead of obsolete it.failIfNoSpecifiedTests." );
Copy link
Member

Choose a reason for hiding this comment

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

Maven 3.9 will print such info itself - but can be now.

@slawekjaranowski slawekjaranowski merged commit 208eae2 into apache:master Feb 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants