When using an install profile that is packaged as a distribution (ie. its projects live in profiles/[name]/modules) tests in any modules therein cannot be run successfully. To reproduce:

  1. Install D7 using the standard install profile
  2. Create a modules directory at profiles/standard/modules
  3. Move one of the core modules (e.g. poll) from modules/poll to profiles/standard/modules/poll and then clear your caches.
  4. Run poll tests and watch #fail

While the scenario above is obviously unrealistic, downloading an install profile from Drupal.org that contains, say, views in profiles/[name]/modules is a very real case.

Problem

The problem is caused by the module whose test is to be run not being available in drupal_system_listing() given that it lives in a profile directory:

  1. Views is in profiles/[name]/modules/views,
  2. When Views' tests are run, the install profile used is standard, leading to the following paths scanned by drupal_system_listing():
    • profiles/standard/modules
    • sites/all/modules
    • sites/[x]/modules
  3. Views is obviously not in any of these directories and so the test fails as it cannot enable Views from the get go.

Approaches

Some possible approaches,

  1. Treat this as a UI problem. Don't show any tests for modules in profiles/[name]/modules. Install profiles and distros would not be able to run tests for any of their modules.
  2. Always have $test->profile use the current install profile if the installed profile is not one of the Drupal core install profiles. Tests will likely take much longer to run, especially when they could just be using the lighter testing profile.
  3. Adjust drupal_system_listing() to use not only drupal_get_profile() but the 'parent' install profile (the install profile of the site that the test is running on).
Files: 
CommentFileSizeAuthor
#85 911354-drupal-profile-85.patch1.89 KBhefox
PASSED: [[SimpleTest]]: [MySQL] 40,337 pass(es).
[ View ]
#76 drupal7.simpletest-profiles.76.patch7.71 KBlangworthy
PASSED: [[SimpleTest]]: [MySQL] 37,847 pass(es).
[ View ]
#73 drupal8.simpletest-profiles.73.patch7.54 KBsun
PASSED: [[SimpleTest]]: [MySQL] 34,195 pass(es).
[ View ]
#70 drupal8.simpletest-profiles.70.patch7.5 KBsun
PASSED: [[SimpleTest]]: [MySQL] 34,619 pass(es).
[ View ]
#69 drupal8.simpletest-profiles.69.patch7.72 KBsun
PASSED: [[SimpleTest]]: [MySQL] 34,620 pass(es).
[ View ]
#66 simpletest-profiles-911354-66-D7.patch5.05 KBjrbeeman
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch simpletest-profiles-911354-66-D7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#62 911354-62-simpletest-profiles.patch5.37 KBjhedstrom
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 911354-62-simpletest-profiles.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]
#60 911354-60-install-profile-tests.patch5.4 KBlangworthy
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 911354-60-install-profile-tests.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]
#58 911354-57-install-profile-tests.patch5.37 KBjhedstrom
PASSED: [[SimpleTest]]: [MySQL] 33,656 pass(es).
[ View ]
#56 911354-56-install-profile-tests.patch2.46 KBjhedstrom
FAILED: [[SimpleTest]]: [MySQL] Repository checkout: failed to checkout from [git://git.drupal.org/project/drupal.git].
[ View ]
#46 911354.46.patch2.76 KBdixon_
PASSED: [[SimpleTest]]: [MySQL] 30,367 pass(es).
[ View ]
#43 911354.43.patch2.61 KByhahn
PASSED: [[SimpleTest]]: [MySQL] 26,024 pass(es).
[ View ]
#19 search_path_4.patch9.05 KBadrian
PASSED: [[SimpleTest]]: [MySQL] 25,319 pass(es).
[ View ]
#17 search_path_3.patch9.03 KBadrian
FAILED: [[SimpleTest]]: [MySQL] 25,298 pass(es), 5 fail(s), and 28 exception(es).
[ View ]
#15 search_path_add.diff7.89 KBadrian
PASSED: [[SimpleTest]]: [MySQL] 25,308 pass(es).
[ View ]
#9 extra_system_dirs.patch3.42 KBadrian
PASSED: [[SimpleTest]]: [MySQL] 25,297 pass(es).
[ View ]
#8 extra_system_dirs.diff3.42 KBadrian
PASSED: [[SimpleTest]]: [MySQL] 25,313 pass(es).
[ View ]
#6 911354-host-profile.1.patch2.83 KByhahn
PASSED: [[SimpleTest]]: [MySQL] 24,822 pass(es).
[ View ]
#5 911354-ignore-profile-modules.patch1.4 KBboombatower
PASSED: [[SimpleTest]]: [MySQL] 24,832 pass(es).
[ View ]
#4 911354-ignore-profile-modules.patch2.26 KBboombatower
FAILED: [[SimpleTest]]: [MySQL] 23,753 pass(es), 45 fail(s), and 38 exception(es).
[ View ]
#3 911354-ignore-profile-modules.patch1.41 KBboombatower
PASSED: [[SimpleTest]]: [MySQL] 24,805 pass(es).
[ View ]

Comments

This is the culprit:

<?php
 
/**
   * Preload the registry from the testing site.
   *
   * This method is called by DrupalWebTestCase::setUp(), and preloads the
   * registry from the testing site to cut down on the time it takes to
   * set up a clean environment for the current test run.
   */
 
protected function preloadRegistry() {
   
$original_connection = Database::getConnection('default', 'simpletest_original_default');
   
db_query('INSERT INTO {registry} SELECT * FROM ' . $original_connection->prefixTables('{registry}'));
   
db_query('INSERT INTO {registry_file} SELECT * FROM ' . $original_connection->prefixTables('{registry_file}'));
  }
?>

This is plain broken. preloadRegistry() (or the code that calls preloadRegistry()) needs to check whether the search path of the host site and the test site are the same before it decides to copy the registry of the host site to the client site.

i may have shot too fast here. preloadRegistry should actually make sure that the host site's registry is used, thus modules sitting in the host site's profiles directory should always be found...

Assigned:Unassigned» boombatower
Status:Active» Needs review
StatusFileSize
new1.41 KB
PASSED: [[SimpleTest]]: [MySQL] 24,805 pass(es).
[ View ]

After a discussion in IRC confirmed that the testing system cannot test modules in a profile since it cannot guarantee the modules tests will enable the profile and thus make the module available. Hacking the modules system could fix this, but does not seem appropriate. As such the only useful test is the profile's test which is assumed to enable itself and can test any of the modules included with the profile. Considering the only modules this will be an issue for are the modules written for the profile (aka not views or other contrib modules) which can (and make sense) be tested by the profile itself.

If you want to run tests for modules included in a profile move (or sym link) them into one of the sites module directories.

The patch ignores all modules in profiles (expect the profile itself) so that the UI and run-tests.sh script do not list module tests they cannot run.

StatusFileSize
new2.26 KB
FAILED: [[SimpleTest]]: [MySQL] 23,753 pass(es), 45 fail(s), and 38 exception(es).
[ View ]

or

StatusFileSize
new1.4 KB
PASSED: [[SimpleTest]]: [MySQL] 24,832 pass(es).
[ View ]

My luck, commit as I roll.

StatusFileSize
new2.83 KB
PASSED: [[SimpleTest]]: [MySQL] 24,822 pass(es).
[ View ]

Discussed this issue further with alex_b and we agreed that an approach where hiding tests for modules in profiles/[x] would be a serious blow to Drupal distributions and promoting usage of Drupal as a platform for building products. For example, this would prevent any of the install profiles packaged by Drupal.org from being able to run the tests for their contrib modules. Symlinking from sites/all/modules is not a real solution to me and requires further explaining to users why such a hack is necessary.

The patch below implements option #3 listed in the initial post -- Simpletest sets the host profile to be used by drupal_system_listing().

Status:Needs review» Needs work

instead of hardcoding these paths into the drupal_system_listing, I think a simpler
and more elegant approach would be to add a drupal_add_search_path() function,
which will allow you to add the path to a control array.

So we would do :

<?php
  drupal_add_search_path
('host_profile', "profiles/{$profile}");
?>

Other useful places to configure additional paths include settings.php and sites.php.

StatusFileSize
new3.42 KB
PASSED: [[SimpleTest]]: [MySQL] 25,313 pass(es).
[ View ]

here's a first roll of the patch.

This only handles the host_profile case for now, but we can add additional places to specify paths.

StatusFileSize
new3.42 KB
PASSED: [[SimpleTest]]: [MySQL] 25,297 pass(es).
[ View ]

test bot hates me ?

renamed to .patch.

We should not be running tests for the contrib modules in an install profile as those are run on the contrib project already...no need to waste the testbot's time with overly redundant testing. Hacking core to achieve this purely for testing isn't any better either.

The profile can run tests for any of it's own modules...and any glue code with contrib modules, configurations of contrib modules, or whatever, but running the contrib tests verbatim is just bad. Since they won't run with the install profile they are found in (ie they will use standard profile during tests) that is confusing for one and a waste for the testbot and doesn't make a lot of sense. Contrib projects should not be micromanaged in distributions (tests, issues, or anything). Distributions simply package contrib modules, configure them, and provide glue code. If anything we need to display the test results for contrib modules on the distribution project page.

no need to waste the testbot's time with overly redundant testing

I'd like to find a different solution for that other than axing patch #9.

The point of #9 is strengthening distros. To that end we need to find a solution on how to run tests of arbitrary contrib modules shipped with a profile.

BTW, the real conceptual problem here is that module tests depend on profiles. This collides with the fact that profiles have their own search path.

I view the current behavior as a regression. It was possible to run tests of any contrib module residing in a profile in D6.

Including all profiles in the search path seems like including all sites in the search path...how is that what we want? Or allowing one to run tests from siteb in sitea...sure it is possible, but really?

I dunno...this is obviously possible I'm just not sure it makes sense...and having to special case (like any special case) the testbot seems like another pointer that this isn't a clean solution.

So what is/are our usecase(es) here?
- developer has profile installed and wants to run tests on contrib module locally? if he is developing the contrib module (or finds bug) he will need a checkout to make patch or commit? which drush make won't give you so won't they need to checkout the module (which could be done in site folder) in order to work on it? (maybe not? obviously could make raw diffs with some hacking, but...)
- want to see summary of contrib modules for distro (d.o functionality)
- end users should never be running tests

I mean normally people shouldn't be running tests locally when testbot does it already and has optimized machines that do it faster (if anything we should improve it). So what exactly is our usecase here? I don't do distro work, but those in this issue I know have more experience in that area...so what does that look like?

My understanding is the distros have drush make scripts that grab the contrib modules...so unless there is an alternative running tests on those checkouts is fairly useless since development will need to be done against a checkout of dev not a stable dump, right?

developer has profile installed and wants to run tests on contrib module locally? if he is developing the contrib module (or finds bug) he will need a checkout to make patch or commit? which drush make won't give you so won't they need to checkout the module

Just add --working-copy to it.

(which could be done in site folder) in order to work on it? (maybe not? obviously could make raw diffs with some hacking, but...)

Every module would need to be symlinked to the sites/all folder, because if you write a test for a custom module that depends on a contrib module, those modules would not get enabled at all.

What happens is that the testing interface shows you a bunch of things to be tested, but then fails when you try to test to run those tests because they are no longer found.

Including all profiles in the search path seems like including all sites in the search path...how is that what we want? Or allowing one to run tests from siteb in sitea...sure it is possible, but really?

We are not including all profiles, only the host site's profile is getting included.

For example, if modules are present in sites/sitename.com/modules you will be able to test them because the 'test site' is installed as 'sitename.com' too and therefor sites/sitename.com/modules is in your search path.

If the site directories had the same behavior as profiles have currently, the testing interface would show you the modules in your interface, but when you run them the site will be installed explicitly as 'testing.com' (or whatever the test required) and only sites/testing.com/modules is added to the search path. Hence all the tests fail because the 'test site' cant actually find the modules it needs to test.

Status:Needs work» Needs review
StatusFileSize
new7.89 KB
PASSED: [[SimpleTest]]: [MySQL] 25,308 pass(es).
[ View ]

here's a new version of this patch. I moved more of the paths to the control array.

I also moved some functions to bootstrap.inc, because they get called during bootstrap.

This would also help #545452: Store install profile in the generated settings.php file , as the drupal_add_system_directory could be called after including the settings.php.

Title:Tests in profiles/[name]/modules cannot be runMake system directories configurable to allow tests in profiles/[name]/modules to be run.

Improve the title to illustrate the approach.

Related issues :
#295434: Support contrib being placed in profiles/all - Allows additional paths to be added by the profile maintainer.
#545452: Store install profile in the generated settings.php file - Simplify profile selection so we can do it earlier in the bootstrap process.
#25720: Add additional system directories to search within settings.php - We could finally do this, very easily, and without globals.

StatusFileSize
new9.03 KB
FAILED: [[SimpleTest]]: [MySQL] 25,298 pass(es), 5 fail(s), and 28 exception(es).
[ View ]

Here is a more complete version of the patch that handles priorities a lot more cleanly.

I ironed out the concept of 'scope' so that there are now 3 primary levels of precedence. They are system-wide, profile-wide, site-wide.
site wide takes precedence over profile wide takes precedence over system wide.

Each of these 3 levels also have 'before' or 'after' levels too. For instance, the sites/all search path is actually added "before:site", meaning it has precedence over anything in the profiles, but not in sites/$sitename.

This was necessary so that you can target what priority the additional search paths should have. A good example here is related to #295434: Support contrib being placed in profiles/all
To add an additional profiles/all search path, the profile developer simply needs to add the following to his .profile file :

<?php
 
function standard_init() {
   
drupal_add_system_directory('before:profile', 'profiles/all');
  }
?>

The other reason I did this, is because we no longer need to have a use case specific override for the host site's profile.
We can accurately determine where we are adding the path without needing to modify core to make a provision for it.

Status:Needs review» Needs work

The last submitted patch, search_path_3.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new9.05 KB
PASSED: [[SimpleTest]]: [MySQL] 25,319 pass(es).
[ View ]

Fix tests. The file path had a '.' prefixed due to the handling of the system level modules/themes dir. I now have special handling for those paths.

subscribe

subscribe + my 2 cents:

Thanks to those who are working on this patch - I would consider this extremely important as someone who bears a good amount of the overarching responsibility of whether the sites we launch are all up to snuff.

For anyone who is doing the majority of their development using install profiles, this is something that is absolutely necessary. We want to have full test coverage on any custom code/features within our profile that are dependent on contrib modules, just like for any part of core.

Unless I'm missing something, it sounds like we would have severely incomplete test coverage without fixing this? Never mind that it could help with some of the other (highly annoying) issues that we've come across working this method such as #545452: Store install profile in the generated settings.php file.

I'm don't think symlinking should be considered a fix to this. As a developer who has benefited from the ability to test custom install profiles in D6, not fixing this for D7 seems like a step backwards.

Also, why is the priority of this normal when the problem removes testing abilities when upgrading D6 distributions to D7?

My company build and supports 2 distros (with a 3rd on the way) and we consider test coverage paramount in the development of our install profiles. There are already 100s of things that make building and maintaing these distro difficult and very time consuming. Having something like #19 from adrian in place will make testing new releases a much cleaner and easier process w/o workarounds.

+1

Huge thanks to all who are working on this;

1. Agreed with langworthy in #22, since this exists in D6 not fixing for D7 is certainly a step back.
2. As others have indicated symlinking is not a solution
3. Adrian's explanation in #14 is a good summary of the awkwardness of the current implementation
4. This will make it easier for shops to maintain distributions/install profiles which is a good thing for the Drupal community.

+1

subscribing

I mean normally people shouldn't be running tests locally when testbot does it already and has optimized machines that do it faster (if anything we should improve it). So what exactly is our usecase here? I don't do distro work, but those in this issue I know have more experience in that area...so what does that look like?

In a nutshell: anyone developing an installation profile for use with Drupal who wants to use SimpleTests. There are theoretically ways around the problem, but they all involve a huge amount of wrangling to get around what was previously doable with out-of-box SimpleTest module.

We're building what amounts to a custom distro for a large client that will be relying on SimpleTest for regression testing: they're definitely concerned about D7 portability moving forward, and losing the ability to test their profile system without all sorts of symlinked singing and dancing would be a real disaster.

I'm curious if boombatower is still leaning against this one after hearing the testimonials from distro folks. I like the idea of supporting distros natively instead of requiring a symlink workaround. It seems broken to simply hide the tests in the UI.

Boombatower said one thing that maybe hasn't been fully addressed:

We should not be running tests for the contrib modules in an install profile as those are run on the contrib project already...no need to waste the testbot's time with overly redundant testing.

I have a couple of questions about this:

  1. Is it possibly a good thing to run tests on distros, so that end users can see that the distro is good to go, rather than the users having to cross-reference all the contrib modules too? Or is this info aggregated in in the distro info?
  2. Would it be possible to turn this off for the testbot but still allow devs and end-users to run the tests without the symlink workaround (which seems lame)
  3. If it can't be turned off, what kind of burden are we talking about here?

I'm also curious what changed from D6 to D7 that caused this to break. Is it any indicator of an important change that this patch would be un-doing?

The move to test driven development in Drupal will (and have already) changed things to be much better. And the distro movement is also very very important for Drupal, as Dries keeps mentioning from time to time. We need to fix this or a smooth test driven development will never happen for distros in D7. As said, two very important things for Drupal.

As @langworthy says, symlinking is not a solution to the problem. Regardning @boombatower's concernc on this issue I think @adrian sums it up very well:

We are not including all profiles, only the host site's profile is getting included.

For example, if modules are present in sites/sitename.com/modules you will be able to test them because the 'test site' is installed as 'sitename.com' too and therefor sites/sitename.com/modules is in your search path.

If the site directories had the same behavior as profiles have currently, the testing interface would show you the modules in your interface, but when you run them the site will be installed explicitly as 'testing.com' (or whatever the test required) and only sites/testing.com/modules is added to the search path. Hence all the tests fail because the 'test site' cant actually find the modules it needs to test.

So huge +1 for this. Maybe bump the priority of this issue?

Does this need to be in before beta or 7.0 release or can it be implemented in a point release?

Does this need to be in before beta or 7.0 release or can it be implemented in a point release?

While it's not a major API change, I'd be very hesitant to change something about the way our testing system works after a major release. It seems like that is something we would want to stay consistent.

This looks a bit over-engineered. Why is a simple drupal_alter() not sufficient?

FWIW, for http://drupal.org/project/edge, we are also in the need to be able to place themes for testing purposes into the module's directory, so I recalled this issue, but the functionality of this patch does not make it any simpler than achieving the same through a dirty hack.

What is being perhaps missed here (although mentioned by some above), is that many are doing ALL site development for clients as install profiles -- i.e. common every day site building functionality. So, very important to have this work "out of the box" (man, I'm getting tired of writing that...)

RE: "many are doing ALL site development for clients as install profiles" - yes. I suspect that as install profiles become easier to build, this will come to be best practice, and eventually something that everyone just does (remember building sites before CCK and Views?).

Losing the ability to test install profiles without hacks is a big loss, and it will create unnecessary barriers in the way of creating and maintaining install profiles throughout the D7 lifecycle.

Big +1 to addressing this now, and addressing this in core.

To me, this sounds like a pretty straight-forward regression from what was possible in D6 and should be fixed in D7. Symlinks are not an answer, and neither is the response that contrib can test its own modules. While true, the tests that contrib does will test each module individually. A profile will likely have one or more custom modules in it that will want to test the entire stack. Am I missing any other objections?

Obviously, though, what we do to fix here should be as non-invasive as possible, given that we're past beta now.

We could do with a hell of a lot less +1ing and a hell of a lot more actual code reviews in this issue, however.

Status:Needs review» Needs work

We do not do anything like that. It's WAY overengineered. And, all this chatter did not even brush the real problem: tests will break if not ran in the profile they are designed for. For example, one presumes the existence of certain node types. Presumes the status of modules and as a consequence the DOM of the page when running XPath over it. So just making it possible (with a simple solution) to get a test to run on another profile, well it won't solve anything at all.

Also, any nontechnical chatter followup will be deleted without further warning.

tests will break if not ran in the profile they are designed for

chx, please read the entire thread or at least the original post. The issue is not about running test A with a different profile than it was designed for. It is about the scope with which drupal_system_listing() makes code available and how this can cause problems for modules and their tests in profiles/[x].

subscribe.

I read the issue and for example the opening post contradicts itself. How so? the first list is about "Move one of the core modules (e.g. poll) from modules/poll to profiles/standard/modules/poll and then clear your caches." but then the second list says "When Views' tests are run, the install profile used is standard, leading to the following paths scanned by drupal_system_listing(): profiles/standard/modules" erm, so profiles/standard/modules is good, after all? What is your problem, after all?

We're discussing on irc now to clear this up.

which channel? or did i miss it in #drupal-contribute already?

So the issue is superb convoluted. If your test specifies and the system contains profile X (say, standard) but you have installed the profile Y and the module is under it then profile X will be set up for the child Drupal and so the module will not be found under Y. The test needs to use X, yes, but the search needs to happen under Y. Note that if the profile X is such that it contains modules that are also under Y then a lot of of grief is going to happen. We can't help that for now. The hope is that X is indeed a core profile which usually does not contain modules. I proposed a superb small, insanely targeted fix (iterated with yhahn) and it's at http://paste.pocoo.org/show/272516/ , yhahn is rolling a patch with added comments, simpletest use it and a test. It's all good now.

There. That looks more like it. ;)

Thanks for working that out, guys!

Status:Needs work» Needs review
StatusFileSize
new2.61 KB
PASSED: [[SimpleTest]]: [MySQL] 26,024 pass(es).
[ View ]

As mentioned by chx, this patch takes a minimal approach to addressing this problem.

  • Introduces simpletest_parent_profile variable that is only meant to be used by simpletest to add one additional profile (the parent site's active profile) to the test site's search paths.
  • Adjust DrupalWebTestCase to set the parent profile during ->setUp() and delete it during ->tearDown().

Tested successfully with tests in profiles/[x] that use the testing and standard profiles.

I successfully tested yhahn patch from #43.

Status:Needs review» Needs work

It still needs comments in drupal_system_listing and I am wondering whether a test would be possible (without making the testing profile another test module dumping ground, webchick did not like that)

Status:Needs work» Needs review
StatusFileSize
new2.76 KB
PASSED: [[SimpleTest]]: [MySQL] 30,367 pass(es).
[ View ]

Here is yhahn's patch rerolled with some more comments in drupal_system_listing() as per chx's request. I can't figure out how to write a test for this, though.

Thanks @dixon_ ... the only way I can think of to write a test for this is to actually create a test module in profiles/standard/modules that uses, say, the testing profile. Maybe there is another way that doesn't spam the profiles with test modules?

It looks like discussion of how (or if) to write a test for this is semi-postponed on #938600: Decide what to do with test modules under testing profile.

If I understand this issue correctly, though, then because the bug depends on the profile that the main site (the site which is running the tests, not the testing profile itself) is using, the only way to test this would be to do some kind of "meta-test"? Basically, put a module in profiles/testing/modules which contains a test which itself specifies use of the 'standard' profile. Then, write a test which uses the 'testing' profile, and within that test, have the Simpletest browser go through the admin UI and make sure that it can successfully run the module's test. It seems a little scary, but would probably work. Not sure if it's worth the effort... (It does look like within modules/simpletest/simpletest.test there are already some "meta-tests" that run tests from within the tests though.)

The patch itself looks fine to me. Ugly, but effective :)

Is this still under consideration for D7? This post on Development Seed's blog suggests that they need it for the work that they are doing with distributions in D7: http://developmentseed.org/blog/2010/sep/30/features-and-exportables-dru....

subscribing

subscribe

#46: 911354.46.patch queued for re-testing.

Stalled?

Issue tags:+Testing system

The blocker issue mentioned in #48 is resolved and it seems David is correct that we can put a module in profiles/testing/modules.

Given this is a regression of D6 and that the patch seems to have the okay do we require a test for this to be committed?

Version:7.x-dev» 8.x-dev
Issue tags:+needs backport to D7
StatusFileSize
new2.46 KB
FAILED: [[SimpleTest]]: [MySQL] Repository checkout: failed to checkout from [git://git.drupal.org/project/drupal.git].
[ View ]

This would presumably have to be applied to 8.x first. Here's the same patch from #48 re-rolled against 8.x. I'm currently researching whether tests are possible for this patch.

Status:Needs review» Needs work

The last submitted patch, 911354-56-install-profile-tests.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new5.37 KB
PASSED: [[SimpleTest]]: [MySQL] 33,656 pass(es).
[ View ]

Here's the same patch as from #56, but I also added a test. This test first bundles a test case under the 'testing' installation profile. There is then a new test case in simpletest.test that enables the testing profile, and verifies that the test bundled with that profile appears on the test admin form.

The patch in #58 works for me.

I used Drupal 8.x HEAD and copied the standard install profile to another directory and installed to that profile. I grabbed the 7.x-dev versions of views and ctools, placed them in my new profile directory, and changed the core version to 8.x. I ran a single views test and it failed completely. When I applied the patch in #58 and ran the test again it passed.

StatusFileSize
new5.4 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 911354-60-install-profile-tests.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]

re-roll of #58

Status:Needs review» Needs work

The last submitted patch, 911354-60-install-profile-tests.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new5.37 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 911354-62-simpletest-profiles.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]

Looks like the testbot couldn't apply the patch in #60 for some reason. Here's another reroll.

My bad. It looks to me now that #58 doesn't need a re-roll when applying the patch using git apply with drupal.git.

For some reason the patch won't apply for me when using drush make. But #60 does.

Status:Needs review» Needs work

+++ b/includes/common.inc
@@ -5151,8 +5149,14 @@ function drupal_system_listing($mask, $directory, $key = 'name', $min_depth = 1)
+  $profiles = array(drupal_get_profile(), variable_get('simpletest_parent_profile', ''));
...
+    if ($profile && file_exists("profiles/$profile/$directory")) {

Minor: if we do

<?php
if($profile)
?>
, we might as well return FALSE as default value in variable_get()

+++ b/modules/simpletest/simpletest.test
@@ -503,3 +503,31 @@ class SimpleTestMissingDependentModuleUnitTest extends DrupalUnitTestCase {
+    // There is a test bundled with the drupal_system_listing_compatible_test module.

That's just a liiiittle bit too long. :)

+++ b/profiles/testing/modules/drupal_system_listing_compatible_test/drupal_system_listing_compatible_test.info
@@ -4,3 +4,4 @@ package = Testing
\ No newline at end of file
+++ b/profiles/testing/modules/drupal_system_listing_compatible_test/drupal_system_listing_compatible_test.test
@@ -0,0 +1,17 @@
\ No newline at end of file

:(.... :)

+++ b/profiles/testing/modules/drupal_system_listing_compatible_test/drupal_system_listing_compatible_test.test
@@ -0,0 +1,17 @@
+ *   Include a test to make sure tests withing installation profiles
+ *   can be run by simpletest.

Is that actually a standard or are there just a lot too many spaces there?

Could we just call this new variable install_profile? Drush already honors that variable when set. This variable name comes from #545452: Store install profile in the generated settings.php file which tried valiantly but failed to get into D7.

StatusFileSize
new5.05 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch simpletest-profiles-911354-66-D7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

@moshe, I was confused about it too, but upon further inspection, it looks like the simpletest_parent_profile variable is set using $this->originalProfile.

I've backported the patch in #62 to apply against Drupal 7.x head and attached it here. The patch appears to be working great for our D7 install profile.

Status:Needs work» Needs review
Issue tags:-needs backport to D7, -Testing system

#62: 911354-62-simpletest-profiles.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+needs backport to D7, +Testing system

The last submitted patch, 911354-62-simpletest-profiles.patch, failed testing.

Title:Make system directories configurable to allow tests in profiles/[name]/modules to be run.Tests in profiles/[name]/modules cannot be run and cannot use a different profile for running tests
Assigned:boombatower» sun
Status:Needs work» Needs review
StatusFileSize
new7.72 KB
PASSED: [[SimpleTest]]: [MySQL] 34,620 pass(es).
[ View ]

As I'm neck-deep into the testing system currently, let me drive this home.

Re: #65: Nope, 'install_profile' would be a bogus name, as the variable does not hold the name of the actual installation profile, but instead, the profile of the parent site that is executing a test. The 'install_profile' variable is already in use.

Also note that SimpleTest did not properly install the installation profile of a test at all until very recently, if the profile was different to the profile of the parent site triggering the test run. #1373634: Installation profile is not installed and not registered as module, unless identical to parent site contained the essential fix to prime the 'install_profile' variable, so that the correct profile is installed in test runs.

Attached patch

  1. verifies that tests in installation profile modules are found.
  2. goes one step further and verifies that tests in an installation profile module can use a different installation profile for running tests (e.g., the testing profile), and modules within the installation profile are still found.

    E.g., a contrib module that happens to be packaged with a non-core installation profile may use the Testing profile for running tests, but still wants to install itself (and is located in the non-core profile, not the testing profile).

  3. goes another step further and verifies that tests of another/different installation profile than the parent site do not appear.

    I.e., modules and tests in profiles/whatever/modules are not supposed to appear when the site is installed with the profiles/awesome profile.

  4. clarifies the code comments for why this special casing is needed.
  5. cleans up class and test names.
  6. fixes various coding style issues.
  7. removes the variable_del() from tearDown(), as it's unnecessary, since the variable is only set in the child site.

StatusFileSize
new7.5 KB
PASSED: [[SimpleTest]]: [MySQL] 34,619 pass(es).
[ View ]

oopsie, fixed the stale comment in SimpleTestOtherInstallationProfileModuleTestsTestCase.

Status:Needs review» Reviewed & tested by the community

Nice tests! I raised an eyebrow at the name SimpleTestInstallationProfileModuleTestsTestCase but if thats how we roll, then OK with me.

Status:Reviewed & tested by the community» Needs work

+++ b/core/includes/common.incundefined
@@ -5314,9 +5314,22 @@ function drupal_system_listing($mask, $directory, $key = 'name', $min_depth = 1)
+  // For SimpleTest to be able to test modules packaged together with a
+  // distribution we need to include the profile of the parent site (in which
+  // test runs are triggered).
+  $testing_profile = variable_get('simpletest_parent_profile', FALSE);
+  if ($testing_profile && $testing_profile != $profile) {
+    $profiles[] = $testing_profile;
+  }
+  // In case both profile directories contain the same extension, the actual
+  // profile always has precedence.
+  $profiles[] = $profile;
+  foreach ($profiles as $profile) {
+    if (file_exists("profiles/$profile/$directory")) {
+      $searchdir[] = "profiles/$profile/$directory";
+    }

Any code that only runs inside a test environment should be inside a check for drupal_valid_test_ua().

Status:Needs work» Needs review
StatusFileSize
new7.54 KB
PASSED: [[SimpleTest]]: [MySQL] 34,195 pass(es).
[ View ]

Status:Needs review» Reviewed & tested by the community

#72 is fixed, bot approves, I'd call this RTBC again.

Version:8.x-dev» 7.x-dev
Status:Reviewed & tested by the community» Patch (to be ported)

Committed/pushed to 8.x, back to 7.x (I think I added the right files, we'll soon find out if not).

Status:Patch (to be ported)» Needs review
StatusFileSize
new7.71 KB
PASSED: [[SimpleTest]]: [MySQL] 37,847 pass(es).
[ View ]

Status:Needs review» Reviewed & tested by the community

That should pass. Thanks!

Status:Reviewed & tested by the community» Needs work

The last submitted patch, drupal7.simpletest-profiles.76.patch, failed testing.

Strange, the failing File API test passes for me fine.

Status:Needs work» Needs review

#76: drupal7.simpletest-profiles.76.patch queued for re-testing.

Status:Needs review» Reviewed & tested by the community

I know we don't RTBC our own patches but sun originally set that status on the patch before the test bot messed up.

Status:Reviewed & tested by the community» Fixed

Committed and pushed to 7.x. Thanks!

Status:Fixed» Closed (fixed)

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

Issue tags:+7.13 release notes

x

Status:Closed (fixed)» Needs review
StatusFileSize
new1.89 KB
PASSED: [[SimpleTest]]: [MySQL] 40,337 pass(es).
[ View ]

Sorry for re-opening, but this appears to not work. Of course could be a regression or I'm doing something wrong with what I'm doing and thus should file/look for a new issue, but has anyone tested this with the 7.x version? No one mentions what they did to test the patch and from what I understand, simpletest in 8.x has diverged (though whether it was that different when this was committed, no idea). The original 7.x patches (pre issue going to 8.x) did not include drupal_valid_test_ua part.

As far as I can tell, during web test case's setUp, drupal_valid_test_ua returns false because while setup is running, a test isn't (the browser agent is still mozilla). So when it goes to enable modules, it can't.

(hm, It'd be possible to create a test for this by adding new install profile with a module with the test in it, and test that module can be enabled... overkill?).

After reading the thread (though only once), is the goal being able to run tests from modules that exist in a custom installation profile? If so, that is currently not possible. But as soon as I move the modules to `sites/all/`, tests are runnable.

I was not able to run tests of my custom module that resided in the sites/all/modules/custom folder on an installation that uses the custom installation profile "recruiter". This profile installs Drupal 7.19 by default.

After applying the patch posted in #85 and testing again, i can confirm that the patch is solving the problem.

Status:Needs review» Reviewed & tested by the community

We are using this (#85) in Open Atrium 2 to support our automated testing on Drupal 7.

Status:Reviewed & tested by the community» Needs work

I think if we can we should provide tests for this, and #85 seems to suggest that would be possible. Also I think instead of introducing a new function I would rather fix drupal_valid_test_ua() if possible in WebTestCase::setUp(). I'm not sure, though, if that is possible.

Hint. If you, for some strange reason, want to run tests for modules in a profile that is not actually your parent profile, you can manually set the originalProfile property in a test like this:

<?php
/**
+   * {@inheritdoc}
+   */
+  protected function prepareEnvironment() {
+   
parent::prepareEnvironment();
+   
// Make it possible to find modules in the profile.
+    $this->originalProfile = 'your_profile_name';
+  }
?>

Assigned:sun» Unassigned
Issue summary:View changes