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

Honour regex in dependsOnMethods #2838

Merged
merged 1 commit into from
Dec 2, 2022

Conversation

krmahadevan
Copy link
Member

Closes: #141

Fixes #141 .

Did you remember to?

  • Add test case(s)
  • Update CHANGES.txt
  • Auto applied styling via ./gradlew autostyleApply

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.

Note: For more information on contribution guidelines please make sure you refer our Contributing section for detailed set of steps.

@krmahadevan krmahadevan requested a review from juherr as a code owner November 23, 2022 08:04
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.

It need more tests to cover the feature.

testng-core/src/main/java/org/testng/DependencyMap.java Outdated Show resolved Hide resolved
@krmahadevan krmahadevan force-pushed the fix_141_regex_methods branch 2 times, most recently from bfb51cb to d7a5303 Compare November 28, 2022 03:23
@krmahadevan
Copy link
Member Author

@juherr - I have added the tests. Please take a look and let me know if there's anything additional that you are looking for.

@krmahadevan krmahadevan force-pushed the fix_141_regex_methods branch from d7a5303 to 00de134 Compare November 29, 2022 08:14
@krmahadevan
Copy link
Member Author

@juherr - I have addressed all the review comments. There's one bug that I found which seems to exist when you use dependsOnMethods and use a fully qualified method name. Since that defect exists in regular dependsOnMethods usage (without regex), when I fix that defect I will do it for regex as well.

Here's the issue link #2840

@juherr
Copy link
Member

juherr commented Dec 1, 2022

What do you thing to add the following case:

public class TestClassSample {

  @Test(dependsOnMethods = "test_C[0-9]{7}")
  public void randomTest() {}

  public static class Inner {
    @Test
    public void test_C6390323() {}
  }
}

randomTest is supposed to fail here because test_C6390323 is out of scope.

@krmahadevan krmahadevan force-pushed the fix_141_regex_methods branch from 00de134 to 0ef294a Compare December 2, 2022 05:56
@krmahadevan
Copy link
Member Author

@juherr - I have added that test as well. Please check.

@krmahadevan krmahadevan merged commit a5b8508 into testng-team:master Dec 2, 2022
@krmahadevan krmahadevan deleted the fix_141_regex_methods branch December 2, 2022 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

regular expression in "dependsOnMethods" not work
2 participants