Install Drupal with the Minimal install profile, then enable the Testing module.
Go to the Testing page, and run the "Search Excerpt Extraction" test (in the Search group).
You will immediately get a failure error like "Fatal error: Call to undefined function search_excerpt() " (this is a function in search.module).

Now if you enable the Search module and then run the test, you are fine.

So it seems as though simpletest is assuming it doesn't need to enable the search module, although that test says it needs to be enabled in its setup function?

  function setUp() {
    parent::setUp('search');
  }
CommentFileSizeAuthor
#18 887134-unit-test-include.patch3.42 KBmfb
#16 887134-unit-test-include.patch3.48 KBmfb
#8 887134-search.patch6.29 KBklausi
#2 unit-test-include.patch1.46 KBmfb

Comments

mfb’s picture

mfb
they or he
commented
Status: Active » Needs review
StatusFileSize
new unit-test-include.patch1.46 KB

Only the WebTests can install modules, nothing happens if you try to enable a module in a UnitTest setUp function. See http://api.scratch.drupal.org/api/drupal/modules--simpletest--drupal_web...

A work-around would be to include the module file manually.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Title: Interactive tests fail mysteriously » Setting up modules in DrupalUnitTestCase is useless
klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Needs work

Not sure what the best solution is here. Simpletest just lists all test cases on the testing page, which is a problem for unit tests that cannot specify module dependencies. We could do this hack with the manual include, but then this statement should stay in the setUp() method I think.

damien tournoud’s picture

damien tournoud commented
Status: Needs work » Reviewed & tested by the community

#2 is the correct solution.

webchick’s picture

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

This is an API change and a WTF for test writers. Don't like this approach.

We should fix this by changing DrupalUnitTestCase::setUp() or whatever so that it accepts the setUp('module') syntax in tests and does the proper require_once statement behind the scenes.

damien tournoud’s picture

damien tournoud commented
Status: Needs work » Reviewed & tested by the community

Nope.

DrupalUnitTestCase cannot install module. That's by design. There is no API change at all here.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new 887134-search.patch6.29 KB

I disagree. Setup tasks should stay in setUp(). Sorry for the whitespace fixes, Eclipse does that automatically.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

Damien: why do you think DrupalUnitTestCase should not allow the modules the use case is testing to be installed? I guess means that for any module that is not part of the Standard install profile (which I think is what is loaded during automatic tests), we cannot define a Unit Test that will run successfully by automated testing, without explicitly loading the module inside the test?

That just seems silly. Can you explain why that is the design?

mfb’s picture

mfb
they or he
commented

The UnitTests don't access the database. How would you install a module without accessing the database?

See http://api.scratch.drupal.org/api/drupal/modules--simpletest--drupal_web... "These tests can not access the database nor files. Calling any Drupal function that needs the database will throw exceptions. These include watchdog(), module_implements(), module_invoke_all() etc."

All you can do is include a module.

webchick’s picture

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

Right. So we document that setUp('module') in DrupalUnitTestCase behaves differently (require_once $foo.module) because there isn't a database available. But test authors don't have to learn a whole new test syntax.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Needs work

+1 to webchick's plan - to make it look like setup works the same in DrupalUnitTestCase as for DrupalWebTestCase.

In which case, the patches above don't do that. They are rather putting band-aids on the problem by fixing this one test case that I happened to notice doesn't work, and not addressing any other unit tests in core that might also have the same problem.

mfb’s picture

mfb
they or he
commented

Having it work the same could also cause some confusion.. Developers might think their module is actually being installed when it isn't?

I grepped the core test files. batch.test and graph.test have their require_once statements at the beginning of their test methods. The other unit tests are for required modules: filter.test simpletest.test system.test or include files: bootstrap.test common.test menu.test so I believe search.test is the only case of this problem in core.

By the way, it might be prettier to do drupal_load('module', 'search') than require_once? This function can work with or without the database so seems to work here.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

OK. Well let's get a clean patch that just does the module load for search.module then (skip the other changes), and leave it at that.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

Oh, and maybe we should also establish the pattern that all these includes should be in the setup functions? That seems like a reasonable idea from the patch in #8, so it sounds like the graph and batch tests should do that?

mfb’s picture

mfb
they or he
commented
Status: Needs work » Needs review
StatusFileSize
new 887134-unit-test-include.patch3.48 KB

here's a patch as per #15

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

This docblock needs to start with a one-line description:

+  /**
+   * Unlike DrupalWebTestCase::setUp(), DrupalUnitTestCase::setUp() does not
+   * install modules because tests are performed without accessing the database.
+   * Any required files must be explicitly included by the child class setUp()
+   * method.
+   */
   protected function setUp() {

I like the description here, so let's keep that, but all docblocks must start with a one-line description of what the function does.

Other than that, I am OK with the patch in #15.

mfb’s picture

mfb
they or he
commented
StatusFileSize
new 887134-unit-test-include.patch3.42 KB

added one-liner

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Looks fine. I think this is a reasonable approach, and at least it's now documented.

webchick’s picture

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

That works for me. What say you, Damien?

damien tournoud’s picture

damien tournoud commented

Works for me too.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Great. We all like it then. Committed to CVS HEAD.

Status: Fixed » Closed (fixed)

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