Skip to content

Directories in "dev/tests/" should be excluded from some static rules #80

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

Closed
danielrenaud opened this issue Apr 4, 2019 · 3 comments · Fixed by #81
Closed

Directories in "dev/tests/" should be excluded from some static rules #80

danielrenaud opened this issue Apr 4, 2019 · 3 comments · Fixed by #81
Labels
bug Something isn't working

Comments

@danielrenaud
Copy link

danielrenaud commented Apr 4, 2019

Preconditions

magento-coding-standard v1.0.1

Steps to reproduce

  1. Create some code somewhere in dev/tests/* that violates a rule like "Magento2.Exceptions.DirectThrow"

Results

The dev/tests/* directories are not excluded, so all rules are applied, even if they are not necessary for test code.
Exactly which rules should be excluded may be up for discussion, but I would assume the same exclusions for */Tests/* would apply.

@danielrenaud danielrenaud added the bug Something isn't working label Apr 4, 2019
@lenaorobei
Copy link
Contributor

lenaorobei commented Apr 4, 2019

Since this Coding Standard is used not by Magento core only, I would not add such exclude because it is tight to Magento2 codebase and does not make sense for extension developers. I suggest be more generic here. Will this work for your case?

<exclude-pattern>*/_files/*</exclude-pattern>
<exclude-pattern>*/Test/*</exclude-pattern>
<exclude-pattern>*Test.php</exclude-pattern>

@paliarush need your input here.

@danielrenaud
Copy link
Author

This would then not apply to test framework code. However, it's not clear if test framework code should be held to all static tests or not.

@danielrenaud
Copy link
Author

danielrenaud commented Apr 5, 2019

@lenaorobei Probably adding the exclusion you suggested would handle the majority of problems. Regarding test framework code, even if we did want to exclude some rules for that, it would most likely be different for the actual tests. And in that case maybe we could just be more accommodating of phpcs:ignore in these areas.
So I think adding the exclusion you mentioned above in places that currently have only */Test/* will work.
Thanks!

lenaorobei added a commit that referenced this issue Apr 5, 2019
magento-devops-reposync-svc pushed a commit that referenced this issue Sep 28, 2021
…-coding-standard-292

[Imported] Version 12 master update
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants