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

After methods should be skipped when exclusion used #1575

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

krmahadevan
Copy link
Member

Closes #1574

Suppose there are one or more configuration methods
which don’t belong to any group and for which “alwaysRun”
attribute is not set, and when the user specifies
a valid exclusion list for groups, TestNG would still
execute those configuration methods.

Fixed this anomaly.

Fixes #1574 .

Did you remember to?

  • Add test case(s)
  • Update CHANGES.txt

We encourage pull requests that:

  • Add new features to TestNG (or)
  • Fix bugs in TestNG

If your pull request involves fixing SonarQube issues then we would suggest that you please discuss this with the
TestNG-dev before you spend time working on it.

@@ -275,6 +275,10 @@ private static boolean isIncluded(Collection<String> includedGroups, boolean noG
}

private static boolean isExcluded(Collection<String> excludedGroups, String... groups) {
boolean noGroups = (groups == null || groups.length == 0);
Copy link
Member

Choose a reason for hiding this comment

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

groups cannot be null.

test.addExcludedGroup("sometest");
TestNG testng = create(suite);
testng.run();
assertThat(Issue1574TestclassSample.messages).containsExactly("anothertest","@AfterSuite");
Copy link
Member

Choose a reason for hiding this comment

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

Typo: space missing after ','

import java.util.List;

public class Issue1574TestclassSample {
public static List<String> messages = Lists.newArrayList();
Copy link
Member

Choose a reason for hiding this comment

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

Prefer to use InvokedMethodNameListener when possible.

@krmahadevan
Copy link
Member Author

@juherr - I have amended the commit and fixed all the comments. Please help take a look

@krmahadevan
Copy link
Member Author

ping @juherr - Gentle reminder..

Copy link
Member

@juherr juherr left a comment

Choose a reason for hiding this comment

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

The code is ok but I think it doesn't solve any issue.

@cbeust Could you check #1574 (comment)

@@ -275,6 +275,9 @@ private static boolean isIncluded(Collection<String> includedGroups, boolean noG
}

private static boolean isExcluded(Collection<String> excludedGroups, String... groups) {
if (!excludedGroups.isEmpty() && groups.length ==0) {
Copy link
Member

Choose a reason for hiding this comment

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

Typo: space missing after '=='

@krmahadevan
Copy link
Member Author

ping @cbeust - Can you please help comment on this here : #1574 (comment)

I would like to either close this PR if its not a bug or get this merged if its a bug. Please advise.

Closes testng-team#1574

Suppose there are one or more configuration methods
which don’t belong to any group and for which “alwaysRun”
attribute is not set, and when the user specifies 
a valid exclusion list for groups, TestNG would still
execute those configuration methods.

Fixed this anomaly.
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.

Before/After methods are not running when groups "include" in suite
3 participants