Closed Bug 823120 Opened 12 years ago Closed 12 years ago

Intermittent test_bug802557.html | uncaught exception - ReferenceError: go is not defined, | Test timed out.

Categories

(Core :: XPConnect, defect)

Product:
Core
Component:
XPConnect
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla20

People

(Reporter: philor, Assigned: bholley)

References

Details

(Keywords: intermittent-failure)

Attachments

(2 files)

Fix test. v1
7.37 KB, patch
philor
: review+
Details | Diff | Splinter Review
Fix templates. v1
3.31 KB, patch
jmaher
: review+
Details | Diff | Splinter Review
If the mochitest template didn't encourage foolish behavior, and you were instead thinking about how you write a web page, you would have automatically put the functions inside the <head>, or at least before you called them, but it does, so you have the iframe's onload racing with the script interpreter processing go(), and sometimes the onload wins the race. To minimize the effects on blame, at this point most people seem to like to move the <iframe> down below, rather than moving the JS clear up to the <head> where it naturally belongs. https://tbpl.mozilla.org/php/getParsedLog.php?id=18094487&tree=Mozilla-Aurora Rev3 WINNT 5.1 mozilla-aurora pgo test mochitest-4 on 2012-12-19 07:55:10 PST for push e1070c719210 slave: talos-r3-xp-089 15554 INFO TEST-START | /tests/js/xpconnect/tests/mochitest/test_bug802557.html 15555 ERROR TEST-UNEXPECTED-FAIL | /tests/js/xpconnect/tests/mochitest/test_bug802557.html | uncaught exception - ReferenceError: go is not defined at http://mochi.test:8888/tests/js/xpconnect/tests/mochitest/test_bug802557.html:1 15556 ERROR TEST-UNEXPECTED-FAIL | /tests/js/xpconnect/tests/mochitest/test_bug802557.html | Test timed out.
Attached patch Fix test. v1 — — Splinter Review
<script> moves up to head, and the call to document.getElementById moves into go().
Attachment #693952 - Flags: review?(philringnalda)
Having <script> tags after the content, rather than before, encourages people to do <iframe onload="go()"> before go is guaranteed to be defined.
Attachment #693953 - Flags: review?(jmaher)
Comment on attachment 693952 [details] [diff] [review] Fix test. v1 lgtm, as long as it runs (I almost always miss something like the getElementById the first time I try to fix one of these).
Attachment #693952 - Flags: review?(philringnalda) → review+
Comment on attachment 693953 [details] [diff] [review] Fix templates. v1 Review of attachment 693953 [details] [diff] [review]: ----------------------------------------------------------------- I am not sure who uses these templates anymore, but these now look a lot more like a unit test instead of a mixed up test.
Attachment #693953 - Flags: review?(jmaher) → review+
(In reply to Joel Maher (:jmaher) from comment #4) > I am not sure who uses these templates anymore, but these now look a lot > more like a unit test instead of a mixed up test. Ideally people should be using them to make new mochitests, rather than copy-pasting from existing ones (which can have nasty side-effects).
Keywords: checkin-needed
What would be interesting is a grep of the tree to see how many follow this broken pattern...
Blocks: 810924
No longer depends on: 810924
https://hg.mozilla.org/mozilla-central/rev/61c1733d72df https://hg.mozilla.org/mozilla-central/rev/2d2612425e73
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
This template change actually makes writing mochitests that just want to poke at the DOM and don't need to wait for onload _much_ more annoying. :(
(In reply to Boris Zbarsky (:bz) from comment #9) > This template change actually makes writing mochitests that just want to > poke at the DOM and don't need to wait for onload _much_ more annoying. :( Ok. Should we back it out? Any other ideas?
I would somewhat prefer that, but I'm pretty careful about how I use the templates.... I can also copy/paste the <script> inside the resulting file, which is what I just did when writing a test....
philor, ed - any opinions?
Looks like we started using the iframe onload before script pattern in 2008 (bug 446346) knowing right off what we'd done, regressed down to having no idea for a couple hundred failures by 2010 (bug 556698), and now as long as we leave the failure unstarred and unfiled until I see it, we mostly know right off again. Dunno - I can imagine a template that would work for everything without ever having to type a script tag or an onload attribute or function foo(){}, by including a skeleton of every single thing we ever use, but I can't imagine how to make it not be ridiculously slower than it needs to be.
No longer blocks: 810924
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: