Quoting from function getFiles() in drupal_test_suite.php:

    foreach (array_keys(module_rebuild_cache()) as $module) {
      $module_path = drupal_get_path('module', $module);
      $test = $module_path . "/$module.test";

This needs to not be hard-coded to look only at files like modules/system/system.test. How do we write tests for common.inc?

CommentFileSizeAuthor
#16 unit_testing.patch5.06 KBchx
#13 find_files.patch1.51 KBAnonymous (not verified)
#6 findFiles.patch748 bytesAnonymous (not verified)
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented

common.inc can't all kinds of node stuff like have dependencies and implement hooks and so on. similarly, it can't have tests. the standard way to resolve this to date has been to use system.module as a proxy. for example, system_cron() is implemented on behalf of file.inc. so i think tests should go under system module for now. they can be implemented in their own files if desired.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

So the system module folder would have something like:

system.common.inc.test
system.path.inc.test
??

Something like that doesn't actually seem bad at all to me. Although there was discussion of splitting internal browser vs. API tests into seperate files, which'd mean two system module .test files plus the includes. Actually even that's not so bad I guess.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented

Although there was discussion of splitting internal browser vs. API tests into seperate files, which'd mean two system module .test files plus the includes.

The more I think about trying to explain to people when to put a test in the "UI" module.test and when to put a test in the "API" module.test, the more I think that we ought to have (at most) one test file per Drupal file. Period. And then come up with function naming conventions for those that are browser-based tests.

Like:

openid.test: // not openid.module.test, because this isn't for a specific file, but rather all of OpenID module.

// $Id$

// For browser tests, we extend from DrupalWebTestCase to get the browser component.
class OpenIdUiTestCase extends DrupalWebTestCase {  ...
  // Functions are name-spaced with "UI" (or browser, or..) to indicate that we're testing the module
  // "black-box" from the end-user perspective. Function names are "task"-oriented.
  function test_ui_openid_register() { ... }
  function test_ui_openid_login() { ... }
  function test_ui_openid_add_provider() { ... }
  function test_ui_openid_remove_provider() { ... }
  ...
}

// For API tests, we extend from plain ol' DrupalTestCase, cos we don't need the browser stuff.
class OpenIdApiTestCase extends DrupalTestCase {
  ...
  // These functions are named as test_NAME_OF_FUNCTION.
  function test_openid_redirect() { ... }
  function test__openid_is_xri() { ... }
  ...
}
cwgordon7’s picture

cwgordon7 CreditAttribution: cwgordon7 commented

Why not put them in the system.module directory? Or do we want to make a special-case exception and look in /includes/ or /includes/tests/ ?

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented

Yeah, I think that's what was decided above.

Btw, I'm engaging in some "documentation-driven development" and attempting to create the documentation for the handbook that we'll need in order to get 1200+ developers up to speed in this stuff. In the process, I came up with a bunch of stuff we need figured out before we start trying to train the masses at this.

I would love feedback from both those who were involved in the testing sprint, as well as those who've never written SimpleTests before @ http://groups.drupal.org/node/11020.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
FileSize
findFiles.patch748 bytes

attached is a patch following some discussion on #drupal about where to put test files.

it allows for finding tests that live in a tests directory in a modules directory: <module_dir>/tests/*.tests

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Status: Active » Needs review

forgot to set to patch needs review.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Needs review » Reviewed & tested by the community

I renamed system.test to noodle.test and put it in a tests/ directory. Confirmed that it was found. RTBC! :D

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Needs work

Not sure about introducing test-directory just yet. I think I prefer conventions over flexibility here.

The idea is to create a .test file for every file on the system (when possible):

  foo.module -> foo.test
  foo.pages.inc -> foo.pages.test
  foo.admin.inc -> foo.admin.test

If test files start to live in different places, it's going to get confusing as I can no longer guess right.

I think we should load tests in the includes directory. It's certainly possible to test the XML-RPC library that is in there.

Dries’s picture

Dries CreditAttribution: Dries commented

(BTW, xmlrpc.test in includes is currently not loaded. I think we should pick it up, load it, and run the tests.)

boombatower’s picture

boombatower CreditAttribution: boombatower commented

The SimpleTest code used to pick up test directories. This was hastily changed at the sprint. It was decided that the web interface would only run functional tests due to the requirements for unit testing.

So unit tests would be run via a script that already exists. Now that it doesn't appear unit testing will be conducted with mock functions and all this may no longer be necessary. If so we can simple revert to the old code if we want that functionality.

@#10: Following what I said above that is a unit test and thus isn't picked up by the declared functional test running web interface.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

this is a trivial patch, way less important than the conventions we decide upon and document. perhaps we ought to continue the discussion here?

FWIW, i agree with dries that it should be easy to guess where to put a test, and its hard to get simpler than the mapping he suggests.

do we need more flexibility than this?

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Status: Needs work » Needs review
FileSize
find_files.patch1.51 KB

here's a patch that does it Dries way. this includes the patch for simpletest_load, without which loading the xmlrpc test file --> WSOD.

chx’s picture

chx CreditAttribution: chx commented

Is here anyone -- boombatower? Zlender? dlhubler? cyberswat? cwgordon7? -- who knows what drupal_unit_test_case.php is and why we need it?

dlhubler’s picture

dlhubler CreditAttribution: dlhubler commented

no and doesn't appear to be incredibly useful over straight web test case.

chx’s picture

chx CreditAttribution: chx commented
FileSize
unit_testing.patch5.06 KB

That's what I thought -- simply it was a collateral, it was there when stuff got committed :)

floretan’s picture

floretan CreditAttribution: floretan commented
Status: Needs review » Reviewed & tested by the community

Patch applies cleanly, works, and I don't see any style issue with it.

As justinrandell mentioned in #12, the real issue here is the convention (upon which there seems to be a common agreement), the patch itself is trivial.

I've myself been confused about the purpose of the DrupalUnitTestCase class, so removing it is one more step that should make it easier for new people to get into the testing thing.

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Needs work

When I try to run the XML-RPC tests, I get "0 passes, 0 fails and 0 exceptions". It doesn't look like it is actually running the tests.

I'll commit the patch anyway as it gets us one step closer and it doesn't seem to break anything. Marking this code needs work.

Dries’s picture

Dries CreditAttribution: Dries commented

When I try to run the XML-RPC tests, I get "0 passes, 0 fails and 0 exceptions". It doesn't look like it is actually running the tests.

I'll commit the patch anyway as it gets us one step closer and it doesn't seem to break anything. Marking this code needs work.

floretan’s picture

floretan CreditAttribution: floretan commented
Status: Needs work » Fixed

The problem comes from the XML-RPC test itself, not simpletest.

See #255415: xmlrpc.inc.test clean-up.

Anonymous’s picture

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

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