Regression: Poll tests don't pass
boombatower - June 28, 2008 - 22:01
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | tests |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | closed |
Description
The poll tests no longer pass. :(
Looks like something got committed without testing again. :(
27 passes, 7 fails, 0 exceptions
Choice 0 bar is themed Other poll.test 94
Choice 1 bar is themed Other poll.test 94
Choice 2 bar is themed Other poll.test 94
Choice 3 bar is themed Other poll.test 94
Choice 4 bar is themed Other poll.test 94
Choice 5 bar is themed Other poll.test 94
Choice 6 bar is themed Other poll.test 94In IRC chx suggested theme registry.
#1
I can reproduce, failing tests are critical.
#2
I did a revert of the patch on http://drupal.org/node/275092 and the tests pass. Marking postponed against that.
#3
I disabled theme developer and it passed.
Theme Developer changes the output of theme(), and poll.test compares the local output (where it's enabled) with the CURL response (where it evidently is not).
#4
Nope, it's broken without theme developer too, and not just the test.
#5
Whoops. It must be admitted that the fingerprinting addition DID, indirectly, trigger the bug. Theme developer does not do its stuff before.
I conjecture that the problem is that of calling theme() early in the page load (for the HTTP header) before something else happens - whatever that something else is.
#6
I closed the header issue again - running poll test by itself passes both before and after that patch. Whoops from me too.
So the issue seems to be with running all tests. Almost certainly looks like a SimpleTest then (if a sleeper that's been exposed due to core changes).
Theming of the poll bars fails on firefox, works in IE7, that may be a different issue entirely.
#7
#8
This bug is beginning to look very odd! My own experience so far:
- with Generator patch, with devel_themer: fail
- with Generator patch, without themer: pass
- without patch, with themer: pass
- without either: pass
And in all those cases I was testing only the Poll Creation test case...
#9
Running just the Poll test on HEAD without devel I get "Poll create" 67 passes, 7 fails, 0 exceptions
#10
For debugging purposes, I recommend the following which has helped me a lot:
<?php// in poll.test
$fp = fopen('test.txt', 'a');
fwrite($fp, $themed_bar); // We're looking for this text
fwrite($fp, $this->_content); // Inside this page.
fclose($fp);
?>
#11
With the following you don't need to have the file pointer so you can run this code from any file.
If you omit the FILE_APPEND parameter the first time it is called that would clear the file, creating a log-like affect.
<?phpfile_put_contents('test.txt', $data, FILE_APPEND);
?>
#12
chx alerted me that without devel, the poll test also fails, and a poll is in fact not themed correctly. I've confirmed both of these, but the latter was a caching problem on my computer. For some reason, poll.css was not being added to the page.
#13
Solved the issue. Problem was poll module wasn't be loaded even though it was enabled, to confirm this just enable poll module and see that this passes, although no one wants to *actually enable poll module* in real life, so no one knew that. This patch solves the problem by adding a module_load_all() to DrupalWebTestCase::setUp(). Someone needs to run all tests to make sure this didn't break anything, otherwise everything should be fine. Also contains two small coding standard changes in the relevant part of poll.test. Poll tests pass with this patch.
#14
As mikey_p pointed out, the above patch did not work. This patch does, problem is mainly that theme() statically caches the hook implementations, with no method of refreshment possible. This removes the assertion altogether, with a TODO note to re-add the assertion once theme() gets around its static caching problem(s).
#15
Poll module: 67 passes, 0 fails, 0 exceptions
#16
Well, the test was wrong in comparing the result of theme() to what's shown on the page. This test makes no sense: (1) it is pointless, (2) you should never use internal API in an UI test. So #14 is the way to go, but I would also drop the comment.
#17
Re-rolled minus the comment, agree with Damien.
#18
How about a patch?
#19
Looks good to me, but I can't put this in RTBC (that's a patch originating from my own review). catch, do you mind?
#20
OK. Tested the re-roll, poll tests pass, logic is improved, back to RTBC then.
#21
Committed to CVS HEAD. Thanks.
#22
Automatically closed -- issue fixed for two weeks with no activity.