Closed
Bug 996031
Opened 9 years ago
Closed 9 years ago
crashtest layout/generic/crashtests/455407.html connects to mozilla.org everytime
Categories
(Core :: Layout, defect)
Tracking
()
People
(Reporter: mcmanus, Assigned: froydnj)
References
Details
Attachments
(1 file, 2 obsolete files)
1.87 KB,
patch
|
dholbert
:
review+
|
Details | Diff | Splinter Review |
The test infrastrucutre is meant to be self contained - so xpcshell, mochitests, and crashtests need to be self hosted. crashtest layout/generic/crashtests/455407.html connects to mozilla.org everytime should be fixed to be localhost only
![]() |
Assignee | |
Comment 1•9 years ago
|
||
This patch should fix the mozilla.org issue, though I think other local changes are interfering with testing locally, so I'm going to wait until try results come through before asking for review.
![]() |
Assignee | |
Comment 2•9 years ago
|
||
Figured out what the local testing issue was. And since reftests/crashtests don't run proxies, we can't just connect to example.com; we need to have something else instead. A data URI seems appropriate.
Attachment #8407083 -
Attachment is obsolete: true
Attachment #8407092 -
Flags: review?(dholbert)
Comment 3•9 years ago
|
||
So, this is considerably simplifying the testcase (by making the inner iframe load a blank page instead of mozilla.org). This may be fine, but it's possible that this change prevents the testcase from triggering the original bug, rendering it useless. Could you test the current & modified testcase against an affected build? (Looks like a nightly from 2008-09-08 should be able to test it, base on bug 455407 comment 0. Nightlies that old may not run on recent mac/linux, but I'd expect them to run on Windows.) Hypothetically, if the current test crashes in affected builds & your modified version does not crash, then we should not take this patch. We'd probably need a patch with a more complex data URI (more similar to mozilla.org in some relevant way), or failing that, a test-disabling patch.
Comment 4•9 years ago
|
||
(It's also possible that the current test no longer crashes in affected builds, due to changes on mozilla.org. If that's the case, then it may be better to just drop the test, since then it wouldn't actually be functioning as a unit test for its original bug anymore.)
![]() |
Assignee | |
Comment 5•9 years ago
|
||
(In reply to Daniel Holbert [:dholbert] from comment #3) > Could you test the current & modified testcase against an affected build? > (Looks like a nightly from 2008-09-08 should be able to test it, base on > bug 455407 comment 0. Nightlies that old may not run on recent mac/linux, > but I'd expect them to run on Windows.) I grabbed a nightly from 2008-09-07: http://ftp.mozilla.org/pub/firefox-old-nightlies/2008/09/2008-09-07-06-mozilla1.9.0/ and ran the testcase with and without the patch. No issues in either case. (Though it seems poor form to have a test that just loops until timeout...) Also tried: http://ftp.mozilla.org/pub/firefox-old-nightlies/2008/09/2008-09-08-03-mozilla1.8/ nightly on Windows, no problems there either. Perhaps we should just delete the test?
Flags: needinfo?(dholbert)
Comment 6•9 years ago
|
||
OK. In that case, it sounds like the testcase was unintentionally depending on some piece of www.mozilla.org to reproduce the issue, and www.mozilla.org has now changed. Given that, there's probably no way to make the test useful again, aside from maaaybe some trial-and-error with archive.org. So, I'd support removing it, but I'll CC mw22 [test author] in case he has any additional information / objections / suggestions. (Also: given that bug 455407 was resolved WORKSFORME & fixed by an unknown patch, it's possible that its issue is also being tested by tests that landed with the unknown patch.)
Flags: needinfo?(dholbert) → needinfo?(Camw22)
Comment 7•9 years ago
|
||
[er, sorry - needinfo-autocomplete-fail. Meant to tap mwargers].
Flags: needinfo?(Camw22) → needinfo?(martijn.martijn)
Comment 8•9 years ago
|
||
I'm fine with removing the test. Indeed, the http://www.mozilla.org reference was probably necessary for some reason to trigger the bug and apparently, I was not able to minimize the testcase without using that external reference.
Flags: needinfo?(martijn.martijn)
![]() |
Assignee | |
Comment 9•9 years ago
|
||
OK, removing it is much easier!
Attachment #8407092 -
Attachment is obsolete: true
Attachment #8407092 -
Flags: review?(dholbert)
Attachment #8407710 -
Flags: review?(dholbert)
Updated•9 years ago
|
Attachment #8407710 -
Flags: review?(dholbert) → review+
![]() |
Assignee | |
Comment 10•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d8efaa0f46d5
Flags: in-testsuite-
![]() |
Assignee | |
Updated•9 years ago
|
Assignee: nobody → nfroyd
Comment 11•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d8efaa0f46d5
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Comment 12•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/65aecee8b069 https://hg.mozilla.org/releases/mozilla-beta/rev/6431641fb1b6 https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/782e03676817 https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/19765ef79eba https://hg.mozilla.org/releases/mozilla-esr24/rev/fa9f5d048e52
status-b2g-v1.2:
--- → fixed
status-b2g-v1.3:
--- → fixed
status-b2g-v1.4:
--- → fixed
status-b2g-v2.0:
--- → fixed
status-firefox29:
--- → fixed
status-firefox30:
--- → fixed
status-firefox31:
--- → fixed
status-firefox-esr24:
--- → fixed
Updated•9 years ago
|
status-b2g-v1.3T:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•