From #345428: drupalLogin uses wrong function to create new user it was pointed out that if no user object is passed to DWTC>drupalLogin(), a new user is automatically created. Currently, that codes does not work since it incorrectly calls DWTC->_drupalCreateRole() instead of DWTC->drupalCreateUser().
For the sake of consistency in tests, my solution is to remove the default user creation from DWTC->drupalLogin(). I did a search for = ?\$this->drupalLogin|\$this->drupalLogin\((NULL)?\)
across all .test files in core and contrib, and only found two core .test results. Both of them never actually use the return value (the created user) from DWTC->drupalLogin(). We don't even have a test to make sure that it works, since it's been using the wrong function this entire time. If this 'feature' is not used by any core or contrib tests, why do we need it? Let's leave it up to the tests to make sure they create the appropriate and correct users.
Comment | File | Size | Author |
---|---|---|---|
#9 | 345838-dwtc-drupal-login-D7.patch | 3.48 KB | Dave Reid |
#1 | 345838-dwtc-drupal-login-D7.patch | 3.48 KB | Dave Reid |
Comments
Comment #1
Dave ReidPatch provided for review.
Comment #2
Damien Tournoud CreditAttribution: Damien Tournoud commentedDave, I agree that
drupalLogin()
should have only one purpose, and should not attempt to create a user. But, this is really a duplicate of #345428: drupalLogin uses wrong function to create new user. I know that the original poster is not really listening, but this issue should be discussed in one place only.Comment #3
Dave ReidWith the current code:
What kind of permissions does $test_user have? Do they have no permissions? This is cause for tests to not perform as desired.
Whereas with:
I specify the exact permissions I need for the test and am guaranteed I know what that user can do.
Both codes will have the same number of assertions performed, but one is easier to understand what is going on.
Comment #4
Dave ReidThanks Damien...fixing cross-post.
Comment #5
Darren OhComment #6
Damien Tournoud CreditAttribution: Damien Tournoud commentedPer agreement from the original poster.
Comment #7
PanchoPer agreement with catch, Dave and Damien: RTBC
Comment #8
keith.smith CreditAttribution: keith.smith commentedThe patch in #1 has a
setup the a clean environment
in a comment.Comment #9
Dave ReidWell, shucks thanks for finding that. That section was just some trailing whitespace that is auto-corrected by my IDE, so I didn't even read over the comment. Thanks!
Comment #10
keith.smith CreditAttribution: keith.smith commentedBack to rtbc after comment was fixed.
Comment #11
Panchocrossposted: No need to remove rtbc status. The miswording was not introduced by this patch, it just wasn't fixed by it.
Comment #12
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks!