"test_dependencies" for dependencies / integration tests

We need to determine a way to do d7 style dependencies with version information, I don't see a reason why we couldn't add soft dependencies at the same time, they can be treated virtually the same by the testing system and just completely ingored by Drupal itself.

#102102: Parse project .info files: present module list and dependency information

Example of simple 6.x style .info file.

; $Id$
name = My Module
description = Provides my functionality.
package = Some package
core = 6.x
files[] = mymodule.module
dependencies[] = myothermodule

with 7.x style dependency information (also used as soft dependencies)

; $Id$
name = My Module
description = Provides my functionality.
package = Some package
core = 6.x
files[] = mymodule.module
dependencies[] = myothermodule

soft_dependencies[] = myothermodule (>=1.2)
soft_dependencies[] = someothermodule (>=1.1)

Then of course the same can hold true for Drupal 7, but the soft_dependencies will have a dual meaning in 6.x. They will add/override detail of regular dependencies and add soft dependencies that are ignored by drupal, but used by testing system.

So in the example above, both the soft_dependencies would be downloaded by the testing system, but the version restriction would be used for myothermodule instead of the plain entry.

I am not sure if I like the name "soft_dependencies", since I had to ask boombatower in IRC what it meant. :)

How about test_dependencies or simpletest_dependencies?

But I like the concept, I should add.

After discussing in IRC and looking for core documentation, jhodgdon found:
http://drupal.org/node/542202
http://api.drupal.org/api/function/drupal_parse_dependency/7

print_r(drupal_parse_dependency('views (2.x)'));

outputs

Array
(
    [name] => views
    [original_version] =>  (2.x)
    [versions] => Array
        (
            [0] => Array
                (
                    [op] => <
                    [version] => 3.x
                )

            [1] => Array
                (
                    [op] => >=
                    [version] => 2.x
                )

        )

)

which seems like about all will be useful from d7, since the system will need to find the -dev version that fits that criteria. I assume we'll just start form the latest and work our way down until a -dev fits those criteria. Use current algorithm if no version information specified.

The remaining question is, what to do if no release could be found. I'm thinking we cause a build error?

If a specific version is specified, like (2.0) I guess we'll just look for that specific release, otherwise always shoot for the highest dev version that meats the restrictions.

This code seems to do the trick (adapted from http://api.drupal.org/api/function/drupal_check_incompatibility/7).

$v = drupal_parse_dependency('views (2.1,2.2)');
print_r($v);

$current_version = '3.1';

if (!empty($v['versions'])) {
  foreach ($v['versions'] as $required_version) {
    if ((isset($required_version['op']) && !version_compare($current_version, $required_version['version'], $required_version['op']))) {
      echo $v['original_version'];
      break;
    }
  }
}

Revised example from above:

Drupal 6.x

; $Id$
name = My Module
description = Provides my functionality.
package = Some package
core = 6.x
files[] = mymodule.module
dependencies[] = myothermodule

test_dependencies[] = myothermodule (>=1.2)
test_dependencies[] = someothermodule (2.x)

Drupal 7.x

; $Id$
name = My Module
description = Provides my functionality.
package = Some package
core = 7.x
files[] = mymodule.module
dependencies[] = myothermodule (>=1.2)

test_dependencies[] = someothermodule (2.x)

This may apply to the 7.x system as well as 6.x, but is it possible to specify that you want specifically 2.x and not 3.x? I think when I looked at the code for version checking in 7.x, it would accept 3.x as a substitute for 2.x. That will not always work (sometimes module developers change APIs from version to version in incompatible ways, and the module could break).

>=2.x works for all 2.x and 3.x, (2.x) is only 2.1,2.2, no 3.x.

OK, then I'm in favor of the .info structure you proposed in #9 and clarified in #11.

You may also be interested in this issue on Simpletest: #699216: Module dependencies not being checked in SimpleTest 6.x (and needs doc)
Basically, the 6.x version of SimpleTest is not checking dependencies for tests. I guess it's only peripherally related to this issue, but...

Why do we need a separate .info dependencies when we already have them in the getInfo() function. Seems like unnecessary duplicate to me. :/

I don't think getInfo() can specify versions of modules.

We could and easily extend it to use the same format as the .info file syntax. I guess it's easier to parse in PIFR if it's all in .info file.

This information may also exist outside of tests.

This should be all that is needed once the parsing patch is in.

Actually, should update test as well.

#1023220: Tests enable extra, unrelated modules (and more) has been marked as duplicate of this issue...

It's my opinion that dependencies in tests should be the next push once we get current code deployed and stable.

subscribe

IRC conversation with boombatower:

<boombatower> the .info parsing should allow for optional version details and otherwise assume logically as it has code currently
<boombatower> so if 'views' which is ambigious goto latest api release
<boombatower> so if 2.x-dev and 3.x-dev then goto 3.x
<boombatower> and if stable then use, otherwise -dev
<rfay> boombatower, OK. What do you mean by 7.x-style dependency information?
<boombatower> that was the rules we came up with
* realityloop has quit (Remote host closed the connection)
<boombatower> if a module needs something more they need to add version string
<boombatower> views (3.1)
<boombatower> which is legal in .info file and is shown in the issue i just linked
<boombatower> or views (>= 3.1)
<boombatower> or whatever
<rfay> Oh. I see drupal_parse_dependency(). Never knew anything about that.
<boombatower> and alwasy assume largest possbile valid release
<boombatower> and prefer stable over dev
<boombatower> we should document those rules on a book page somewhere
<boombatower> for module devs
<boombatower> I think there were some code comments, but been awhile
<rfay> You mean in drupal_parse_dependency()? Or elsewhere?
rfay: there is code that attempts to follow the rules I just laid out
<boombatower> and I believe there are code comments, I'll look
<boombatower> should be in the dependency resolving coed that take component and tries to match release
<boombatower> which is why I don't see how you can do in one phase
<boombatower> you need all the components and releases before you can resolve dependencies
<boombatower> since one may depend on views_ui which requires the views project, but that isn't known until you parse all the projects
<boombatower> at least once obviously

We now have the ability to download dependencies, so time to take this up again. First the stuff in project_dependency will have to be updated for this.

Are there currently any workarounds for this?

I maintain the Mailhandler module, which has a test that runs Coder Review. Obviously Coder isn't a hard dependency, but I'd really like the automated test bots to be able to run the test...

Ok, so is the soft dependency feature implemented? How do I use it to test a patch?

I wrote a patch for the fivestar module, but can get it to pass the tests via the testbot(http://qa.drupal.org/pifr/test/203748). On my local installation it does pass the tests.

The issue, i think is fivestar's test case class extending the AJAXTestCase class. The AJAXTestCase then requires the 'ajax_test' and 'ajax_forms_test' modules to be enabled, which fails because fivestar doesn't include those modules as dependencies(which is right, because it doesn't need/depend on them).

ericduran(one of the maintainers of fivestar) has an issue related to this in http://drupal.org/node/1334506, but there has been no response so far on that.

Add a 'dependencies' => array('ajax_test', 'ajax_forms_test') definition to the getInfo() method. This isn't a fix, but it will prevent the testbot from executing tests which are doomed to fail anyway.

Thanks for the response.

I'm still lost here. Do you mean i should change the lines below:

 public static function getInfo() {
    return array(
      'name' => 'Fivestar widgets',
      'description' => 'Make sure fivestar widgets can be created and used.',
      'group' => 'Fivestar',
    );
  }

to

 public static function getInfo() {
    return array(
      'name' => 'Fivestar widgets',
      'description' => 'Make sure fivestar widgets can be created and used.',
      'group' => 'Fivestar',
      'dependencies' => array('ajax_test', 'ajax_forms_test'),
    );
  }

in the fivestar.test file. If so, would that mean that i'll have to patch that file or how does my changes get to the testbot?

Yes, exactly.

This enforces that a test class is only executed if the necessary dependencies are met. And yes, you need to patch the module to do this, either as part of your patch or in a separate issue.

Obviously, if this is the only test case, this means that testbot won't run anything. But maybe you can move the test cases which require ajax_test into a separate class, if there are actually some that don't require it.

Actually the 'workaround' I've used for this is to move features with soft dependencies into submodules with hard dependencies. Architecturally I think this is actually a better design anyway.

EDIT: Nevermind, this doesn't actually work unless the main module is dependent on the submodule...

Moving over to Project Dependency, as its calculations are what determines what modules are loaded.

I think this patch gets the ball rolling. It parses the test_dependencies in the info file (I haven't experimented with this yet) and puts them into the database. The next thing that has to be accomplished is to have the actual dependency generation (at request time) take into account these requirements. I think this will require some signature changes to a couple of functions, and it may very well be that we should change the dependency type to a mask (or maybe a string).

This was all part of the original patch...I get more and more confused the more I read these issues. All the goods were dropped and the structure changed so it didn't fit in project....meh.

FYI here is a similar issue in the testbot issue queue: #1347790: Specify dependency overrides on the Automated Testing tab

I have posted a proposal to allow dependency overrides on #1347790: Specify dependency overrides on the Automated Testing tab, comment #3.

One problem I see with specifying test dependencies in the .info file is it is the wrong scope. If the information pertains to testing it should appear there and not at the module scope (if possible). It is the equivalent of using a global variable when a local variable would suffice.

But the approach in the other issues makes it specific to d.o which is silly. We need less of that, much less. It is just like module dependencies only these are more like "recommends" from the linux packaging world aka things I integrate with so I obviously need them during testing to see if I integrate well. If we really don't want to put in the .info file...great but then with the .test files or somewhere within the code tree. Frankly I think we should just leave them in .info files.

I agree with boombatower, the .info-file is a better idea than a field on drupal.org. That way, if you download a module for testing, you can easily see which projects it needs to run those tests. Else you would have go to the automated tests tab to gain that information (or examine the code). Another benefit is that you can patch an automated test with an extra soft dependency (although this may hardly occur).

I also like the (striked through) idea of Dane Powell in #31: that you add a specific test module which lists all dependencies. (I already tried that with Ubercart Addresses, figured why that didn't work, and then I saw Dane's comment was striked through.)

The hidden test module trick should work fine, you just need to give it some time to rebuild the dependencies, which usually happens when a new -dev snapshot is built. There is no need for the main module to depend on the hidden one. I have multiple projects which have optional modules that have external dependencies and that works just fine.

There is no requirement to have a "main module" that is named like the project and that would be the only way to detect that a module is the main one. Pretty sure that the dependency builder script looks at all modules in a project.

@Berdir,
Well, in case of Ubercart Addresses 7.x-1.x that didn't work. I had added a hidden module in a subfolder called 'tests' and (some of) the dependencies noted there are not loaded (for example rules). Not even after other commits and several rebuild snapshots. Or did I something wrong?
http://drupalcode.org/project/uc_addresses.git/blob/4170010c4162b94e2a5d...

@MegaChriz, please file an issue when you have a specific problem.

Followups about dependencies on the Automated testing tab should be over in #1347790: Specify dependency overrides on the Automated Testing tab

I don't support doing dependency overrides instead of .info file dependency generation, but I do support it as an override technique.

The hidden test module trick should work fine, you just need to give it some time to rebuild the dependencies, which usually happens when a new -dev snapshot is built. There is no need for the main module to depend on the hidden one. I have multiple projects which have optional modules that have external dependencies and that works just fine.

This doesn't work for me, see #1670936: Testbot does not detect/download additional dependencies.

@MegaChriz I think I have the same problem. Did you file a new issue? did you find out how to fix it?

@lucascaro
No, I didn't filed an issue about it, because sometime later all automated tests of Ubercart Addresses suddenly got executed by the testbot. At first, I thought somebody who watched this issue had fixed it for me without notifying me, but later (and now) I guess dependencies of dependency projects are picked up now by the testbot (correct me if I'm wrong), as I only listed dependencies in the hidden module that already were dependencies of Ubercart or the Ubercart Addresses main module. Because I could no longer demonstrate the problem, I saw no point in opening an issue about it afterwards (and maybe also because I forgot to do something with this issue).

@MegaChriz
I believe it was #1507650: "Failed to check out": Internal dependencies not followed properly within a project (extra dependency) that solved your problem.

Well, as I'm having a similar problem (the bot doesn't pick up the dependencies) I've created an issue at: #1712656: Dependencies for modules are not updated....

Came here from #1804716: Setup optional modules fails -> patch review fails and asking me if this feature may go in soon. This case is open for nearly 2 years. What are we waiting for?

The patch in this issue has never been tried out.

There may be existing code now in the module that would support this if tested.

We're just waiting for somebody to push this one through. It would certainly be helpful for @hass's case.

Right now is a fairly unsuitable time to work on any of this stuff because of the pending D7 move. At least that's my line.

The patch in #33 probably gets us pretty close to in range. We could also just add the test_dependencies to the regular list of dependencies.

I think this one will do the job. It's based on #33, but there are a few more details that were required.

This adds "dependency_type" handling (required or recommended). It will require changes to PIFT to be useful.

Note that we *could* just add test_dependencies found in the info file to the regular dependencies and process them the same. This would work fine for the testbots (and then would require no pift changes), but might cause problems for other potential future uses of this information.

Committed to 6.x-1.x in http://drupalcode.org/project/project_dependency.git/commitdiff/952e481

Moving to 7.x

#1833154: Enable optional token tests on d.o. It fails. Custom variable tests are all skipped.

Please see #1804716-22: Setup optional modules fails -> patch review fails for information.

Ported to D7 and committed in http://drupalcode.org/project/project_dependency.git/commitdiff/5ee8fcf