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)
| Comment | File | Size | Author |
|---|---|---|---|
| #16 | rename.php_.txt | 1.7 KB | boombatower |
| #15 | rename.php_.txt | 1.66 KB | boombatower |
| #3 | simpletest_naming.patch | 376.68 KB | boombatower |
Comments
Comment #1
KentBye commentedcontent_actions (trigger_module.test)
Comment #2
KentBye commentedThe 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.
Comment #3
boombatower commentedKentBye 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:
Rules:
functionaldirectory and unit tests are placed in theunitdirectory.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.testmodulefolder. Tests are to be named after the module they test.Example:
path.module->path.testExample:
xmlrpc.inc->xmlrpc.inc.testChanges:
The tests that currently end with
_modulehave been moved to themoduledirectory 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.testshould 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.
Comment #4
Rok Žlender commentedNot 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.
Comment #5
boombatower commentedJust 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] > 100I 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.Comment #6
dlhubler commentedas 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.
Comment #7
catchI'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.
Comment #8
dlhubler commentedyes, 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.
Comment #9
dlhubler commentedcatch, 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.
Comment #10
catchdlhubler, 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:
Comment #11
boombatower commentedI 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.testComment #12
catchpath.module, book.module, comment.module -> book-comment-path.testSorry 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.
Comment #13
boombatower commentedAfter 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
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
modulein the name. The naming would then follow the convention below.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
modulecannot be excluded like the convention above. This would result in the convention below.Statistics
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.
Comment #14
Rok Žlender commentedLets 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.
Comment #15
boombatower commentedI 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.
Comment #16
boombatower commentedFixed script.
Comment #17
boombatower commentedI was given maintainer status so I commit changes.
Comment #18
Anonymous (not verified) commentedAutomatically closed -- issue fixed for two weeks with no activity.