Closed Bug 934062 Opened 12 years ago Closed 12 years ago

Intermittent test_policyuri_regression_from_multipolicy.html | [SimpleTest.finish()] No checks actually run. (You need to call ok(), is(), or similar functions at least once. Make sure you use SimpleTest.waitForExplicitFinish() if you need it.)

Categories

(Core :: Security, defect)

Product:
Core
Component:
Security
Platform:
All
Windows 8
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla28
Tracking Flags:
Tracking Status
firefox26 --- fixed
firefox27 --- fixed
firefox28 --- fixed
firefox-esr24 --- unaffected
b2g-v1.2 --- fixed

People

(Reporter: KWierso, Assigned: grobinson)

Details

(Keywords: intermittent-failure)

Attachments

(2 files, 2 obsolete files)

2.patch
1.45 KB, patch
geekboy
: review+
Details | Diff | Splinter Review
2-fix
1.62 KB, patch
grobinson
: review+
Details | Diff | Splinter Review
https://tbpl.mozilla.org/php/getParsedLog.php?id=29995931&tree=Fx-Team slave: t-w864-ix-130 15:26:56 INFO - 4602 INFO TEST-PASS | /tests/content/base/test/csp/test_csp_redirects.html | script-src-redir-spec-compliant test: http://example.com/tests/content/base/test/csp/file_csp_redirects_resource.sjs?res=script&testid=script-src-redir-spec-compliant 15:26:56 INFO - 4603 INFO TEST-PASS | /tests/content/base/test/csp/test_csp_redirects.html | style-src-redir-spec-compliant test: http://example.com/tests/content/base/test/csp/file_csp_redirects_resource.sjs?res=style&testid=style-src-redir-spec-compliant 15:26:56 INFO - 4604 INFO TEST-PASS | /tests/content/base/test/csp/test_csp_redirects.html | worker-redir-spec-compliant test: http://example.com/tests/content/base/test/csp/file_csp_redirects_resource.sjs?res=worker&testid=worker-redir-spec-compliant 15:26:56 INFO - 4605 INFO TEST-PASS | /tests/content/base/test/csp/test_csp_redirects.html | font-src-redir-spec-compliant test: http://example.com/tests/content/base/test/csp/file_csp_redirects_resource.sjs?res=font-spec-compliant&testid=font-src-redir-spec-compliant 15:26:56 INFO - JavaScript error: http://mochi.test:8888/tests/content/base/test/csp/file_csp_redirects_resource.sjs?res=xhr, line 1: : 15:26:56 INFO - JavaScript error: http://mochi.test:8888/tests/content/base/test/csp/file_csp_redirects_resource.sjs?res=xhr-spec-compliant, line 1: : 15:26:56 INFO - 4606 INFO TEST-PASS | /tests/content/base/test/csp/test_csp_redirects.html | img-src-redir test: http://example.com/tests/content/base/test/csp/file_csp_redirects_resource.sjs?res=image&testid=img-src-redir 15:26:56 INFO - 4607 INFO TEST-PASS | /tests/content/base/test/csp/test_csp_redirects.html | xhr-src-redir test: http://example.com/tests/content/base/test/csp/file_csp_redirects_resource.sjs?res=xhr-resp&testid=xhr-src-redir 15:26:56 INFO - 4608 INFO TEST-PASS | /tests/content/base/test/csp/test_csp_redirects.html | xhr-src-redir-spec-compliant test: http://example.com/tests/content/base/test/csp/file_csp_redirects_resource.sjs?res=xhr-resp-spec-compliant&testid=xhr-src-redir-spec-compliant 15:26:56 INFO - 4609 INFO TEST-END | /tests/content/base/test/csp/test_csp_redirects.html | finished in 380ms 15:26:56 INFO - 4610 INFO TEST-START | /tests/content/base/test/csp/test_policyuri_regression_from_multipolicy.html 15:26:56 INFO - 4611 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/csp/test_policyuri_regression_from_multipolicy.html | [SimpleTest.finish()] No checks actually run. (You need to call ok(), is(), or similar functions at least once. Make sure you use SimpleTest.waitForExplicitFinish() if you need it.) 15:26:56 INFO - 4612 INFO TEST-END | /tests/content/base/test/csp/test_policyuri_regression_from_multipolicy.html | finished in 61ms 15:26:56 INFO - 4613 INFO TEST-START | /tests/content/base/test/test_CrossSiteXHR.html 15:26:56 INFO - JavaScript warning: http://mochi.test:8888/tests/content/base/test/test_CrossSiteXHR.html, line 660: yield without a value is deprecated, and illegal in ES6 (use 'yield undefined' instead) 15:26:56 INFO - JavaScript warning: http://mochi.test:8888/tests/content/base/test/test_CrossSiteXHR.html, line 840: yield without a value is deprecated, and illegal in ES6 (use 'yield undefined' instead) 15:26:56 INFO - JavaScript warning: http://mochi.test:8888/tests/content/base/test/test_CrossSiteXHR.html, line 1156: yield without a value is deprecated, and illegal in ES6 (use 'yield undefined' instead) 15:26:56 INFO - 4614 INFO TEST-PASS | /tests/content/base/test/test_CrossSiteXHR.html | shouldn't have failed in test for ({pass:1, method:"GET", noAllowPreflight:1}) 15:26:56 INFO - 4615 INFO TEST-PASS | /tests/content/base/test/test_CrossSiteXHR.html | wrong status in test for ({pass:1, method:"GET", noAllowPreflight:1})
Attached patch 1.patch (obsolete) — — Splinter Review
The initial test tests for the side effects of an inline script executing after the 'load' event has fired. I'm assuming the intermittent failures are caused by a race condition between the load event and the inline script changing the DOM. I've replaced this test with one that uses waitForExplicitFinish and only finishes if the inline script explicitly calls into the parent Mochitest to register its execution; otherwise, in the case of a test failure, it times out. Verified that this patch fails if 924708 is reverted.
Attachment #830387 - Flags: review?(sstamm)
I think there's two problems: 1. waitForExplicitFinish isn't called when all the checks are async 2. if the script is accidentally blocked, the test will time out but not fail immediately. Before and after the patch, if the script is blocked is() will never get called, so this won't fix (2), though I'm not sure we need to fix (2). What if you just add waitForExplicitFinish() before adding the load event listener, then call finish() inside the load listener? Seems to me it should fix (1) with a simpler patch, though will also not fix (2).
Attached patch 2.patch — — Splinter Review
Discussed on IRC, decided this is a simpler way to solve the intermittent oranges.
Attachment #830387 - Attachment is obsolete: true
Attachment #830387 - Flags: review?(sstamm)
Attachment #830444 - Flags: review?(sstamm)
Comment on attachment 830444 [details] [diff] [review] 2.patch Review of attachment 830444 [details] [diff] [review]: ----------------------------------------------------------------- Yeah, that's what I was thinking was simplest to solve (1). Lets see if this works, and if not we can try to solve for (2), though that's a bigger patch.
Attachment #830444 - Flags: review?(sstamm) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/b398d3330001
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch 2-fix.patch (obsolete) — — Splinter Review
I think SimpleTest.waitForExplicitFinish() needs to be called in the global namespace. If it's called in the load handler, as it was before this patch, then it isn't being properly registered and you still have a race condition.
Attachment #8336494 - Flags: review?(sstamm)
Attachment #8336494 - Flags: review?(sstamm) → review+
Attached patch 2-fix — — Splinter Review
Updated commit message. Carrying over r+ from sstamm.
Attachment #8336494 - Attachment is obsolete: true
Attachment #8337043 - Flags: review+
Please checkin "2-fix" to fix the occasional failures that continued after "2.patch" landed.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/30e39643eefc
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
I can't understand why we saw another error after the last patch. Sid, mind taking a look?
Flags: needinfo?(sstamm)
It was on Mozilla-B2g26-v1.2 where the second patch never landed. I'll fix that :)
status-b2g-v1.2: fixed → affected
status-firefox26: fixed → affected
status-firefox27: fixed → affected
Flags: needinfo?(sstamm)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: