Coding Standards [#234405]

Coding standards need to be figured out and corrected as noted by webchick:

Coding standards compliance for both SimpleTest module and all tests.

Figure out and document test naming/coding conventions + get them in Coder module so they're flagged. March 28, 2008 (boombatower, Doug Green?)

Proposal
I'm not sure what others are thinking, which is why I'm starting this issue, but I think we should make SimpleTest compliant with drupal standards. Meaning change the camelCase standard being used to the drupal standard.

Since a large portion of the public methods are wrapped by DrupalTestCase the methods can just be renamed there. Any non-wrapped methods can be wrapped in order to renamed them.

The class names for each of the tests as well as DrupalTestCase itself need to be renamed as well.

After the above has been completed the coder module will be useful for checking tests as well. That is a nice benefit of going all out to follow drupal standards.

Conclusion
Once this has been discussed, modified, and agreed upon I will be happy to write some patches to fix the SimpleTest framework as well as the individual tests.

Comments

Comment #1

boombatower CreditAttribution: boombatower commented 15 March 2008 at 21:24

After talking with webchick in IRC we came to the following conclusion.

Classes
Classes should follow the PEAR standards.
For example:

Drupal_Test_Case

So all classes outside of the SimpleTest library would need to be renamed.

Methods
All methods should use the Drupal standards.

Functional
Method names should reflect the functionality they are testing.
For example:

test_box()

would test the custom blocks called boxes.

In general methods should only cover one "task." Meaning they should only test the functionality related to one task a user might want to complete.

So testing both editing of module-define blocks and custom boxes in the same function would be testing two tasks instead of one.

Unit
Method names should match the function they are testing.
For example:

db_query()

the test method name would be:

test_db_query()

For the underscored function names the same standard should be applied.

_db_query

would tested with the method named:

test__db_query <- double underscore

The pattern then should always follow test_ + function name

Core Methods
The drupal_test_case.php class will need to wrap all SimpleTest methods and use the Drupal coding standards.

References

Drupal Coding Standards - http://drupal.org/coding-standards

Comment #2

chx CreditAttribution: chx commented 16 March 2008 at 00:27

Dunno why we should care about PEAR. If you look at PHP5 OOP stuff, esp. SPL that's consistently named as DrupalTestCase is. Method names, someMethodName. Function names, though should be lowercase with an underscore.

Comment #3

boombatower CreditAttribution: boombatower commented 17 March 2008 at 22:05

Updated proposal.

Classes
Classes should follow the PHP SLP standards.
For example:

DrupalTestCase

Functional
Functional test classes should be named: [MODULE_NAME] + TestCase
Example:

Block Module Test:
BlockTestCase

Unit
Unit test classes should be named: [FILE_NAME] + TestCase
Example:

block.module
BlockModuleTestCase

block.admin.inc
BlockAdminIncTestCase

Classes outside of the SimpleTest library will need to be renamed. Should only be tests, where necessary.

Methods
All methods should use the Drupal standards.

Functional
Method names should reflect the functionality they are testing.
For example:

test_custom_blocks()

would test the custom blocks called boxes.

In general methods should only cover one "task." Meaning they should only test the functionality related to one task a user might want to complete.

So testing both editing of module-define blocks and custom boxes in the same function would be testing two tasks instead of one.

Unit
Method names should match the function they are testing.
For example:

db_query()

the test method name would be:

test_db_query()

For the underscored function names the same standard should be applied.

_db_query

would tested with the method named:

test__db_query <- double underscore

The pattern then should always follow test_ + function name

Core Methods
The drupal_test_case.php class will need to wrap all SimpleTest methods and use the Drupal coding standards.
Example:

assertTrue

becomes

assert_true

References

Drupal Coding Standards - http://drupal.org/coding-standards
PHP SLP standards - http://www.php.net/~helly/php/ext/spl

Comment #4

webchick

she/they

English

Vancouver 🇨🇦

CreditAttribution: webchick commented 23 March 2008 at 02:46

boombatower and I had a discussion on IRC about this tonight. While I was originally gung-ho about formatting tests according to Drupal naming conventions (assert_false, etc.) now that I've taken a look at the syntax of a dozen or so testing frameworks, I'm less enthused about this.

If we keep stuff as $this->assertFoo() then people can transfer their existing unit testing knowledge to Drupal, and Drupal developers can transfer their Drupal unit testing knowledge to other projects.

We also kicked around a couple ideas for naming conventions within the unit test files themselves, and came up with http://pastebin.ca/953464 as the syntax we ought to use for *unit* tests while keeping testThingyYouAreTesting as the naming convention for *functional* tests, to make it easy to see which is which.

Thoughts?

Comment #5

boombatower CreditAttribution: boombatower commented 24 March 2008 at 02:38

Committed functional tests, making all function names are camelCase.

Committed functional tests, making their class names consistent.

Comment #6

boombatower CreditAttribution: boombatower commented 15 May 2008 at 00:50

Status:

Active

» Fixed

This has been completed

Comment #7

Anonymous (not verified) CreditAttribution: Anonymous commented 29 May 2008 at 00:52

Status:

Fixed

» Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.

Comment #8

Skiller-1 CreditAttribution: Skiller-1 commented 5 June 2008 at 14:48

Sorry that I stumbled across this issue rather late due to an actual project, but I'd like to throw my two cents in as well.

What I like about the PEAR coding conventions for class names is that it allows me to autoload a class without worrying about whether or not I've written the proper call to 'require_once'.

If I encounter an unknown class name all I have to do is to replace the underscore with the DIRECTORY_SEPARATOR, run the path through require_once() --and voilà, the class will be loaded, given that the path construct is based on a directory which actually is in the PHP include_path.

Additionally, this provides me with a handy way to avoid name collisions, as it allows me to encapsulate my whole class library in a directory structure like com/example/Framework/Thingamabob/SomeClass, which will lead to a class name of com_example_Framework_Thingamabob_SomeClass. Talk about namespaces!

This inherent folder/class structure is the one reason why I've always felt uncomfortable with Drupal's underscored naming convention. I prefer camelCase anytime, but I understand that it is much easier to parse for hook name suggestions when you only have to look for underscores.

We propably have to stick with the underscore convention--I don't see how there could possibly be any kind of smooth transition towards camelCase convention, especially concerning Drupal's Hook system.

Simpletest itself, by the way, seems a little outdated to me (in terms of cutting edge, state of the art PHP 5 programming). I wonder how that will ever blend with Drupal, if it weren't overhauled completely. But that would be a different discussion.

Best regards
skiller