Closed Bug 822281 Opened 8 years ago Closed 8 years ago

Intermittent xpcom/tests/unit/test_bug325418.js | 0 == 1

Categories

(Core :: XPCOM, defect)

Product:
Core
Component:
XPCOM
Platform:
x86
Windows XP
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla22

People

(Reporter: emorley, Assigned: bzbarsky)

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

Stop assuming XPCOM timers can't fire early (at least where IEEE doubles are concerned).
2.53 KB, patch
avih
: review+
Details | Diff | Splinter Review 
Rev3 WINNT 5.1 mozilla-inbound debug test xpcshell on 2012-12-15 14:24:13 PST for push d685cc4b4741

slave: talos-r3-xp-087

https://tbpl.mozilla.org/php/getParsedLog.php?id=17982830&tree=Mozilla-Inbound

{
TEST-UNEXPECTED-FAIL | C:\talos-slave\test\build\xpcshell\tests\xpcom\tests\unit\test_bug325418.js | test failed (with xpcshell return code: 0), see following log:
>>>>>>>
### XPCOM_MEM_LEAK_LOG defined -- logging leaks to c:\docume~1\cltbld\locals~1\temp\tmp83ygf1\runxpcshelltests_leaks.log

TEST-INFO | (xpcshell/head.js) | test 1 pending

TEST-INFO | (xpcshell/head.js) | test 2 pending

TEST-INFO | (xpcshell/head.js) | test 2 finished

TEST-INFO | (xpcshell/head.js) | running event loop

TEST-UNEXPECTED-FAIL | C:/talos-slave/test/build/xpcshell/tests/xpcom/tests/unit/test_bug325418.js | 0 == 1 - See following stack:
JS frame :: C:\talos-slave\test\build\xpcshell\head.js :: do_throw :: line 452
JS frame :: C:\talos-slave\test\build\xpcshell\head.js :: _do_check_eq :: line 546
JS frame :: C:\talos-slave\test\build\xpcshell\head.js :: do_check_eq :: line 567
JS frame :: C:/talos-slave/test/build/xpcshell/tests/xpcom/tests/unit/test_bug325418.js :: observeTC2 :: line 39
native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0
}
Any idea why this has started failing every now and then after a year of everything being okay?
Boris, as test reviewer, any ideas? It's failing at:
http://hg.mozilla.org/mozilla-central/file/3825fdbcec62/xpcom/tests/unit/test_bug325418.js#l39
Flags: needinfo?(bzbarsky)
Whiteboard: [disable-me 2013-04-01]
Assignee: nobody → bzbarsky
Whiteboard: [disable-me 2013-04-01] → [need review][disable-me 2013-04-01]
Thank you Boris :-)
Whiteboard: [need review][disable-me 2013-04-01] → [need review]
No problem.  ;)
Comment on attachment 728323 [details] [diff] [review]
Stop assuming XPCOM timers can't fire early (at least where IEEE doubles are concerned).

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

r=me if the infallible part is unrelated and removed.

::: docshell/base/nsIDocShell.idl
@@ +229,5 @@
>  
>    /**
>     * Attribute stating whether or not images should be loaded.
>     */
> +  [infallible] attribute boolean allowImages;

Am I missing how is this relevant, or different than other nearby booleans which were not modified?

::: xpcom/tests/unit/test_bug325418.js
@@ +16,5 @@
>        // Stop timer, so it doesn't repeat (if test runs slowly).
>        timer.cancel();
>  
> +      // Actual delay may not be exact, so convert to seconds and round.
> +      do_check_eq(Math.round((Date.now() - gStartTime1) / 1000),

This works for integral values of gStartTime1/2 as tolerance of +/- 0.5, which would be OK for this test with these values of 5, 1. Might have been nicer as Math.abs(Date.now() - gStartTime1) < 0.5 (or some smaller value), but not required.
(In reply to Avi Halachmi (:avih) from comment #36)
> ...Might have been
> nicer as Math.abs(Date.now() - gStartTime1) < 0.5 (or some smaller value),
> but not required.

Ermm... Math.abs((Date.now() - gStartTime1) - kExpectedDelay1)
> r=me if the infallible part is unrelated and removed.

It's totally unrelated, and shall be removed.  It was just in that tree when I started editing, apparently.  Good catch!

> which would be OK for this test with these values of 5, 1.

Right.  If the timeout values weren't those we'd have to do something smarter, but they are, so...
Avi, did you mean to mark r+ on the patch?
Assuming so, and trying to avoid extra randomorange:

https://hg.mozilla.org/integration/mozilla-inbound/rev/2a09d354053f
Flags: in-testsuite-
Whiteboard: [need review]
Target Milestone: --- → mozilla22
Comment on attachment 728323 [details] [diff] [review]
Stop assuming XPCOM timers can't fire early (at least where IEEE doubles are concerned).

(In reply to Boris Zbarsky (:bz) from comment #40)
> Avi, did you mean to mark r+ on the patch?

Sure. By the time I finished playing with splinter I forgot to change to r+. Guess it's acceptable for a first review, even if a bit messy (/1000!) ;)
Attachment #728323 - Flags: review?(avihpit) → review+
https://hg.mozilla.org/mozilla-central/rev/2a09d354053f
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.