Log everything!

To start...

When review.runTests() returns anything other than 100% pass in the final self test stage 10, each of the exceptions and failures needs to be logged in watchdog, and I'd do each as their own entry. I know that might mean copying a lot of stuff that's in simpletest table, but watchdog is where people expect to find errors. Same for when the client is in regular testing mode.

When the tests run over the maximum testing timeout, and the cron kills the test as a failure for timing out, it needs to log that it exceeded the max timeout limit and link to the pifr client config page in watchdog param 5 or whatever it is now. As part of this, the 45 minute limit is kind of short and and increase to the default to 60 minutes might be called for, and it should maybe have (MAX_TIMEOUT x 2) for the self tests if possible unless the max timeout resets for each stage of the self test (I never looked to see).

"When requesting next test, server responded: Invalid server" when a client is not enabled is misleading or at least needs improvement. It should say something closer to "When requesting next test, server responded: Client key disabled by master". It doesn't feel right at WATCHDOG_ERROR level either - maybe WATCHDOG_NOTICE?

it would be good idea to add the debug status to the some of the communication with the server.

This would mean that the server could then pass down the last know good patch for testing and even do things like not update the issue queue with the outcome.

I think that the server should not trust anything coming from a server in debug mode. Maybe even set the client status to in debug mode.

...just saw this and thought i'd rather add to this issue than opening a new one.. problem i'm having is that the test bot is just showing

image-fix-sample-image-link.patch failed - Request re-test Failed: Failed to apply patch. Detailed results

and even on the details page, there is no more verbose information, and the patch applies cleanly against HEAD. So the complete output of the patch program might help grokking what exactly is going on...

I assume you're talking about the original patch on #563382: when editing image style, link to sample image broken (missing file_create_url()), which does not apply cleanly to HEAD, which I can tell without even trying to actually apply it. It references a and b directories, which don't exist in a HEAD checkout.

Yes, that is correct. I had already resubmitted the patch without that, but really think this is something the test bot could handle more graciously (i.e. try patch -p1, too) because basically right now, people who use git diff to create a patch are left in the same situation i was: clueless failure.