Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
in modules/profile/profile.test
$fid = db_result(db_query('SELECT fid FROM {profile_fields} WHERE title = "%s"', $title));
results in failed tests with errors like:
Unexpected PHP error [Unknown column 'single_simpletest_MODMgjmufz' in 'where clause' query: SELECT fid FROM simpletest488007profile_fields WHERE title = "single_simpletest_MODMgjmufz"] severity [E_USER_WARNING] in [/includes/database.mysqli.inc line 130]
How in the world did this not get noticed before?! These tests don't work at all! Do other tests have this too?
PHP 5.2.4, MySQL 5.0.45, Darwin 10.4.11 (intel)
Comment | File | Size | Author |
---|---|---|---|
#14 | separate-profile-tests-252920-14.patch | 9.75 KB | floretan |
#13 | separate-profile-tests-252920-13.patch | 8.63 KB | pwolanin |
#9 | profile.test.patch | 47.1 KB | floretan |
#6 | profile.test.patch | 45.08 KB | floretan |
#1 | profile-test-quotes-252920-1.patch | 20.91 KB | pwolanin |
Comments
Comment #1
pwolanin CreditAttribution: pwolanin commentedok, plus the redundant code is really annoying me.
here's an attempt to refactor to avoid redundancy and fix the errors.
Tests seems to pass, but this error makes me think all is not well:
Comment #2
floretan CreditAttribution: floretan commentedprofile.test is a rare example of very diverse programming methodologies which have almost disappeared nowadays, such as:
There's probably more fun stuff to be discovered here. I started fixing some of the code, but I'm almost wondering if it wouldn't make more sense to rewrite this test from scratch.
To reply to the original issue, I think profile.test needs a serious clean-up before we tackle the issues with those queries.
edit: it seems like I cross posted with the previous comment.
Comment #3
floretan CreditAttribution: floretan commentedComment #4
pwolanin CreditAttribution: pwolanin commented@flobruit - if you want to undertake a major rewrite (which is clearly needed) please do. However, advise then on whether you're going to wait for: http://drupal.org/node/73874 and/or whether the current patch is of value.
Comment #5
floretan CreditAttribution: floretan commentedThe test currently breaks when applying the patch from #73874: Normalize permissions table because users and roles are created manually in the tests by referencing the permissions table directly. Once that is fixed, the two issues should not have any direct dependencies.
Comment #6
floretan CreditAttribution: floretan commentedHere's a patch for a totally refactored profile.test (200 lines instead of 1000 lines of code) that tests the same functionality as the original profile.test.
There are still some improvements that can be done, but this is at least a solution that lets us move forward with #73874: Normalize permissions table.
Comment #7
pwolanin CreditAttribution: pwolanin commentedLooks pretty good - remove the trailing
?>
This function seems like to be broken:
Since neither $title or $new_title are set. However, it doesn't seem that the function is ever even called - so there seems to be no checking for field deletion.
Comment #8
boombatower CreditAttribution: boombatower commentedJust to confirm this test needs a rewrite. This was attempted, but never completed in http://drupal.org/node/242934.
Comment #9
floretan CreditAttribution: floretan commentedThe remaining issues have been fixed:
- field deletion
- trailing
?>
- assertion messages not wrapped with t()
- added test for field weight
Comment #10
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks flubruit -- good job.
Comment #11
pwolanin CreditAttribution: pwolanin commentedTrying to run this from the browser, it times out or dies before completing. When I run it via CLI:
php scripts/run-functional-tests.sh ProfileTestCase
I get:
FAILURES!!!
Test cases run: 1/1, Passes: 170, Failures: 276, Exceptions: 0
Comment #12
pwolanin CreditAttribution: pwolanin commentedsee also: http://drupal.org/node/254166
Comment #13
pwolanin CreditAttribution: pwolanin commentedThis patch mostly just moves code around so that we have a common class with extra methods but no tests, and then a bunch classes with one test each.
At the least I can run these one-by-one without a PHP timeout. I tested all of them, and they work from the browser.
Comment #14
floretan CreditAttribution: floretan commentedRefactoring this test into multiple classes is definitely good.
The test has a lot of overhead due to the too frequent switch between admin_user and normal_user. Moving the calls to
$this->drulaLogin()
to the appropriate place reduces the number of assertions by 50%, and the execution time by about as much. If needed, access check test should probably be integrated with profile field visibility tests.Comment #15
Dries CreditAttribution: Dries commentedI get the same as pwoladin:
While this patch improves browser based testing, it degrades CLI based testing. The problem with splitting up the tests in multiple smaller classes is that it becomes tedious to run them from the command-line.
User story: I make some changes to profile.module and I want to run all profile module's test cases.
run-functional-tests.sh ProfileTestCase
is guessable, butrun-functional-tests.sh ProfileTestCase ProfileTestFields ProfileTestSelect ProfileTestDate ProfileTestWeights
. Even if it is guessable, it is a PITA to type.Maybe the solution is to update
run-functional-tests.sh
so we can dorun-functional-tests.sh profile
(i.e.run-functional-tests.sh $module_name
)? But how would that work for the XML-RPC tests in includes/?Comment #16
pwolanin CreditAttribution: pwolanin commented@Dries - given that the CLI tests are not working, I'd indeed rather see a way to just specify a module name. Having to specify the class names is a little odd anyhow.
And most modules (user, etc) already have several tests.
Comment #17
Dries CreditAttribution: Dries commentedYou're right. I've added a note to http://drupal.org/node/254166 and committed this CVS HEAD.
Comment #18
pwolanin CreditAttribution: pwolanin commentedIt looks like the tests report themselves as belonging to a group (not a module), so it should be possible to specify the group name
So, it should be feasible to run all the tests belonging to a certain group (which usually is a single module).
Comment #19
catchBack to fixed.
Comment #20
boombatower CreditAttribution: boombatower commentedChange component is relation to http://drupal.org/node/253744.
Comment #21
Anonymous (not verified) CreditAttribution: Anonymous commentedAutomatically closed -- issue fixed for two weeks with no activity.