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

Consider cut as default name #369

Open
lucasborin opened this issue Apr 9, 2021 · 5 comments
Open

Consider cut as default name #369

lucasborin opened this issue Apr 9, 2021 · 5 comments
Assignees
Labels
new check New check

Comments

@lucasborin
Copy link
Member

Based on Name the code under test meaningfully, or default to CUT, a new Check object could scan test classes for CUT.
If it does not find it, it could recommend the developer to use the default name instead.

@lucasborin lucasborin added feature New feature or refactoring new check New check and removed feature New feature or refactoring labels Apr 9, 2021
@lucasborin lucasborin self-assigned this Apr 13, 2021
@lucasborin
Copy link
Member Author

if the class under test is hidden in a when method, what the Check should do?

For instance:

METHOD Test. 
  Given( ... )
  When( ... )
  Then( ... )
ENDMETHOD.
METHOD When.
  cut-> ...
ENDMETHOD.

@lucasborin lucasborin linked a pull request May 12, 2021 that will close this issue
@pokrakam
Copy link
Contributor

Isn't this check contrary to the style guide? It recommends a meaningful name or cut as a default. Not sure if or how this could be tested.
If I understand correctly, the 'good' example in the guide DATA blog_post... would fail this check.

@lucasborin
Copy link
Member Author

The idea here is to provide an automated way to support the team that follows the cut naming convention to identify non-compliant test methods. As the style guide gives the option to follow cut naming, IMO, it does not go against the style guide. If you do not follow the cut, I won't recommend you to turn on this Check in the Code Inspector variant or Code Pal Profile.

Personally, I had bad experiences following the 'meaningful names' as it depends on the developer's perspective. It can be meaningful to me, but it cannot to someone else. Besides, if the class under test has dependencies (objects), it is not that easy to find the real class under test between the other instances. Therefore, I prefer the cut naming option. Although, I am not against meaningful names in some specific cases. It is up to the team to decide what makes sense. That is why the code pal provides notifications, not errors or warnings.

Feel free to share your perspective as well. :)

@pokrakam
Copy link
Contributor

Thanks, that makes sense. The guide clearly suggests cut as a secondary option and your description that this test is based on that came across as a contradiction, especially since it even linked to it.

Personally I am a little uncomfortable with that too and also usually prefer cut as it's a de facto standard. I'll switch to other names where it makes sense e.g. tests with multiple instances.

@lucasborin
Copy link
Member Author

if the class under test is hidden in a when method, what the Check should do?

For instance:

METHOD Test. 
  Given( ... )
  When( ... )
  Then( ... )
ENDMETHOD.
METHOD When.
  cut-> ...
ENDMETHOD.

Then, it could find the cut in the class definition.

lucasborin added a commit that referenced this issue May 14, 2021
@lucasborin lucasborin linked a pull request May 14, 2021 that will close this issue
@lucasborin lucasborin removed a link to a pull request May 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new check New check
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants