Automated testing for coder_sniffer

I originally started testing the same way, but moved to simpletests when Drupal adopted that standard. I do prefer simpletests because (a) that's the Drupal unit testing standard, and (b) the other coder reviews use simpletest. Given that we don't already have PHPUnit tests, I'd prefer that we continue to build simpletests; and if we find that the current coder test classes (CoderReviewTestCase, et.al.) aren't sufficient (i.e., we don't actually catch exceptions or know what warning causes a pass or failure), that we first work on making the base test class better.

BTW, when the tests were originally written, I tried to make the tests themselves not throw coder errors, thus you see a lot of string concatenation in the tests intended to prevent coder from catching warnings. Now that we have a more robost ignore system, we can likely put an @ignore directive at the top of the file ... however IIRC correctly, I haven't yet implemented a global ignore because I didn't really want to allow people to globally ignore all warnings, so this is something I'll need to add, and something I'll probably restrict to just test files.

@klausi - No need to virtually stone you... You are to be commended for how far you have brought Drupal to being able to utilize PHP_CodeSniffer to check for Drupal's coding and documentation standards.

I have spent some considerable time over past two weeks adding documentation to the coder_review sub-module and reading through all of the coder_sniffer sub-module. Based upon that experience, I believe that the majority of the enhancements that should (or will be needed) to be added to the Coder module should be coded as sniffs rather than as rules. The PHP_CoderSniffer parser allows one to write to much more complex checks and should be utilized if appropriate.

When one runs the Coder module, I think a reasonable expectation (as a default) is that all coder_review rules and all code_sniffer sniffs will be executed and the collected results will be reported as errors. Does this make sense?

The current coder_review simpletests are a bit confusing. For certain text strings, it is not at all clear why that particular string should pass or fail. We need to add/expand the inline documentation explaining what is wrong in those unclear cases. [Throughout the Coder module (and its sub-modules), we need to add cross-references to where that particular portion of the Drupal standard is documented.]

One of the great things about the code_sniffer module is its good and bad example files. I personally would like to see us write a simpletest Test to check that all of the *.good test files pass. For the present, until D9 is closer to reality, I think we should also write Simpletest Test classes for each of the problems in the *.bad example files. (Perhaps there also should be a Simpletest that checks all of the *.bad text files as part of the Coder commit process?)

Yes, these probably will need to be converted in several years to PHPunit tests. However, so will the rest of all of the core simpletest tests. I think we need spend time on the practical rather than the ideal.

Similarly, I appreciate that PHP_CodeSniffer sniffs use a different coding and documentation standard than Drupal. It also is admirable that Drupal might contribute upstream to PHP_CodeSniffer at some point. However, until we contribute upstream, I would suggest since the PHP-CodeSniffer standards are not well understood, any new tests or modifications can be written to conform to either Drupal or PHP_CodeSniffer standards. At a minimum, we need to document what is expected for a patch that includes a new or enhanced sniff.

Finally, I think that any new or enhanced sniff should include a Simpletest that proves that the detection functionality works as expected. In many cases, I would hope that appropriate *.bad file could be modified to demonstrate what is being checked for.

This may have some bearing on the PHPunit option: #1801176: Deploy a PHPUnit system with a bottom-up approach.

We are in code thaw, and I don't see a patch that rewrites any of our thousands of tests. Coder should convert to PHPUnit testing when the Drupal community does, but AFAICT, we won't do it until at least 9.x. Am I wrong on this?