There seems to be an inconsistent naming convention for the *.test files. At the beginning it seems that the function that was being tested was the test name (i.e. search_match.test, modules_system.test, story_edit.test or story_preview.test). But lately it seems like the tests are named after the module that they're testing (i.e. forum_module.test, book_module.test or blogapi_module.test)

Shouldn't there be more consistency for deciding how these test should be named?
For example, shouldn't modules_system.test be system_module.test and search_match.test be named search_module.test?

And should story_preview.test and story_edit.test be combined into one test and renamed story_module.test?
And should node_revisions.test and node_teaser.test also be combined and renamed to node_module.test

Are any of these technically integration tests (i.e. testing how two modules relate to each other) -- and if so, then how should they be named?

Here's a list of tests that are consistently named after the module:
* blogapi_module
* book_module
* comment_module
* forum_module
* image_module
* locale_module
* menu_module
* path_module
* poll_module
* profile_module
* translation_module
* user_module

Here is the list of modules inconsistently named:
* content_actions
* search_match (search_module.test)
* modules_system (system_module.test)
* node_revisions (node_module.test)
* node_teaser (node_module.test)
* page_creation (page_module.test)
* page_view (page_module.test)
* story_edit (story_module.test)
* story_preview (story_module.test)
* taxonomy.module (taxonomy_module.test)
* upload_tests (upload_module.test)

Not a module test -- but a unit test, and we also need a naming convention -- like should they be put into separate folders (module, unit, integration)
* xmlrpc_validator1 (unit_xmlrpc.test, xmlrpc_inc.test, xmlrpc_unit.test, etc)

Comments

KentBye’s picture

content_actions (trigger_module.test)

KentBye’s picture

The image_module.test file requires image.module, and should be deleted from the included SimpleTests and shipped with the image.module contrib module if this is indeed the case.

boombatower’s picture

Status: Active » Needs review
FileSize
376.68 KB

KentBye and I discussed this at the coding sprint.

I have come up with a naming schema that I think will create a nice layout. I have also included a patch to fix the naming schema and merge tests. I will work on cleaning up the naming inside the tests once this is agreed on.

Structure:

tests
  |- functional
    |- integration
    |- module
  |- unit

Rules:

  • Functional browser tests are placed in the functional directory and unit tests are placed in the unit directory.
  • Tests that span multiple modules and how they work with each other are placed in integration. Tests are to be named with each module that they test in alphabetical order.
    Example: path.module, book.module, comment.module -> book-comment-path.test
  • Tests that only check functionality in one module are placed in the module folder. Tests are to be named after the module they test.
    Example: path.module -> path.test
  • Unit tests are to be named after the file containing the functions they test.
    Example: xmlrpc.inc -> xmlrpc.inc.test

Changes:
The tests that currently end with _module have been moved to the module directory and the suffix has been removed.

The one unit test has been moved to unit and been renamed to match the file it tests.

The image_module.test should be removed since it isn't for a core module. (not included in patch)

Conclusion:
This will provide a general layout, but the tests themselves still need a bit of cleanup which I will work on.

Rok Žlender’s picture

Not sure I like creating new directories for diff kind of tests it makes it harder to get an overview. I would suggest appending or prepending _unit, _integration or _module to every test file. I dont think we will have that many files per module test folder.

boombatower’s picture

Just to give everyone an idea.

List of how many files would be in each folder:

  • tests/functional/integration - However many are useful.
  • tests/functional/module - 49 (http://groups.drupal.org/node/9408)
  • tests/unit - 53 (include files) + [one per core module (53 * [multiple files]) file] > 100

I think it would just make them easier to find separated and would make the file names shorter (which I like).

Obviously it would work fine with suffixes (all suffixes for consistency).

The one thing that might make sense would be to get rid of the sub-directories under functional. That would split the tests up into the two largest logical groups.

dlhubler’s picture

as far as file naming, I agree w/this
xmlrpc.inc -> xmlrpc.inc.test

outside that, i would suggest no convention, so we can be free to have very explicit file naming such
user_and_permission_upgrade.test
distributed_multi_faceted_search.test
to focus on problematic areas that are module agnostic

If directories match core identically
modules/blog/blog.module
had
test/modules/blog/blog.module.test
then it's very simple to correlate
function blog_node_info()
to it's test
function test_blog_node_info()

explaining to folks they put a test in the wrong directory because a test involved another module and now is more of a "functional test" can be frustrating for core maintainers and patch submitters.

catch’s picture

I'm pretty new to all this, but is it possible to isolate tests even if they live in the same .test file? I wasunder the (possibly wrong) impression that unit tests = one per function rather than one per module or one per file, or even multiple tests for a complex function.

dlhubler’s picture

yes, each test is isolated inside each .test file. Even setUp() and tearDown() is called before and after each test function. So grouping could be arbitrary but best practice is to group tests in a way that makes it easy to understand the grouping and a test class name that conveys useful information in a failure report

Example
UserCrudTests extends DrupalTestCase {

function test_add()

}

UserEventsTest extends DrupalTestCase {

function test_create_event()

function test_delete_event()

}

This is 100% for human readability, no logic cares how you break down your tests or what you name them, so I would encourage people to listen to others on useful strategies as you go and not get too bogged down in the beginning of setting a concrete plan.

dlhubler’s picture

catch, sorry, i probably wasn't clear on your question, a typical unit test class may have 4 or 5 test functions. But one or 50 functions under the right circumstance is ok too.

catch’s picture

dlhubler, I think I was getting confused between the multiple test functions in one class vs. multiple classes with one function discussion and how the .test files are organised overall. So no wonder you're not sure if you answered my question! All makes sense now, so a single folder for unit tests with one .test per core file sounds grand. Excuse the noise.

As to modules and functional/integration tests this does seem a little tricky - not to mention how many core modules have their fingers in other modules' business whether cleanly or not - comment and node, forum and taxonomy for example, so I can see things being moved around and arguments about where stuff belongs.

Since we're starting with a small(ish) number, I'm leaning towards one folder for unit tests, one for functional with no subfolders. With the naming convention for module specific tests in both unit and functional, but (at the moment) no convention for integration tests. If this becomes unwieldy it'd be one big cleanup patch to split things out later, but it'd probably mean less issues to start with.

So to modify boombatower's chart:

tests
  |- functional
   --user.module.test
   --some-weird-edge-case.test
  |- unit
   -- user.module.test
boombatower’s picture

I made a comment above regarding this model as another logical step. If this were used I think the integration tests need a convention, otherwise we could get a mess like we have now.

This is the same as before:
path.module, book.module, comment.module -> book-comment-path.test

catch’s picture

path.module, book.module, comment.module -> book-comment-path.test

Sorry should've mentioned I like this too. I think we could see some weird integration tests which include different bits of multiple modules, but if it's simply testing if x module works with y module then definitely x-y.test makes sense.

boombatower’s picture

After reading all the comments and obviously applying my own spin to things I have come up with the following based on the ideas above.

Structure

tests
  |- functional
  |- unit

The structure is based on the completely different nature of functional testing vs. unit testing. I think this is necessary due to the enormous number of tests and the different nature of the tests. This will also alleviate the naming issue that is encountered when a unit test is written for a module. Since both the functional test and module would have the same name (unless a screwy convention is used) it would cause problems.

Naming
Functional
Since functional tests are all based on modules and integration between modules there is no need to include the word module in the name. The naming would then follow the convention below.

single module: [module-name].test
integration: [module-name].test - create separate test class and place in same "group"

Unit
Unit testing is based on testing individual functions in a file. Because of this the tests should be named after the file that contains the functions. Since modules may have more than one file the word module cannot be excluded like the convention above. This would result in the convention below.

[filename].test

Statistics

tests
  |- functional (32 single module tests, numerous integration tests)
  |- unit (150)

Conclusion
This convention seems to encompass the most common aspects from each idea. As mentioned as a goal we need to have this figured out by March 28, 2008. I will write a script, if this idea is approved, to make the changes so that Rok Žlender can just run the script and commit the changes.

Rok Žlender’s picture

Lets go with this and see where it takes us. If we end up not liking it we can always put everything back to one folder.

boombatower’s picture

FileSize
1.66 KB

I have create a simple PHP script to perform the necessary changes.

As a note, we discussed the image_module.test and I believe it should be removed from the core tests and placed in contrib module.

boombatower’s picture

Assigned: Unassigned » boombatower
FileSize
1.7 KB

Fixed script.

boombatower’s picture

Status: Needs review » Fixed

I was given maintainer status so I commit changes.

Anonymous’s picture

Status: Fixed » Closed (fixed)

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