Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Follow-up from #512026: Move $form_state storage from cache to new state/key-value system.
As you can see in the added DUBT test there, I need to take special care to restore the uid of the global user because that is, in fact the user of the parent site.
We could either completely unset the global user and require that tests which rely on it's existence add it themself or mock it using a stdClass.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | anon-user-1950684-7.patch | 1.4 KB | Berdir |
| #5 | anon-user-1950684-5.patch | 784 bytes | Berdir |
| #2 | unset-user-1950684-2.patch | 727 bytes | Berdir |
Comments
Comment #1
sunUnset it.
Comment #2
BerdirHm, looks like we are already doing most of that, except unsetting the user.
Let's see what happens if I do this...
Comment #3
Damien Tournoud CreditAttribution: Damien Tournoud commentedConceptually, I think we always have a user. Ideally, it should be part of the standard container and default to the anonymous user. There is very little thing you can actually do without a proper principal to perform access checks against.
So, pending future refactoring, I would vote to set the anonymous user structure in unit tests.
Comment #4
sunSorry, yes. Because Drupal has already bootstrapped when a test is executed, session initialization won't happen again.
Therefore, we need to
$user = drupal_anonymous_user();The suggested location for doing this seems correct to me — for WebTests, the anonymous user will install Drupal and after installation, $this->root_user is logged in. For *UnitTests, the anonymous user will remain unless manually futzed with.
Comment #5
BerdirOk.
Comment #6
sunLook ma! I knew I did this in the past already! ;)
http://drupalcode.org/project/drupal.git/blob/HEAD:/core/modules/simplet...
That can be deleted then.
TestBase::prepareEnvironment()already ensures to disable session saving and backs up the original user/session into$this->originalUser, if any, NULL otherwise.TestBase::tearDown(), however, performs this at the very end:To make that reversion work correctly under all circumstances, we'd have to do:
Alternatively, we can assign
drupal_anonymous_user()to $this->originalUser already.Comment #7
BerdirRemoved that part.
Not sure I understand the last part. If global $user does not exist, why do we need to restore it to a different value afterwards?
Comment #8
sunMayhaps I was mistaken. I thought of re-entrance; i.e., when tearDown() is followed by another setUp(). But I guess you're right in that NULL should stay NULL, if it originally was NULL before.
Thanks!
Comment #9
BerdirNote about missing test coverage here.
The result of this patch is not visible in run-tests.sh because there, we already are the anonymous user. So we can only write a test for it when using the UI as an authenticated user. Which means that to be able to test this change, we would need to write a test that then enables simpletest within the test, and runs a test within a test. We are already doing this but not for a unit test so far.
That is IMHO a bit too complicated for just this but we should probably open a follow-up to make sure that running unit tests within the browser works.
The other thing that we are not yet doing is making sure the session id doesn't change. We do protect the session from being changed but not the session id. In the form_cache issue, I was trying to write a test in the case the session id changes and was not able to do that. http://php.net/session_id supports to set the session id to an explicit value, do we want to do this here as well? Then my test could be a bit less hacky...
Comment #10
catch#7: anon-user-1950684-7.patch queued for re-testing.
Comment #11
alexpottCommitted 8a01600 and pushed to 8.x. Thanks!