test_dynUnsecureRedirect.html intermittently fails, change timeouts to callbacks where possible

RESOLVED FIXED

Status

()

Core
Security: PSM
RESOLVED FIXED
9 years ago
5 years ago

People

(Reporter: mayhemer, Assigned: mayhemer)

Tracking

({intermittent-failure})

Trunk
intermittent-failure
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

9 years ago
Even bug 480713 has been fixed, we are still getting mixed content detection failures. I am enable to reproduce locally, so it will take time to figure out what's going on here.

For now this test has been disabled.
(Assignee)

Updated

9 years ago
Blocks: 487632
(Assignee)

Comment 1

9 years ago
Created attachment 372231 [details] [diff] [review]
v1

It more looks like a test is wrong in this case. I am using timeout instead of onload. This is a fix and where is no other way prolong of timeout.
Attachment #372231 - Flags: review?(kaie)
(Assignee)

Comment 2

9 years ago
Created attachment 372235 [details] [diff] [review]
v1

The previous patch was not built on tip... sorry for bloat.
Attachment #372231 - Attachment is obsolete: true
Attachment #372235 - Flags: review?(kaie)
Attachment #372231 - Flags: review?(kaie)
(Assignee)

Updated

9 years ago
Duplicate of this bug: 487632
(Assignee)

Comment 4

9 years ago
Comment on attachment 372235 [details] [diff] [review]
v1

Bob, could you please take a look? Just test changes.. Thanks.
Attachment #372235 - Flags: review?(kaie) → review?(rrelyea)

Comment 5

9 years ago
Comment on attachment 372235 [details] [diff] [review]
v1

Changing review of PSM java and html to Kai.

bob
Attachment #372235 - Flags: review?(rrelyea) → review?(kaie)
(Assignee)

Updated

8 years ago
Whiteboard: [orange]
(Assignee)

Updated

8 years ago
Attachment #372235 - Flags: review?(kaie) → review?(ted.mielczarek)
Comment on attachment 372235 [details] [diff] [review]
v1

I like the img onload change, but bumping the other timer values feels like it's not addressing problems, just trying to hide them more. In general, we should avoid using setTimeout in tests, as it's a known cause of flaky behavior. Is there some other notification your test can wait for to ensure that whatever it's waiting for is ready?
(Assignee)

Comment 7

8 years ago
Agree.

- test_bug329869.html might wait for onload of the new script element
- test_bug472986.html might wait for onload of the img element
- I have no idea what to do for test_cssContent2.html, it adds content with URL and I don't know about any callback for this, but that test has never randomly failed
- test_dynDelayedUnsecurePicture.html might wait for onload of the img
- No idea for test_dynUnsecureBackground.html... is there a callback for background image load? also this test has never failed
- test_dynUnsecurePicture.html might use img onload
- test_dynUnsecureRedirect.html as well
- test_innerHtmlDelayedUnsecurePicture.html might inject the onload code to innerHTML
- test_innerHtmlUnsecurePicture.html as well
- test_unsecureIframeMetaRedirect.html might probably use onload of the iframe
Status: NEW → ASSIGNED
Comment on attachment 372235 [details] [diff] [review]
v1

Ok. If you want to make those changes, feel free to do so without further review. You can push the test_dynDelayedUnsecurePicture.html  change, since that looks good.
Attachment #372235 - Flags: review?(ted.mielczarek) → review-
(Assignee)

Updated

8 years ago
Summary: test_dynUnsecureRedirect.html intermittently fails, security UI issue → test_dynUnsecureRedirect.html intermittently fails, change timeouts to callbacks where possible

Updated

8 years ago
Blocks: 438871
(Assignee)

Comment 9

8 years ago
Created attachment 466871 [details] [diff] [review]
v2 - mainly fixing bug 553326 [Check-in comment 11]

Fixing just the 4 failing tests from bug 553326 for now to see the effect.

After landing the new redirect API (my strongest suspect) we got over the edge of 500ms timeout to get the security UI updated.  All the failing tests are simply using timeout of 500ms to check for sec UI changes expecting the image to load in that time.

I changed the code to poll every 100ms for the state change, but not longer then 20 seconds.
Attachment #372235 - Attachment is obsolete: true
Attachment #466871 - Flags: review?
(Assignee)

Updated

8 years ago
Blocks: 553326

Comment 10

8 years ago
Comment on attachment 466871 [details] [diff] [review]
v2 - mainly fixing bug 553326 [Check-in comment 11]

r=kaie
Attachment #466871 - Flags: review? → review+
(Assignee)

Comment 11

8 years ago
Comment on attachment 466871 [details] [diff] [review]
v2 - mainly fixing bug 553326 [Check-in comment 11]

http://hg.mozilla.org/mozilla-central/rev/ea40235da826

So far, fixing just 4 mostly failing tests.  Remaining test will be patched later after we check this helped.
Attachment #466871 - Attachment description: v2 - mainly fixing bug 553326 → v2 - mainly fixing bug 553326 [Check-in comment 11]
(Assignee)

Updated

7 years ago
Duplicate of this bug: 553326
No longer blocks: 553326

Updated

5 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Keywords: intermittent-failure
Whiteboard: [orange]
You need to log in before you can comment on or make changes to this bug.