Posted by dmitrig01 on June 30, 2008 at 5:48am
| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | user.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | Needs tests |
Issue Summary
This information is from the code coverage report (see http://coverage.cwgordon.com/coverage).
We need to test:
1) Loading users from various services (hook_info + hook_auth)
2) Changing a user's roles
3) Deleting a blocked user's session
4) Submitting a username with an invalid character
5) User searching
6) All the user blocks
7) Updating users from the old to the new hashing algorithm
8) Permissions, roles, multiple roles, etc.
9) Signatures
10) All user actions.
Comments
#1
I will do my hardest best to complete this test within the next two weeks.
#2
Is this issue still active? The above URL gives me an error. By the way, if it is, it would still need some testing.
#3
The user role and permissions tests are being added in #300993: User roles and permissions API.
#4
From the list in the initial issue:
4) Submitting a username with an invalid character
Coverage seems to have been started, but all illegal characters do not seem to be tested. It only seems that the forward slash is tested for.
#5
9) Signatures in the issue list above:
There appears to be no tests for existence or validity of signature in user.test
A test might include:
1) enabling of signatures
2) creation of new user
3) creation of valid and invalid signatures for user
4) enable comment module if not so
5) create comment to check that signature appears
6) remove comment and created user
#6
Missing "Who's New" block test. (#6 on the list above)
#7
No user search test committed yet. (#5 on the list above)
#8
Test missing for creating a new role (for example: "test" user role), and assigning user(s) to that role. Also, multiple role needs to be accounted for.
It would be expected that the following would be in the test:
- Create new user.
- Create new role(s).
- Apply core permission(s) to the new role.
- ...
- revoke role from user
- remove role from site
#9
Albeit really needed, this is not critical.
#10
Tagging.
#11
subscribe
#12
the attached patch has tests for the "who's new" block and for signatures...
There is one weirdness on signatures: The test I'm using works (user with signature can leave a comment and the comment will have the correct sig), but for some (inexplicable) reason I cannot seem to verify that the user has properly created a signature when filling out the profile form. To see what I'm referring to, please see the @todo in this patch. Un commenting that chunk *should* work, but it does not.
Any ideas on that? Once I get past that roadblock I will try to make a few more user tests
#13
ooops ... forgot patch
#14
The last submitted patch, user_test.patch, failed testing.
#15
obviously this will fail testing ... I still need someone to review though
#16
Ignore comment #15 .. clearly I was drunk (aka meant to post that on another "Tests" issue I was working on)
looking at this error
#17
Turns out that this test found an error when using signatures: #699594: Comments throw error when user has a signature
Once that issue is fixed, the test in #13 *should* be working
#18
This patch adds one more small test for signatures ... again, #699594: Comments throw error when user has a signature needs to be fixed before this patch will pass
#19
The last submitted patch, user-test.patch, failed testing.
#20
Ok ... patch in #18 is waiting for #520760: SA-CORE-2009-007 user signature format to land before testbot will be happy with it
#21
sub
#22
Added a SimpleTest that creates a user and role from the api using user_save and user_role save, as well as removing the role with user_role_delete. As far as I understand what's supposed to be happening, the code in the test should pass, but two of the tests fail. These tests relate to the Role ID being returned from the database not returning as an int, as expected from user_role_delete. Additional eyes are welcome.
#23
Had to modify the patch as it had git-styled dates and was messing up the patching process. When I run this test it passes all.
#24
subscribing
#25
passed all tests
#26
The last submitted patch, 276582-additionalusertests-v2.patch, failed testing.
#27
#23: 276582-additionalusertests-v2.patch queued for re-testing.
#28
All 924 user tests passed for 276582-additionalusertests-v2.patch.
#29
Hm. I can't get this to apply cleanly. Can someone give it a re-roll?
#30
I could apply it, problem possible caused by the /no newline at the end of the file.
Anyway: reroll
#31
#32
+++ modules/user/user.test 23 Apr 2010 08:43:03 -0000@@ -1585,3 +1585,67 @@
+ ¶
white space issues
+++ modules/user/user.test 23 Apr 2010 08:43:03 -0000@@ -1585,3 +1585,67 @@
+ $this->assertFalse(user_access('administer nodes', $myTestUser), t('User does not have permission to administer content.')); ¶
more white space
+++ modules/user/user.test 23 Apr 2010 08:43:03 -0000@@ -1585,3 +1585,67 @@
+ $perm = db_query('SELECT rid FROM {role_permission} WHERE rid = :rid AND permission = :permission',
I'm sure this is just something that I'm unfamiliar with, but why are we using ":" here and not "%"?
95 critical left. Go review some!
#33
The ':uid' markup is what's in the database abstraction layer documentation.
http://drupal.org/node/310072
I understand that's how it was in D6, and if % signs are the correct way to go, I'll modify the code.
#34
dont change it ... I just hadnt seen that syntax before. Based on the link you posted, it certainly seems correct. Still have white space issues thought ;)
#35
Details, details... ;-)
Patch takes care of the white space issues in the new test. I'd like to run it through code review, but it screams when I try to enable in on HEAD.
#36
here testbot ... here boy
#37
bump, easy review for somebody...
#38
Looked over the patch at comment #35. The patch logic seems fine to me and test passes. The patch line numbers did not match up for me but was able to put in the changes. Although the variable names are camel case when I think they should be lower case with underscores. See the Drupal coding standard:
http://drupal.org/coding-standards#naming
This is a little confusing since objects are camel case in SimpleTest. Should this be changed? Looks like other test are using lower case with underscores for variable names.
#39
Stumbled upon this old issue. AFAICS, all the tests listed in the original post can be found in user.test in D7. I assume the work was done and committed somewhere else, so I set this issue fixed.
#40
Automatically closed -- issue fixed for 2 weeks with no activity.