tests broken by incorrect quotes
pwolanin - April 30, 2008 - 00:38
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | tests |
| Category: | bug report |
| Priority: | normal |
| Assigned: | flobruit |
| Status: | closed |
Description
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)
#1
ok, 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:
user warning: Table 'dp70.profile_fields' doesn't exist query: SELECT f.name, f.type, v.value FROM profile_fields f INNER JOIN profile_values v ON f.fid = v.fid WHERE uid = 1 in /Users/Shared/www/drupal-7/modules/profile/profile.module on line 230.#2
profile.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.
#3
#4
@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.
#5
The 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.
#6
Here'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.
#7
Looks pretty good - remove the trailing
?>This function seems like to be broken:
<?phpfunction deleteProfileField($fid) {
// Delete field.
$this->drupalPost("admin/user/profile/delete/$fid", array(), t('Delete'));
$this->drupalGet("admin/user/profile");
$this->assertNoText($new_title, "Checking deleted field $title");
}
}
?>
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.
#8
Just to confirm this test needs a rewrite. This was attempted, but never completed in http://drupal.org/node/242934.
#9
The remaining issues have been fixed:
- field deletion
- trailing
?>- assertion messages not wrapped with t()
- added test for field weight
#10
Committed to CVS HEAD. Thanks flubruit -- good job.
#11
Trying 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
#12
see also: http://drupal.org/node/254166
#13
This 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.
#14
Refactoring 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.#15
I get the same as pwoladin:
$ ./scripts/run-functional-tests.sh ProfileTestCaseTest cases run: 1/1, Passes: 170, Failures: 276, Exceptions: 0
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 ProfileTestCaseis 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.shso 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/?#16
@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.
#17
You're right. I've added a note to http://drupal.org/node/254166 and committed this CVS HEAD.
#18
It 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).
#19
Back to fixed.
#20
Change component is relation to http://drupal.org/node/253744.
#21
Automatically closed -- issue fixed for two weeks with no activity.