If we want to use this as a required tool for the D7 release cycle (which I think we ought to), then we need the code in both the module and the tests themselves clean enough that you could eat off of them. Our goal should be nothing less than core-worthy code (even though obviously we don't want to make this module part of core ;)).

This is going to be my #1 priority for the next while: coding standards compliance, bountiful comments, consistency, and so on. I want to make it as simple as possible for a new developer who's never heard of SimpleTest to dig in and start being productive.

I apologize in advance for the forthcoming pedantic patches that will be coming, which include:

  • It's "SimpleTest," not "Simpletest" or "simpletest".
  • We need clearer, more detailed, and corrected/updated documentation pretty much everywhere.
  • Coding standards compliance for both SimpleTest module and all tests.
CommentFileSizeAuthor
#4 simpletest_code_style.patch216.94 KBcwgordon7

Comments

Rok Žlender’s picture

Rok Žlender commented

+10 :) you will have to watch me, my eyes usually don't notice the diff between "". or "" . and stuff like that. Can we even tweak coder module to scan .test files.

vladimir.dolgopolov’s picture

vladimir.dolgopolov commented

My 10 cents for new deweloper's sake:

I suggest to add more helper methods in SimpleTest for common uses cases.

I suggest add a parameter to this (and existed) methods to avoid a lots of asserts' output messages. If I test only page creation I wish to hide messages of sucessful user login and so on.

PS. SimpleTest is awesome

cwgordon7’s picture

cwgordon7 commented

+11 :) Perhaps run the whole thing through coder format?

cwgordon7’s picture

cwgordon7 commented
StatusFileSize
new simpletest_code_style.patch216.94 KB

Initial code style patch

KentBye’s picture

KentBye commented

Boombatower and I were looking at this during the code sprint, and created a sub-issue related to this for finding a consistent naming convention for the *.test files: http://drupal.org/node/231348

catch’s picture

catch
he/him
Primary language English
commented
Status: Active » Needs review
boombatower’s picture

boombatower commented

Do we want comment blocks for the file that contain test description and author?

cwgordon7’s picture

cwgordon7 commented

I just removed them to standardize things. But I also think it is not good practice to credit yourself after you write a simpletest.

catch’s picture

catch
he/him
Primary language English
commented

We should see tests being created as patches to core soon, so credits will probably happen via the commit log. Nice to have a description of the test somewhere though.

boombatower’s picture

boombatower commented

After reviewing community ideas and talking with chx and Rok Žlender we have agreed upon the last convention and I have written a script to correctly name the tests.

I started a discussion about coding standards and group convention.

boombatower’s picture

boombatower commented

Changed project name from Simpletest to SimpleTest.

boombatower’s picture

boombatower commented

Performed general cleanup of SimpleTest module files: #107173.
Tests: #107174.

boombatower’s picture

boombatower commented

Committed several other cleanup related to coding standards which are documented there.

boombatower’s picture

boombatower commented

As I see it we only have to things left.

1) Get coding standards into Coder Module
2) Clean up documentaion

birdmanx35’s picture

birdmanx35 commented

I'm working on the second now.

boombatower’s picture

boombatower commented

Changed module name in .info file to "SimpleTest instead of "SimpleTest Module" and updated description.

boombatower’s picture

boombatower commented

Fixed more naming issues is simpletest.module simpletest_menu().

http://drupal.org/cvs?commit=109535

boombatower’s picture

boombatower commented

Added un-install hook to remove variables.

http://drupal.org/cvs?commit=109886

boombatower’s picture

boombatower commented
Version: 6.x-1.x-dev »

Since I seem to keep forgetting to change the version.

boombatower’s picture

boombatower commented
Status: Needs review » Fixed

This has been completed and will stand as a record.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

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