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

improve rule to handle more than one test class matching impl class #920

Merged
merged 1 commit into from
Aug 9, 2022

Conversation

mslowiak
Copy link
Contributor

@mslowiak mslowiak commented Jul 15, 2022

This will improve the existing rule to test that test classes reside in the same package as their implementation. In particular the rule

  1. Should not fail if there are multiple test classes with the same simple name as long as one test class resides in the correct package
  2. Should fail if none of multiple test classes matching by name with implementation class are in correct package

Resolves #918

Copy link
Collaborator

@codecholeric codecholeric left a comment

Choose a reason for hiding this comment

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

Thanks a lot for taking care of this so quickly 😃 Looks good to me, just a couple of minor nitpicks you could look into.

@codecholeric codecholeric added this to the 1.0.0 milestone Jul 24, 2022
@codecholeric
Copy link
Collaborator

Ah, one more thing, could you also make sure that each commit message header is not longer than 70 chars? So GitHub doesn't break it over? And it would be cool, if you could put the details from the PR description into the commit message as details as well. Because I want to keep a clean Git history that is self-explanatory and does not depend on GitHub as a platform for further context.

@mslowiak mslowiak force-pushed the feature/issue-918 branch from 7ec7d8e to 2ab4386 Compare July 24, 2022 21:09
@mslowiak
Copy link
Contributor Author

Ah, one more thing, could you also make sure that each commit message header is not longer than 70 chars? So GitHub doesn't break it over? And it would be cool, if you could put the details from the PR description into the commit message as details as well. Because I want to keep a clean Git history that is self-explanatory and does not depend on GitHub as a platform for further context.

Thanks for explaining point of view. I adjusted the PR according to comments 🚀

@mslowiak mslowiak requested a review from codecholeric July 24, 2022 21:10
Copy link
Collaborator

@codecholeric codecholeric left a comment

Choose a reason for hiding this comment

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

Looks very good now, thanks a lot!! 😃

This will improve the existing rule to test that test classes reside in the same package as their implementation. In particular the rule

1. Should not fail if there are multiple test classes with the same simple name as long as one test class resides in the correct package
2. Should fail if none of multiple test classes matching by name with implementation class are in correct package

Signed-off-by: Marcin Słowiak <[email protected]>
@codecholeric codecholeric changed the title Handle scenarios when there are more than one matching test class to implementation class name improve rule to handle more than one test class matching impl class Aug 9, 2022
@codecholeric codecholeric merged commit 8baaff4 into TNG:main Aug 9, 2022
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.

testClassesShouldResideInTheSamePackageAsImplementation fails on duplicated simple class name
2 participants