This patch does several things:
- Fixes a problem whereby the Mollom tests were not showing up correctly in the SimpleTest web interface. In fixing this, it seemed worthwhile just to....
- Update the Mollom tests to use DrupalWebTestCase rather than DrupalTestCase. SimpleTest for Drupal 6 now supports this, so it seems to make sense to do this now rather than wait for Drupal 7, especially since I will be adding additional tests later and it's easiest to write them all the same way. (I'm not sure if we care that this might cause PHP4 compatibility problems, though?)
- Generally reorganizes and improves the tests (they are now broken up into several test cases rather than all in one).
After this patch, I can confirm that all the Mollom tests pass... yay!
Note, however, the "interesting" fact (which pretty much drove me up a wall and took several hours to solve) that DrupalTestCase and DrupalWebTestCase do not have consistent behavior with regard to the tests passing! When I first wrote this patch, I "broke" about 10-15 tests which had been working fine in DrupalTestCase, so I naturally assumed that my changes had caused some kind of problem. Actually, however, it seems to be a more fundamental issue with the way SimpleTest interacts with a module like Mollom, and in particular the fact that Mollom can make dynamic changes to forms based on the contents of the previous form submission. It seems like drupalPost() must have changed its behavior in this regard between DrupalTestCase and DrupalWebTestCase, thereby leading to some test failures that weren't there before and that only occur when the tests are run in a certain order... ugh! My solution for this (documented in the code) was to just have the SimpleTest browser log out and then log in again when we need to make sure that we are getting a fresh form and not being hung up on a CAPTCHA that was inserted as result of the previous submission... not sure if there was a better way to do it, but it works.
Additionally, a couple of the new test failures seem to have been due to an actual (but minor) bug in Mollom that was not revealed with the previous behavior of drupalPost() but is revealed now... it seems that Mollom somehow makes the "Save" button on comment forms disappear when a CAPTCHA is there, and some of the tests try to click on the Save button, so they fail depending on what order they run in. However, the bug is pretty minor and seems like it would be very difficult to fix and is not the main thing we are trying to test in the place where it came up, so I worked around that in the tests too (and documented it). Perhaps it would be better to intentionally set up a separate test that breaks it, even though we have no good way to fix it right now? I'm not sure...
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | mollom_update_tests_2.patch | 31.49 KB | David_Rothstein |
| mollom_update_tests.patch | 21.63 KB | David_Rothstein |
Comments
Comment #1
dries commentedThanks David. These changes look good, but it would even be better if we could fix some of the "fluffiness" (TODO-items) that were introduced.
Comment #2
David_Rothstein commentedOK, here is a version that fixes the "fluffiness" (hm, I like that word) as well as adding even more improvements:
drupalPost()'s behavior without needing a workaround... basically, I just had to think carefully about the order I tested things in.variable_set()'s with actually visiting the Mollom administration page and submitting the form... that way we test to make sure the admin interface is working as well.Also note that what I identified in my previous post as a minor bug in Mollom is actually more of a feature/limitation in Drupal core... see http://drupal.org/node/272054. Sorry about that ;)
Comment #3
David_Rothstein commentedBy the way, all the tests pass after this patch except for two of them... however, with good reason, they seem to have uncovered a bug! I've reported it at http://drupal.org/node/272059.
Comment #4
dries commentedI've committed these tests but haven't fixed the failing tests yet. Thanks David.
Comment #5
Anonymous (not verified) commentedAutomatically closed -- issue fixed for two weeks with no activity.