1091077 - gloda test timeout: TEST-UNEXPECTED-TIMEOUT | C:/slave/test/build/tests/xpcshell/tests/mailnews/db/gloda/test/unit/test_cleanup_msf_databases.js | Test timed out

Status: RESOLVED FIXED

(MailNews Core :: Database, defect)

Description

[Kent James (:rkent)]

Actual list of failing tests is long, up to 30 tests.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

I am guessing there are several reasons of timed out, but the most important thing is:

let throwingAppender = new Log4Moz.ThrowingAppender(do_throw);

in mailnews/db/gloda/test/unit/resources/glodaTestHelper.js.

After bug 1033126, do_throw invokes _testLogger.error(), so it causes infinite recursive calls. We need to use another *throw* function without any log output instead of do_throw().

Comment 2

[Hiroyuki Ikezoe (:hiro)]

OK. The regression caused by bug 1033125 is more complicated. Even if this time out is solved, no errors report any more.

Comment 3

[Hiroyuki Ikezoe (:hiro)]

Attachment: Fix for timed out

I found the reason of the timed out error. The timed out is caused by the collision of "_testLogger".

But there are still lots of failures...

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Attachment: Fix timed out and other errors

This fixes timed out and other errors in gloda tests, but I don't run other tests yet.

Comment 6

[Hiroyuki Ikezoe (:hiro)]

Attachment: Fix

Try server results seem fine:
https://treeherder.mozilla.org/ui/#/jobs?repo=try-comm-central&revision=e3000e2a60fe
All failures are in calender or mozilla-central. They are not related to this issue at all.

This patch is slightly different from the try one:
1. indentation fix
2. a message to do_report_result in glodaTestHelper.js

Comment 7

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 8514969 [details] [diff] [review]
Fix

Review of attachment 8514969 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.  I confirmed that bug 1033126 eliminated _passedChecks and added _testLogger, making everything in here look quite sane.  Just one nit.  Note that I did remove myself as an owner of the gloda submodule and peer of mailnews, so I'm not technically a reviewer for this code anymore.  However, you're also unlikely to find someone as familiar with this code, so... if you can get someone on IRC to just rubber-stamp review deferral to me or something.

::: mailnews/db/gloda/test/unit/resources/glodaTestHelper.js
@@ +894,5 @@
>        do_throw("Query should have returned " + key + " (" + value + ")");
>      }
>  
> +    // notify xpcshell that we did something
> +    let msg = "query satisfied with:";

The loop below doesn't include whitespace.  It might be easier to just do:
let msg = "query satisfied with: " + aCollection.items;

aCollection.items is an array so its toString() method will be called which will rougly do aCollection.items.join(', ') anyways.

Comment 8

[Hiroyuki Ikezoe (:hiro)]

Attachment: Fix a nit

Carrying over review+.
Thanks asuth.

Comment 9

[Kent James (:rkent)]

Checked in http://hg.mozilla.org/comm-central/rev/29fb511ba387