Node revisions test fails if admin has a personal timezone offset

moshe, can you try something like this?

Another issue where $GLOBALS/static (which are the very same mechanism in PHP just static has a scope limitation btw.) causes problems. drupalLogin needs to do a full user switching: disable session writes, switch users. Same for drupalLogout (if we have one but i suspect we do).

Thanks for the pointer. Here's an updated patch.

On IRC, moshe suggested to simply load the uid 1 from the tested site. So moved to loading after variable cleaning. Here is an updated patch.

Agreed and then some.

The problem with this is that we might have to pull similar tricks for every global in every module that one wants to test (not just the core modules). This doesn't look like a scalable solution ... a more generic fix is in order, IMO. For example, does it make sense to iterate over every global and to unset it? :P

We really should only run testing from a clean source environment. PHP doesn't allow unloading functions, so we can't really guarantee that except by controlling every aspect of that environment.

What I would do: add a test.php in Drupal root directory, bootstrap a controlled environment from that (like what's done at the top of install.php), and only then create the test environment.

I think this needs a bit more thought and discussion. The proposed patch does not guarantee a clean environment at all.

Damien's patch #5 still applied to CVS HEAD.

here's a clean re-roll, still makes all the tests pass.

won't apply now that session_save_session() is drupal_save_session()

Marked #345979: Node revision tests fail with timezones as a duplicate of this bug, although progress has seemed to have stalled here.

Rerolled for HEAD. If this isn't fixed somehow, I see it coming back to bite us once a lot of contrib modules get testing in.

Better reroll of the same idea.

Thanks Damien, that's much better for documentation and obviously your diff isn't being wonky on you today. :) All tests are a green from me, let's see what testing bot says.

I'm still not 100% on this because it only solves this problem for $user and not for the entire environment.

This is holding up test bot: #348111: Upload.test fails when run from run-tests.sh.

Update session test to reflect changes.

This also correct upload.test failure.

patch..

Confirmed this fixes node revisions and upload tests both logged in when running simpletest, logged out running simpletest, and running tests from command line as well.

As a note the bot will be forced to sit idle until this patch (or comparable solution) makes it in.

I vote for getting this in and opening an issue to discuss a more scalable solution, rather than leave testing bot in a useless state.

I've been running into this bug as well recently, with random test failures people start mistrusting the test framework in general, and I'd rather have our patches tested and coverage increase with some not-perfect stuff in simpletest.module making that possible. Also I'm more interested in tidying up some of the (still, hideously) ugly core tests themselves than simpletest internals.

long term solution: #348455: Provide a scalable way to ensure a clean testing environment for SimpleTest

This is one of those "between a rock and a hard place" issues.

Dries is 110% bang-on that this is simply fixing a symptom of the overall larger problem of "tainting" the test environment. And we really do need to solve this in a clean and proper way.

Unfortunately, when #339929: Move node links into $node->content was committed, it introduced this little line:

+    $this->assertTrue(strpos($teaser, format_plural(2, '1 attachment', '@count attachments')), 'Attachments link found on node teaser.');

And due to the lengthy sleuthing done by boombatower in #348111: Upload.test fails when run from run-tests.sh, we see that this is yet another symptom of this bug. And we can't turn back on the testing bot without it.

When testing bot is disabled, core committing essentially stops, because the alternative is running the entire test suite each patch which takes 20+ minutes, or closing your eyes and committing and hoping you didn't break an edge-case somewhere.

So... for now, I've committed this. :\ And I've asked boombatower to start #348455: Provide a scalable way to ensure a clean testing environment for SimpleTest which is where interested parties should go to fix this in the *proper* way so we don't need any more of these special-case things.

(Dries, please don't hit me. ;) Though if you feel strongly about it, I would be fine with a roll-back, but we probably need a more concrete plan of attack so we can unleash our coders at it.)