Problem/Motivation

Usually, file_public_path is set to sites/example.com/files. We chose to change this to a different path.

On simpletest results pages, the 'verbose' links point to the wrong place, and give a 404, if you're using a non-standard file_public_path. This was caused by a regression in #1213536: Non-resettable theme_get_registry() cache causes problems for non-interactive installations.

Proposed resolution

These are the offending lines in DrupalTestCase::setUp():

$conf = array();
drupal_static_reset();

Since the reset occurs almost immediately before calls to variable_get(), these variable_get() calls are not retrieving the desired variables (since they've just been reset) and are using the default values by mistake.

In our case we just moved these lines beneath the calls to variable_get(), but I appreciate this might be a mere part of a larger underlying issue, so this "quick fix" might not be suitable for long-term use.

Remaining tasks

Please review the suggested fix and see if it's appropriate, or file a patch containing a more suitable fix.

Comments

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Active » Closed (duplicate)

The suggested fix is exactly and literally what #1541958: Split setUp() into specific sub-methods is doing.

I linked to that issue and patch at least 3 times.

chriscohen’s picture

chriscohen commented

Please create a new issue to explain and link to it here.

That is precisely what you suggested and what I did.

You should go a little easier on new people who make the effort to report and fix issues in core. There have been many complaints that core is a clique and it's very difficult to get into. A little more time explaining that you did not want a new issue because the other issue dealt with it would have gone a long way to avoiding this duplicate, and make people like me feel more like they're actually doing something to help, rather than not bothering.

Bottom line is that it just means I won't suggest a fix next time, and that could be the time you could actually use it.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

First, I'm not representing Drupal core in any way. I'm a regular contributor like you are.

Second, I intentionally linked to aforementioned issue, because it fixes a whole range of issues. I recommended to create a separate issue if that patch doesn't fix the particular custom public files path issue already. Seeing your suggested solution here, which is exactly what aforementioned patch is doing, meant that you didn't even bother to look into the issue I linked to, even though I precisely stated that it fixes most of the regressions.

Third, if, for some reason, #1541958: Split setUp() into specific sub-methods does not fix the particular bug/regression being outlined here (which you can test already before it gets committed), then feel free to re-open this issue.

Fourth, however, please also note that changing the public files directory for a test can have many unexpected consequences. The run-tests.sh script as well as drupal.org's testing infrastructure partially relies on the environment being set up for tests. But of course that's irrelevant if the custom files directory change is performed in a custom test that's only executed by non-drupal.org test runners.