Sadly, tests are often thrown together in response to the 'Needs tests' tag and use examples from within existing Test classes. As a result, when doing a review of our current Test classes focused simply on drupalCreateUser() using grep, there are numerous violations of Drupal's combined standards in http://drupal.org/node/1354 and http://drupal.org/node/325974.
For instance, there are numerous cases of $this->admin_user in core. According to our class coding standards, this should be something equivalent to '$this->adminUser'. Note the correction from admin_user to adminUser as a member of a class. Before this incorrect example gets used further, all current incorrect examples need to be corrected.
This patch attempts to bring Tests in modules N-Z closer to both of these standards. As a result, admittedly, it crosses both existing documentation and coding standards. However, at least to this patch creator, all of the changes seem to make sense as result of reading through a cross-section of our test classes. I am not aware of when before Drupal tests have attempted to ensure consistency with both of there standards.
Modifications to Tests in modules A-M are covered in issue #1803656: Improve Tests consistency to standards in modules A-M.
Comment | File | Size | Author |
---|---|---|---|
#15 | 1804522-15-tests-consistency.patch | 183.39 KB | Lars Toomre |
#13 | 1804522-13-tests-consistency.patch | 183.95 KB | Lars Toomre |
#11 | 1804522-11-tests-consistency.patch | 181.67 KB | Lars Toomre |
#9 | 1804522-9-tests-consistency.patch | 178.54 KB | Lars Toomre |
#7 | 1804522-7-tests-consistency.patch | 178.91 KB | Lars Toomre |
Comments
Comment #1
Lars Toomre CreditAttribution: Lars Toomre commentedHere is an initial patch that has not been tested locally for this issue. Let's see what the test bot thinks.
Comment #3
Lars Toomre CreditAttribution: Lars Toomre commentedWell, that was missing comma... Here is another untested patch.
Comment #5
Lars Toomre CreditAttribution: Lars Toomre commentedThere were some stray characters in EntityFieldTest.php. Let's try that once more.
Comment #7
Lars Toomre CreditAttribution: Lars Toomre commentedUgh... a silly copy and paste error in the last Test file. Hopefully, this is the last of the PHP syntax errors.
Comment #9
Lars Toomre CreditAttribution: Lars Toomre commentedFixed the replacement error in BreadcrumbTest.php. Let's see how this untested patch does now.
Comment #11
Lars Toomre CreditAttribution: Lars Toomre commentedSomething odd was going on with TaxonomyTestBase.php. Let's see if this fixes the problem. This is another untested patch since I do not have a local testing environment.
Comment #13
Lars Toomre CreditAttribution: Lars Toomre commentedI seems one cannot define a protected member of an abstract class like TaxonomyTestBase.php. I thought that one could.
Let's see how this untested patch with the declaration of 'protected $adminUser;' in each of the classes that extends the base class does.
Comment #15
Lars Toomre CreditAttribution: Lars Toomre commentedAh I removed too much from TaxonomyTestBase.php in #13. Let's see what test bot thinks of this untested patch.
Comment #16
Lars Toomre CreditAttribution: Lars Toomre commentedGrr.. remember to set the status.
Comment #25
dawehnerInstead of doing something by module I think we should do something by concept. Otherwise it is basically impossible to review the changes done here.