Last Comment Bug 736918 - Intermittent "test_nodb_pluschanges.js | false == true"
: Intermittent "test_nodb_pluschanges.js | false == true"
Status: VERIFIED FIXED
[perma-orange on SeaMonkey OSX 10.6]
: intermittent-failure
Product: Firefox
Classification: Client Software
Component: Search (show other bugs)
: Trunk
: x86_64 All
: P2 major (vote)
: Firefox 14
Assigned To: David Teller [:Yoric] (please use "needinfo")
:
:
Mentors:
http://mxr.mozilla.org/mozilla-centra...
Depends on:
Blocks: 438871 jsonSearchSvc
  Show dependency treegraph
 
Reported: 2012-03-18 19:56 PDT by Serge Gautherie (:sgautherie)
Modified: 2012-11-25 19:31 PST (History)
4 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Fix (967 bytes, patch)
2012-03-20 01:20 PDT, David Teller [:Yoric] (please use "needinfo")
gavin.sharp: review+
Details | Diff | Splinter Review

Description Serge Gautherie (:sgautherie) 2012-03-18 19:56:17 PDT
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1332097443.1332100322.29124.gz
OS X 10.6 comm-central-trunk debug test xpcshell on 2012/03/18 12:04:03
{
TEST-UNEXPECTED-FAIL | /builds/slave/test/build/xpcshell/tests/toolkit/components/search/tests/xpcshell/test_nodb_pluschanges.js | test failed (with xpcshell return code: 0), see following log:

TEST-UNEXPECTED-FAIL | /builds/slave/test/build/xpcshell/tests/toolkit/components/search/tests/xpcshell/test_nodb_pluschanges.js | false == true - See following stack:
JS frame :: /builds/slave/test/build/xpcshell/head.js :: do_throw :: line 462
JS frame :: /builds/slave/test/build/xpcshell/head.js :: _do_check_eq :: line 556
JS frame :: /builds/slave/test/build/xpcshell/head.js :: do_check_eq :: line 577
JS frame :: /builds/slave/test/build/xpcshell/head.js :: do_check_true :: line 591
JS frame :: /builds/slave/test/build/xpcshell/tests/toolkit/components/search/tests/xpcshell/test_nodb_pluschanges.js :: <TOP_LEVEL> :: line 59
JS frame :: /builds/slave/test/build/xpcshell/tests/toolkit/components/search/tests/xpcshell/head_search.js :: <TOP_LEVEL> :: line 96
JS frame :: resource:///components/nsSearchService.js :: <TOP_LEVEL> :: line 3744
JS frame :: resource://gre/modules/NetUtil.jsm :: <TOP_LEVEL> :: line 119
}
Comment 1 David Teller [:Yoric] (please use "needinfo") 2012-03-19 03:08:21 PDT
Investigating. Does this happen only on MacOS X? I see that this is a perma-orange, what does it mean exactly?
Comment 2 neil@parkwaycc.co.uk 2012-03-19 08:33:41 PDT
(In reply to David Rajchenbach Teller from comment #1)
> Investigating. Does this happen only on MacOS X? I see that this is a
> perma-orange, what does it mean exactly?
On Firefox, most tests are green, and a couple are random-orange, which means that they don't usually fail and even when they do we don't know why yet. Any patch that causes tests to fail consistently gets backed out pretty quickly.

Unfortunately core test authors either don't realise that SeaMonkey is going to try to run their test as well and/or don't care whether it fails, even if it is consistent, which we call perma-orange. Sometimes the test relies on differences between Firefox and SeaMonkey, but sometimes the test is just buggy.
Comment 3 David Teller [:Yoric] (please use "needinfo") 2012-03-19 08:35:22 PDT
I have the impression that my test relies on an event that is sent only once on FF but twice on SM. First time the event is received, the test succeeds and cleans up after itself, and the second time, which is unexpected, it fails.

I am currently trytesting a fix.
Comment 4 Serge Gautherie (:sgautherie) 2012-03-19 10:53:15 PDT
(In reply to David Rajchenbach Teller [:Yoric] from comment #1)
> Investigating. Does this happen only on MacOS X? I see that this is a
> perma-orange, what does it mean exactly?

Yes, OSX 10.6 only, afaict.
perma means it happens at every run.

*****

Fwiw, first time I notice it on OSX 10.5: (ftb, I assume it's random there.)

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1332140061.1332143030.6826.gz
OS X 10.5 comm-central-trunk debug test xpcshell on 2012/03/18 23:54:21
s: cb-sea-miniosx02
Comment 5 Treeherder Robot 2012-03-19 11:55:56 PDT
mbrubeck
https://tbpl.mozilla.org/php/getParsedLog.php?id=10174218&tree=Mozilla-Inbound
Rev3 Fedora 12x64 mozilla-inbound opt test xpcshell on 2012-03-19 01:29:28

TEST-UNEXPECTED-FAIL | /home/cltbld/talos-slave/test/build/xpcshell/tests/toolkit/components/search/tests/xpcshell/test_nodb_pluschanges.js | test failed (with xpcshell return code: 0), see following log:
TEST-UNEXPECTED-FAIL | /home/cltbld/talos-slave/test/build/xpcshell/tests/toolkit/components/search/tests/xpcshell/test_nodb_pluschanges.js | false == true - See following stack:
Comment 6 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-03-19 19:18:34 PDT
Hrm, does this test never remove it's observer at all? That seems bad...
Comment 7 David Teller [:Yoric] (please use "needinfo") 2012-03-20 01:20:13 PDT
Created attachment 607479 [details] [diff] [review]
Fix

Attaching a candidate fix. At the moment, I have not succeeded at getting a SeaMonkey testing environment (even through TryServer) so I unfortunately cannot test it against SeaMonkey, but this one-liner does not break FF and should solve the issue.
Comment 8 Serge Gautherie (:sgautherie) 2012-03-20 02:37:22 PDT
Comment on attachment 607479 [details] [diff] [review]
Fix

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

::: toolkit/components/search/tests/xpcshell/head_search.js
@@ +92,5 @@
>  function afterCommit(callback)
>  {
>    let obs = function(result, topic, verb) {
>      if (verb == "write-metadata-to-disk-complete") {
> +      Services.obs.removeObserver(obs, topic);

Nit: "browser-search-service" would make it easier to match them, etc.
Comment 9 Serge Gautherie (:sgautherie) 2012-03-20 02:44:02 PDT
(In reply to Gavin Sharp (use gavin@gavinsharp.com for email) from comment #6)
> Hrm, does this test never remove it's observer at all? That seems bad...

Should not be needed in xpcshell, should it?


(In reply to David Rajchenbach Teller [:Yoric] from comment #7)
> At the moment, I have not succeeded at getting a
> SeaMonkey testing environment (even through TryServer) so I unfortunately

SM xpcshell: one can build or download, but no Try (yet).

> cannot test it against SeaMonkey, but this one-liner does not break FF and
> should solve the issue.

More than not break FF, the question is "will it fix the random oranges"?
Comment 10 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-03-20 08:18:07 PDT
(In reply to Serge Gautherie (:sgautherie) from comment #9)
> Should not be needed in xpcshell, should it?

Right, I forgot these were xpcshell.
Comment 11 David Teller [:Yoric] (please use "needinfo") 2012-03-20 08:28:40 PDT
(In reply to Gavin Sharp (use gavin@gavinsharp.com for email) from comment #10)
> (In reply to Serge Gautherie (:sgautherie) from comment #9)
> > Should not be needed in xpcshell, should it?
> 
> Right, I forgot these were xpcshell.

Indeed. Here, the issue is that I have to send "quit-application" to instruct the search engine to flush of search metadata. If I diagnosed the issue properly, SM and FF do not seem to have the same behavior here, and "quit-application" is sent a second time on SM. Regardless, removing the listener after we have received the first occurrence of our event seems the right thing to do, and I hope that it solves the issue.
Comment 12 Ryan VanderMeulen [:RyanVM] 2012-03-20 14:33:54 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/25460df0838f

Please don't checkin-needed patches with Try syntax in them in the future.
Comment 13 David Teller [:Yoric] (please use "needinfo") 2012-03-20 16:41:10 PDT
My apologies. I feel a little stupid.
Comment 14 Mounir Lamouri (:mounir) 2012-03-21 03:40:11 PDT
https://hg.mozilla.org/mozilla-central/rev/25460df0838f
Comment 15 Serge Gautherie (:sgautherie) 2012-03-21 22:40:49 PDT
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1332387115.1332390072.14578.gz
OS X 10.6 comm-central-trunk debug test xpcshell on 2012/03/21 20:31:55

V.Fixed

Note You need to log in before you can comment on or make changes to this bug.