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

Use phpcs for tests dir #20

Merged
merged 5 commits into from
May 17, 2019
Merged

Conversation

scaytrase
Copy link
Contributor

Applies phpcs to test files on a regular basis

@scaytrase
Copy link
Contributor Author

@lezhnev74 what about this one?

@lezhnev74
Copy link
Owner

This PR forces tests to conform to Doctrine coding standards.
Since I did not develop tests with that checks in mind, I might need to refactor all of them to conform to the Doctrine rules.

In general, tests should be of as high quality as possible, as well as the code itself.

@scaytrase
Copy link
Contributor Author

i've applied phpcbf here, is it not enough?

@lezhnev74
Copy link
Owner

lezhnev74 commented May 17, 2019

Try to run this command:
./vendor/bin/phpcs

This is what I see:
image

phpcbf can fix only so much problems. Some code style issues are solvable by hands.

Updated
I am looking at the phpcs report. There are mostly just two error types:

  1. missing visibility identifier
  2. names are not camel caps. test_it_validates_maxItems_green => testItValidatesMaxItemsGreen

@scaytrase
Copy link
Contributor Author

This is what I see:

Ah, I see. In our team we use php-cs-fixer which fix all the things it considered violated, but I'm afraid there is no csc set for doctrine ruleset

I am looking at the phpcs report. There are mostly just two error types:

I can update all of them by hand in this PR

@lezhnev74
Copy link
Owner

That would a hell of the PR. If you can do this, please go ahead 🥇

Pavel.Batanov added 2 commits May 17, 2019 10:54
* add phpcs to travis ci job to track progress
@scaytrase
Copy link
Contributor Author

I've added cscheck as separate travis step, so we can track the progress now

https://travis-ci.org/lezhnev74/openapi-psr7-validator/jobs/533693779

@scaytrase
Copy link
Contributor Author

scaytrase commented May 17, 2019

I'm utilized https://plugins.jetbrains.com/plugin/7160-camelcase for the rest ) 5 errors left to fix

@scaytrase
Copy link
Contributor Author

Aaand all green!

@scaytrase
Copy link
Contributor Author

Dont know why, but locally it show more progress-bar points

PS C:\Work\Projects\lezhnev74\openapi-psr7-validator> vendor/bin/phpcs --no-colors
............................................................  60 / 125 (48%)
............................................................ 120 / 125 (96%)
.....                                                        125 / 125 (100%)

Travis show the same count as your screenshot above. But I hope this does not influence anything

@lezhnev74
Copy link
Owner

Great work!!!

Dont know why, but locally it show more progress-bar points

That is interesting.

@lezhnev74 lezhnev74 merged commit fb470cb into lezhnev74:master May 17, 2019
@scaytrase scaytrase deleted the feature/phpcs-tests branch May 17, 2019 09:27
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.

None yet

2 participants