Broken mass-reporting, session storage, node title mapping, watchdog messages, administrative user creation form

Same patch in green, didn't run tests.

Plus another one for D6 -- it absolutely doesn't hurt to do a visual diff between branches ;)

In addition to both patches, we also want to add the sendFeedback method to our fake server and use that for the tests, so we can double-check that we actually send proper data.

However, let's commit those two first.

So, this is what I tried to explain earlier... it should work and the added changes to specifically pass a list of modules to install shouldn't be required, but is currently blocked by #347959: modules_installed is broken during testing

Hmmm... somehow, we didn't commit everything of this patch to D6?

Alrighty -- our own, previously existing tests were broken, so we didn't assert proper session handling at all, and that's the reason why I was able to break all of this.

Test failures in tests:

Comment submission protection

(which is the only one that actually performs mollom session id assertions)

This patch should fix everything, but I now get obscure test failures: When invoking mollom.checkContent() with a session_id, a different session_id is returned.

I added a "Server responses" test, starting with mollom.checkContent(), which proves that the client gets a different session_id from the server. Please tell me what I did wrong (because I hope I'm doing + expecting something wrong)

Posting something in between... it turns out that the @todo I added is actually required to solve this (since we have a lot of other helper functions like postIncorrectCaptcha()).

Summary of what this patch does:

1. Fixes the bug of no Mollom session_id being stored in the database, when a CAPTCHA was successful, but a unrelated form validation error occurred in the form. In this case, $GLOBALS['mollom_response'] was empty, because we do not communicate with Mollom in subsequent requests after passing a CAPTCHA.
2. Fixes usage of non-existing variable and coding style in _mollom_send_feedback().
3. Replaces previously existing assertions that tried to verify that the Mollom session id remains the same across requests with an entirely new Mollom session id assertion logic. The old code was not able to work at all, because it tried to access the form element value using a wrong xpath property (not invented here, #611860: Revamp the test suite). However, the overall revamp is caused by the fact that a Mollom session id can change across requests in the case of a server list refresh or server redirect. That is, because one Mollom server does not share session ids with other servers.

   We therefore need to introduce some more (weird?) logic into the tests:

   • We need to verify that we send, receive, and output the proper Mollom session id.
   • But we cannot rely on a static Mollom session id.
   • We only see what a regular user sees in the browser.
   • But we have some additional information available due to our watchdog message assertions.

   Instead of hacking testing-related code into the regular module code, I chose to solve this challenge by scannning the recorded watchdog messages. Because those watchdog messages already contain the information we need: In case of a server redirect or server list refresh, we have a corresponding watchdog message, and that simply means, we can forget about the last known session id and use the new one for future comparisons.

   @see assertSessionIDInForm()

4. Adds a new test case "Server responses" along with a first low-level test for mollom.checkContent(). We should complete this test case with other methods in a follow-up issue.

   @see assertSessionID()

5. When being redirected to the next server in the list, we now update the stored server list, because it is likely that we will be redirected in subsequent requests, too. (At least, this change [before I introduced the session id handling] made the low-level mollom.checkContent() server response test occasionally pass.)
6. Adds dblog.module to list of modules to install, because it is now required to run tests.

Incorporated all requested changes.

Sorry for that _drupalPost() hickup... I tried to override it first but that failed for some reason... trying it once again works just nice.

Good news! I just ran all tests with this patch on Presssflow, and all passed. :)

Added some potentially escaped characters to the values in MollomDataTestCase->testAnalysis() - however, I cannot replicate the escaping.

mmmh, ok, this is a bit mind-blowing. Two patches:

1) Syncing D6 with code in HEAD. I ran tests with this, and all passed, so committing these changes may be fine.

2) An initial patch to port these changes to HEAD... however, the relatively new requirement to also account for a potentially changing session_id doesn't map at all to the existing module code. That's also why the existing tests had to be rewritten (already committed to D6), because they tried to verify that the session_id remains the same across requests. Now, the entire module logic and form integration is bound to a session_id. If it changes, then we need to cache the $form_state['mollom'] for a new session_id, while the current module code assumes that it won't change.

With this patch for HEAD, I at least see the "Unknown session id" watchdog warning for the first time (when running the Comment form protection tests).

This minor detail of changing session ids has the potential to invalidate a lot of the module's code. Plenty food for thought. :-/

Summary:

• This works in D6, because we write Mollom session information to the cache at the latest point in time (at the end of #pre_render).
• Doesn't work cleanly in HEAD, because we write the to the cache in different locations. Doesn't matter, because we need to #683998: Kill cache_mollom, rely on Form API anyway.
• However, while working on the port to HEAD, I realized that we do not always update the session_id with a new one returned by Mollom servers. AFAICS, we cache and use the session_id returned from textual analysis in case we are unsure and present a CAPTCHA. I'm not 100% sure, but I guess this means that we need to fix this in D6.
• Fixing this, however, may not be very easy. And it also means that we need to clear more than one session_id from the cache after successful form submission (the module code still expects only one, and clears only one session_id).

Reviewing this once more, looking at http://api.drupal.org/api/function/system_cron/6 and mollom_flush_caches(), we are in good shape.

Yes, we store many more cache entries than actually needed and used, but as long as they are properly flushed, I don't think it makes sense to complicate the code even more - especially, since we will remove that entire cache table for D7.