Fix: Require test code to also be PSR-2-compliant

[localheinz]

This PR

• extracts a configuration file for phpcs
• requires code in tests/ to also be PSR-2 compliant
• runs phpcbf and extracts each class definition into its own file
• moves extracted test assets (as well as existing test assets) into Fixtures namespace

💁 There really isn't any plausible reason for applying different standards for production and test code, is there?

[SammyK]

This is great - thanks! Since it's a bigger one, I'll let @yguedidi give it a once over as well before merging. :)

[yguedidi]

• OK to extract phpcs config, but in a phpcs.xml.dist file as it's the default loaded one, and can be overided by a git ignored phpcs.xml one
• Not sure about using PSR-2 in tests...
  • First they're removed in dist zip
  • Second it's allow us to have multiple classes in one file for small fixture classes instead of many files
  • Finally, look at PhpSpec, a great API design tool, tests are written like public function it_do_this_thing() which is not PSR-2 compliant, then how to use it?

[localheinz]

@yguedidi

OK to extract phpcs config, but in a phpcs.xml.dist file as it's the default loaded one, and can be overided by a git ignored phpcs.xml one

Addressed.

Second it's allow us to have multiple classes in one file for small fixture classes instead of many files.

Can't really see how that is an advantage. Can you explain?

Finally, look at PhpSpec, a great API design tool, tests are written like public function it_do_this_thing() which is not PSR-2 compliant, then how to use it?

These are the options:

1. don't use phpspec (phpunit works just fine) and require test code to be PSR-2-compliant
2. use phpspec, and don't require test code to be PSR-2-compliant
3. don't require test code to be PSR-2-compliant

In regard to 2.: We're not using it here, so it's not a problem.
In regard to 3.: Certainly an option, but not a good one - consistency wins.

[yguedidi]

can just be vendor/bin/phpcs

[yguedidi]

@localheinz I'm not really against using PSR-2 in our current test files, I just think it's not worth it.
BTW, If you really want to separate fixture classes in their own file, please put them under a Fixtures\ namespace.

[localheinz]

@yguedidi

Updated!

[yguedidi]

Fixtures\ must be the top level namespace, so this must be Facebook\Tests\Fixtures\GraphNodes. Same for other files. It's helpful to put all fixtures together :)

[localheinz]

@yguedidi

Amended!

[yguedidi]

@localheinz You missed this one :)

[localheinz]

@yguedidi

And amended! 😄

[yguedidi]

Looks good :)

[SammyK]

Boom! Thanks guys! :)

[localheinz]

Thank you, @SammyK and @yguedidi!