Aggressive watchdog message assertion

Alrighty - to make any sense out of those fails, we also need the $type of each message.

Catch all. :)

Fixed the docs.

Created #652420: Fix errors uncovered using aggressive watchdog assertion to fix the actual failures, so we can focus on this thingy in here.

errr. Fixed the incomplete logic for checking whether {watchdog} exists before trying to query it ;)

Note, this aggressive watchdog logging proposal is not to be confused with #500394: Add new log fetcher, which attempts to allow to retrieve individual log messages.

I'm continuing to fix patch failures in #652420: Fix errors uncovered using aggressive watchdog assertion. Just wanted to mention once again:

This is totally awesome.

One of the failing Locale tests revealed an utterly wrong test. :)

Will this pass?

    // Verify language selection is not present on add article form.
    $this->drupalLogin($web_user);
    $this->drupalGet('node/add/article');
    // Verify language select list is not present.
    $this->assertNoRaw('<select name="language" class="form-select" id="edit-language" >', t('Language select not present on add article form.'));

Sure it will! An "Access denied" page contains no language selector! :-D

Created #653940: Clean-up: Tests do not report all errors to fix SimpleTest's error reporting.

Some code by Damien Tournoud, which might fix those wrong source function identifiers in the test result:

diff --git a/modules/simpletest/drupal_web_test_case.php b/modules/simpletest/drupal_web_test_case.php
index 3ed4983..136922b 100644
--- a/modules/simpletest/drupal_web_test_case.php
+++ b/modules/simpletest/drupal_web_test_case.php
 -185,18 +185,26 @@ abstract class DrupalTestCase {
    *   Array representing the true caller.
    */
   protected function getAssertionCall() {
-    $backtrace = debug_backtrace();
+    $full_backtrace = debug_backtrace();
+    $backtrace = $full_backtrace;
 
     // The first element is the call. The second element is the caller.
     // We skip calls that occurred in one of the methods of our base classes
     // or in an assertion function.
-   while (($caller = $backtrace[1]) &&
+   while (!empty($backtrace[1]) && ($caller = $backtrace[1]) &&
          ((isset($caller['class']) && isset($this->skipClasses[$caller['class']])) ||
            substr($caller['function'], 0, 6) == 'assert')) {
       // We remove that call.
       array_shift($backtrace);
     }
 
+    if (empty($backtrace[1]) || $backtrace[1]['function'] == 'simpletest_script_run_one_test') {
+      // The backtracing failed: we reached the bottom of it. That means that
+      // this assertion occured inside simpletest itself. Do not remove any
+      // call in that case.
+      $backtrace = $full_backtrace;
+    }
+
     return _drupal_get_last_caller($backtrace);
   }

Actually, the nice thing about this patch is that the fails reported in the test results are entirely different to the notices, warnings, and errors that #653940: Clean-up: Tests do not report all errors revealed.

Hence, combining both will make our tests *really* bullet-proof. I really look forward to that.

I'll continue here, when #653940: Clean-up: Tests do not report all errors has landed.

Discussed this some more with smk-ka and carlos8f... we actually think that every request in the internal browser should directly catch any severe messages.

Attached patch implements this as prototype.

So the nice thing about this approach is that you see the actual + real location of the test code that triggered the watchdog message:

access denied: user/3/contact      Watchdog     contact.test    312     ContactPersonalTestCase->testPersonalContactAccess()

So you go in there and you see:

    // Test denied access to the user's own contact form.
    $this->drupalGet('user/' . $this->web_user->uid . '/contact');

However, I can perfectly see how this new approach will trigger uneasy feelings... but OTOH, I can't think of a better location and way to do it.

Of course, the name of the key I added to $options is highly debatable, and of course, the inlined logic in assertResponse() is no longer required. And we likely want to replace the value for the $options key with a string that's passed as $type to assertWatchdogMessages(), so we don't do a catch-all style assertion in case we expect a certain fail.

And this is how it would look like.

I didn't update everything, because I need to know whether we want to go with this approach first.

errr, I really hate my copy + paste mistakes :P

We need an agreement on this approach to move forward here.

Note that this was implemented in a contributed module already, which revealed a bug: $seen_ids cannot be cached statically within the function, because the {watchdog} table is thrown away for each test that is run.

The idea is that we do not expect any severe watchdog messages on any GET/POST request, unless explicitly specified. Therefore, this approach automatically covers any possible scenario and reveals unexpected messages in any request that is performed during testing.