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:
- Install D7 using the standard install profile
- Create a modules directory at
profiles/standard/modules
- Move one of the core modules (e.g. poll) from
modules/poll
toprofiles/standard/modules/poll
and then clear your caches. - 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:
- Views is in
profiles/[name]/modules/views
, - When Views' tests are run, the install profile used is
standard
, leading to the following paths scanned bydrupal_system_listing()
:profiles/standard/modules
sites/all/modules
sites/[x]/modules
- 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,
- 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. - 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 lightertesting
profile. - Adjust
drupal_system_listing()
to use not onlydrupal_get_profile()
but the 'parent' install profile (the install profile of the site that the test is running on).
Comment | File | Size | Author |
---|---|---|---|
#85 | 911354-drupal-profile-85.patch | 1.89 KB | hefox |
#76 | drupal7.simpletest-profiles.76.patch | 7.71 KB | langworthy |
#73 | drupal8.simpletest-profiles.73.patch | 7.54 KB | sun |
#70 | drupal8.simpletest-profiles.70.patch | 7.5 KB | sun |
#69 | drupal8.simpletest-profiles.69.patch | 7.72 KB | sun |
Comments
Comment #1
alex_b CreditAttribution: alex_b commentedThis is the culprit:
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.
Comment #2
alex_b CreditAttribution: alex_b commentedi 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...
Comment #3
boombatower CreditAttribution: boombatower commentedAfter 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.
Comment #4
boombatower CreditAttribution: boombatower commentedor
Comment #5
boombatower CreditAttribution: boombatower commentedMy luck, commit as I roll.
Comment #6
yhahn CreditAttribution: yhahn commentedDiscussed 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 fromsites/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()
.Comment #7
adrian CreditAttribution: adrian commentedinstead 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 :
Other useful places to configure additional paths include settings.php and sites.php.
Comment #8
adrian CreditAttribution: adrian commentedhere'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.
Comment #9
adrian CreditAttribution: adrian commentedtest bot hates me ?
renamed to .patch.
Comment #10
boombatower CreditAttribution: boombatower commentedWe 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.
Comment #11
alex_b CreditAttribution: alex_b commentedI'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.
Comment #12
boombatower CreditAttribution: boombatower commentedIncluding 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?
Comment #13
adrian CreditAttribution: adrian commentedJust add --working-copy to it.
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.
Comment #14
adrian CreditAttribution: adrian commentedWe 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.
Comment #15
adrian CreditAttribution: adrian commentedhere'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.
Comment #16
adrian CreditAttribution: adrian commentedImprove 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.
Comment #17
adrian CreditAttribution: adrian commentedHere 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 :
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.
Comment #19
adrian CreditAttribution: adrian commentedFix 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.
Comment #20
febbraro CreditAttribution: febbraro commentedsubscribe
Comment #21
arianek CreditAttribution: arianek commentedsubscribe + 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.
Comment #22
langworthy CreditAttribution: langworthy commentedI'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?
Comment #23
febbraro CreditAttribution: febbraro commentedMy 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
Comment #24
jgraham CreditAttribution: jgraham commentedHuge 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
Comment #25
jhedstromsubscribing
Comment #26
eaton CreditAttribution: eaton commentedIn 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.
Comment #27
chrisshattuck CreditAttribution: chrisshattuck commentedI'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:
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?
Comment #28
dixon_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:
So huge +1 for this. Maybe bump the priority of this issue?
Comment #29
Jackinloadup CreditAttribution: Jackinloadup commentedDoes this need to be in before beta or 7.0 release or can it be implemented in a point release?
Comment #30
eaton CreditAttribution: eaton commentedWhile 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.
Comment #31
sunThis 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.
Comment #32
Boris Mann CreditAttribution: Boris Mann commentedWhat 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...)
Comment #33
bonobo CreditAttribution: bonobo commentedRE: "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.
Comment #34
webchickTo 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.
Comment #35
chx CreditAttribution: chx commentedWe 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.
Comment #36
yhahn CreditAttribution: yhahn commentedtests 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 inprofiles/[x]
.Comment #37
Anonymous (not verified) CreditAttribution: Anonymous commentedsubscribe.
Comment #38
chx CreditAttribution: chx commentedI 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?
Comment #39
yhahn CreditAttribution: yhahn commentedWe're discussing on irc now to clear this up.
Comment #40
Anonymous (not verified) CreditAttribution: Anonymous commentedwhich channel? or did i miss it in #drupal-contribute already?
Comment #41
chx CreditAttribution: chx commentedSo 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.
Comment #42
webchickThere. That looks more like it. ;)
Thanks for working that out, guys!
Comment #43
yhahn CreditAttribution: yhahn commentedAs mentioned by chx, this patch takes a minimal approach to addressing this problem.
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.DrupalWebTestCase
to set the parent profile during->setUp()
and delete it during->tearDown()
.Tested successfully with tests in
profiles/[x]
that use thetesting
andstandard
profiles.Comment #44
dixon_I successfully tested yhahn patch from #43.
Comment #45
chx CreditAttribution: chx commentedIt 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)
Comment #46
dixon_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.Comment #47
yhahn CreditAttribution: yhahn commentedThanks @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, thetesting
profile. Maybe there is another way that doesn't spam the profiles with test modules?Comment #48
David_Rothstein CreditAttribution: David_Rothstein commentedIt 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 :)
Comment #49
EvanDonovan CreditAttribution: EvanDonovan commentedIs 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....
Comment #50
dasjosubscribing
Comment #51
ademarco CreditAttribution: ademarco commentedsubscribe
Comment #52
EvanDonovan CreditAttribution: EvanDonovan commented#46: 911354.46.patch queued for re-testing.
Comment #53
jason.fisher CreditAttribution: jason.fisher commentedStalled?
Comment #54
sunComment #55
langworthy CreditAttribution: langworthy commentedThe 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?
Comment #56
jhedstromThis 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.
Comment #58
jhedstromHere'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.
Comment #59
langworthy CreditAttribution: langworthy commentedThe 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.
Comment #60
langworthy CreditAttribution: langworthy commentedre-roll of #58
Comment #62
jhedstromLooks like the testbot couldn't apply the patch in #60 for some reason. Here's another reroll.
Comment #63
langworthy CreditAttribution: langworthy commentedMy bad. It looks to me now that #58 doesn't need a re-roll when applying the patch using
git apply
withdrupal.git
.For some reason the patch won't apply for me when using
drush make
. But #60 does.Comment #64
tstoecklerMinor: if we do
if($profile)
, we might as well return FALSE as default value in variable_get()That's just a liiiittle bit too long. :)
:(.... :)
Is that actually a standard or are there just a lot too many spaces there?
Comment #65
moshe weitzman CreditAttribution: moshe weitzman commentedCould 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.Comment #66
jrbeeman@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.
Comment #67
moshe weitzman CreditAttribution: moshe weitzman commented#62: 911354-62-simpletest-profiles.patch queued for re-testing.
Comment #69
sunAs 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
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).
I.e., modules and tests in profiles/whatever/modules are not supposed to appear when the site is installed with the profiles/awesome profile.
Comment #70
sunoopsie, fixed the stale comment in SimpleTestOtherInstallationProfileModuleTestsTestCase.
Comment #71
moshe weitzman CreditAttribution: moshe weitzman commentedNice tests! I raised an eyebrow at the name SimpleTestInstallationProfileModuleTestsTestCase but if thats how we roll, then OK with me.
Comment #72
catchAny code that only runs inside a test environment should be inside a check for drupal_valid_test_ua().
Comment #73
sunSorry for missing that.
And for extra kicks: #1411114: drupal_valid_test_ua() is not statically cached on non-positive match
Comment #74
amateescu CreditAttribution: amateescu commented#72 is fixed, bot approves, I'd call this RTBC again.
Comment #75
catchCommitted/pushed to 8.x, back to 7.x (I think I added the right files, we'll soon find out if not).
Comment #76
langworthy CreditAttribution: langworthy commentedComment #77
sunThat should pass. Thanks!
Comment #79
langworthy CreditAttribution: langworthy commentedStrange, the failing File API test passes for me fine.
Comment #80
langworthy CreditAttribution: langworthy commented#76: drupal7.simpletest-profiles.76.patch queued for re-testing.
Comment #81
langworthy CreditAttribution: langworthy commentedI know we don't RTBC our own patches but sun originally set that status on the patch before the test bot messed up.
Comment #82
webchickCommitted and pushed to 7.x. Thanks!
Comment #84
webchickx
Comment #85
hefox CreditAttribution: hefox commentedSorry 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?).
Comment #86
favrik CreditAttribution: favrik commentedAfter 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.
Comment #87
s_leu CreditAttribution: s_leu commentedI 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.
Comment #88
mpotter CreditAttribution: mpotter commentedWe are using this (#85) in Open Atrium 2 to support our automated testing on Drupal 7.
Comment #89
tstoecklerI 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.
Comment #90
BerdirHint. 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:
Comment #91
sunComment #92
mitokens CreditAttribution: mitokens as a volunteer commentedComment #93
wizonesolutionsFound the same thing as @hefox:
drupal_valid_test_ua()
is alwaysFALSE
duringDrupalWebTestCase::setUp()
. Maybe a simple$_SESSION['simpletest_setup_active']
being set toTRUE
and checked for indrupal_valid_test_ua()
would suffice?This must be broken in Drupal 8, too?
Comment #94
MiSc CreditAttribution: MiSc at Wunder commentedI think this is still an issue in Drupal 8 - if you test a module inside your own profile, you must add:
If you don't do that, tests will fail. Tests should be able to run, without adding that.
Or am I doing it the wrong way?
Comment #103
andrewbelcher CreditAttribution: andrewbelcher at FreelyGive commentedI've just hit this in 8.4.6. Adding
$this->originalProfile
solved the module dependency issue, but it looks likecontainer.namespaces
is still not being set correctly, causing discovery to fail (in my case an entity type not being picked up)...Is there any suggestion on a solution to this, if someone can suggest a direction I can have a quick go at fixing it. Using the install profile for the test adds a huge amount of complexity to the form I'm trying to test (via alters in other modules in the profile) that I really don't think that's very workable for me...
Comment #104
andrewbelcher CreditAttribution: andrewbelcher at FreelyGive commentedOk, with a bit of debugging, it looks like while
$this->originalProfile
adds a line to the test'ssettings.php
which solves the module dependency on install, when it then rebuilds the data (\Drupal\Core\DrupalKernel::moduleData
), it uses$this->getConfigStorage()
, which loads the active config excluding overrides, which means the setting insettings.php
is ignored and it can no longer find the module, so the module is then excluded from the list (and so not included incontainer.namespaces
on rebuild).So I think the fix is that
\Drupal\Core\DrupalKernel::moduleData
needs to read from the config factory (to include overrides), unless there is a specific reason to use the active un-overridden config here?As for a work around, tests can do the following:
Comment #110
quietone CreditAttribution: quietone as a volunteer commentedTriaging issues in simpletest.module as part of the Bug Smash Initiative.
This looks like it belongs in the Simpletest project.