Closed Bug 965656 Opened 11 years ago Closed 10 years ago

Intermittent test_peerConnection_bug834153.html | Test timed out.

Categories

(Core :: WebRTC, defect, P1)

Product:
Core
Component:
WebRTC
Platform:
All
Gonk (Firefox OS)
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla33
Tracking Flags:
Tracking Status
firefox31 --- wontfix
firefox32 --- fixed
firefox33 --- fixed
firefox-esr24 --- unaffected
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: KWierso, Assigned: drno)

References

Details

(Keywords: intermittent-failure)

Attachments

(4 files, 6 obsolete files)

Adding missing close() calls to two tests (without test case)
(deleted), patch
drno
: checkin-
Details | Diff | Splinter Review
wait_for_close_in_tests.patch
(deleted), patch
jesup
: review+
drno
: checkin-
Details | Diff | Splinter Review
bug_965656_close_datachannel.patch
(deleted), patch
cbook
: checkin+
Details | Diff | Splinter Review
bug_965656_add_network_startup.patch
(deleted), patch
jib
: review+
jesup
: review+
Details | Diff | Splinter Review
https://tbpl.mozilla.org/php/getParsedLog.php?id=33777586&tree=Mozilla-Inbound slave: tst-linux64-spot-406 18:10:18 INFO - 17677 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_peerConnection_basicAudio.html | Spec and MapClass variant of RTCStatsReport enumeration agree 18:10:18 INFO - 17678 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_peerConnection_basicAudio.html | Have at least 1 inboundrtp stat(s) * 18:10:18 INFO - 17679 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_peerConnection_basicAudio.html | Have 1 outboundrtp stat(s) 18:10:18 INFO - 17680 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_peerConnection_basicAudio.html | Have localcandidate stat(s) 18:10:18 INFO - 17681 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_peerConnection_basicAudio.html | Have remotecandidate stat(s) 18:10:18 INFO - 17682 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_peerConnection_basicAudio.html | Closing peer connections. Connection state=false 18:10:18 INFO - 17683 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_peerConnection_basicAudio.html | iceConnectionState should not be undefined 18:10:18 INFO - 17684 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_peerConnection_basicAudio.html | PeerConnectionWrapper (pcLocal): oniceconnectionstatechange fired, new state is: closed 18:10:18 INFO - 17685 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_peerConnection_basicAudio.html | PeerConnectionWrapper (pcLocal): Closed connection. 18:10:18 INFO - 17686 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_peerConnection_basicAudio.html | iceConnectionState should not be undefined 18:10:18 INFO - 17687 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_peerConnection_basicAudio.html | PeerConnectionWrapper (pcRemote): oniceconnectionstatechange fired, new state is: closed 18:10:18 INFO - 17688 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_peerConnection_basicAudio.html | PeerConnectionWrapper (pcRemote): Closed connection. 18:17:00 INFO - 17689 INFO TEST-INFO | /tests/dom/media/tests/mochitest/test_peerConnection_basicAudio.html | Test finished 18:17:00 INFO - 17690 INFO TEST-INFO | MEMORY STAT vsize after test: 118722560 18:17:00 INFO - 17691 INFO TEST-INFO | MEMORY STAT residentFast after test: 55169024 18:17:00 INFO - 17692 INFO TEST-INFO | MEMORY STAT heapAllocated after test: 24179420 18:17:00 INFO - 17693 INFO TEST-END | /tests/dom/media/tests/mochitest/test_peerConnection_basicAudio.html | finished in 46874ms 18:17:00 INFO - 17694 INFO TEST-START | /tests/dom/media/tests/mochitest/test_peerConnection_bug822674.html 18:17:00 INFO - 17695 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_peerConnection_bug822674.html | Can set expandos on an RTCPeerConnection 18:17:00 INFO - 17696 INFO TEST-INFO | MEMORY STAT vsize after test: 116690944 18:17:00 INFO - 17697 INFO TEST-INFO | MEMORY STAT residentFast after test: 54067200 18:17:00 INFO - 17698 INFO TEST-INFO | MEMORY STAT heapAllocated after test: 21902256 18:17:00 INFO - 17699 INFO TEST-END | /tests/dom/media/tests/mochitest/test_peerConnection_bug822674.html | finished in 9610ms 18:17:00 INFO - 17700 INFO TEST-START | /tests/dom/media/tests/mochitest/test_peerConnection_bug825703.html 18:17:00 INFO - 17701 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_peerConnection_bug825703.html | mozRTCPeerConnection() succeeds 18:17:00 INFO - 17702 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_peerConnection_bug825703.html | mozRTCPeerConnection(1) throws 18:17:00 INFO - 17703 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_peerConnection_bug825703.html | mozRTCPeerConnection({}) succeeds 18:17:00 INFO - 17704 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_peerConnection_bug825703.html | mozRTCPeerConnection({"iceServers":[]}) succeeds 18:17:00 INFO - 17705 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_peerConnection_bug825703.html | mozRTCPeerConnection({"iceServers":[{"url":""}]}) throws 18:17:00 INFO - 17706 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_peerConnection_bug825703.html | mozRTCPeerConnection({"iceServers":[{"url":"stun:127.0.0.1"},{"url":"stuns:localhost","foo":""},{"url":"turn:[::1]:3478","username":"p","credential":"p"},{"url":"turns:localhost:3478?transport=udp","username":"p","credential":"p"}]}) succeeds 18:17:00 INFO - 17707 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_peerConnection_bug825703.html | mozRTCPeerConnection({"iceServers":[{"url":"turns:localhost:3478","username":"p"}]}) throws 18:17:00 INFO - 17708 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_peerConnection_bug825703.html | mozRTCPeerConnection({"iceServers":[{"url":"turns:localhost:3478","credential":"p"}]}) throws 18:17:00 INFO - 17709 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_peerConnection_bug825703.html | mozRTCPeerConnection({"iceServers":[{"url":"http:0.0.0.0"}]}) throws 18:17:00 INFO - 17710 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_peerConnection_bug825703.html | mozRTCPeerConnection() constructor has readable exceptions 18:17:00 INFO - 17711 INFO TEST-INFO | MEMORY STAT vsize after test: 118788096 18:17:00 INFO - 17712 INFO TEST-INFO | MEMORY STAT residentFast after test: 54284288 18:17:00 INFO - 17713 INFO TEST-INFO | MEMORY STAT heapAllocated after test: 22659964 18:17:00 INFO - 17714 INFO TEST-END | /tests/dom/media/tests/mochitest/test_peerConnection_bug825703.html | finished in 34428ms 18:17:00 INFO - 17715 INFO TEST-START | /tests/dom/media/tests/mochitest/test_peerConnection_bug834153.html 18:17:00 INFO - 17716 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/media/tests/mochitest/test_peerConnection_bug834153.html | Test timed out. 18:17:00 INFO - 17717 INFO TEST-INFO | MEMORY STAT vsize after test: 114528256 18:17:00 INFO - 17718 INFO TEST-INFO | MEMORY STAT residentFast after test: 51224576 18:17:00 INFO - 17719 INFO TEST-INFO | MEMORY STAT heapAllocated after test: 20129452 18:17:00 INFO - 17720 INFO TEST-END | /tests/dom/media/tests/mochitest/test_peerConnection_bug834153.html | finished in 330145ms 18:17:00 INFO - 17721 INFO TEST-START | /tests/dom/media/tests/mochitest/test_peerConnection_bug835370.html 18:17:00 INFO - 17722 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_peerConnection_bug835370.html | createOffer(step1, failed) succeeds 18:17:00 INFO - 17723 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_peerConnection_bug835370.html | createOffer(step1, failed, 1) throws 18:17:00 INFO - 17724 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_peerConnection_bug835370.html | createOffer(step1, failed, {}) succeeds 18:17:26 INFO - 17725 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_peerConnection_bug835370.html | createOffer has readable exceptions 18:17:26 INFO - 17726 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_peerConnection_bug835370.html | createOffer(step1, failed, { mandatory: { FooBar: true } }) throws 18:17:26 INFO - 17727 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_peerConnection_bug835370.html | createOffer(step1, failed, { optional: [] }) succeeds 18:17:26 INFO - 17728 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_peerConnection_bug835370.html | createOffer(step1, failed, { optional: [1] }) throws 18:17:26 INFO - 17729 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_peerConnection_bug835370.html | createOffer(step1, failed, { optional: [{ OfferToReceiveVideo: false, OfferToReceiveAudio: true, }] }) throws 18:17:26 INFO - 17730 INFO TEST-PASS | /tests/dom/media/tests/mochitest/test_peerConnection_bug835370.html | createOffer(step1, failed, { mandatory: { OfferToReceiveVideo: false, OfferToReceiveAudio: true, MozDontOfferDataChannel: true}, optional: [{ VoiceActivityDetection: true }, { FooBar: "42" }] }) succeeds 18:17:26 INFO - 17731 INFO TEST-INFO | MEMORY STAT vsize after test: 114528256 18:17:26 INFO - 17732 INFO TEST-INFO | MEMORY STAT residentFast after test: 52178944 18:17:26 INFO - 17733 INFO TEST-INFO | MEMORY STAT heapAllocated after test: 20931988
Adam, this is currently the #8 top orange on trunk. Can you please look into this?
Flags: needinfo?(adam)
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #103) > Adam, this is currently the #8 top orange on trunk. Can you please look into > this? I'm not going to have any cycles to look at this. I spoke to Maire, and she's going to find someone to assign it to. Lateralling the "needinfo" to her...
Flags: needinfo?(adam) → needinfo?(mreavy)
I'm concerned that the test (at least in #102) according to their logging show overlapping network activity from tests which are finished already. Here is proposed action plan: #1 make sure we call close() on the PC objects in these tests to have them stop network activity. #2 if #1 does not help try to call the garbage collector explicitly at the end of each of these tests in the attempt to shut down the PC objects sooner.
Assignee: nobody → drno
Flags: needinfo?(mreavy)
Attachment #8400871 - Flags: review?(rjesup)
Attachment #8400871 - Flags: review?(rjesup) → review+
Keywords: checkin-needed, leave-open
I removed the test case from the patch which caused the failures. Carrying forward the r+=jesup. Try run: https://tbpl.mozilla.org/?tree=Try&rev=3d028fb15e49
Attachment #8400871 - Attachment is obsolete: true
Keywords: checkin-needed
So obviously action #1 did not helped. But rather then trying to mess with garbage collector my new plan is to get Bug 989936 fixed, which would allow tests to wait for PC's to be closed before moving on. I would hope this should help us with this one here.
(In reply to Nils Ohlmeier [:drno] from comment #140) > So obviously action #1 did not helped. But rather then trying to mess with > garbage collector my new plan is to get Bug 989936 fixed, which would allow > tests to wait for PC's to be closed before moving on. I would hope this > should help us with this one here. This intermittent failure is #7 on OrangeFactor, and as such worthy of escalation due to https://wiki.mozilla.org/Sheriffing/Test_Disabling_Policy Do you have a rough estimate for timeframe for the bug 989936? :-)
Depends on: 989936
Flags: needinfo?(drno)
(In reply to Ed Morley (Away until 6th May) [:edmorley UTC+0] from comment #242) > Do you have a rough estimate for timeframe for the bug 989936? :-) Thanks for the reminder. I'll try to add something within the next 2-3 days.
Flags: needinfo?(drno)
Attached patch wait_for_close_in_tests.patch (deleted) — — Splinter Review
Getting everything closed in all our tests is more complicated then I expected. Lets get this done in several patches. This first patch calls close() on all PC's in the PC specific tests. Try run: https://tbpl.mozilla.org/?tree=Try&rev=ab5777087020
Attachment #8421078 - Flags: review?(rjesup)
Attachment #8421078 - Flags: review?(rjesup) → review+
Attachment #8401456 - Flags: checkin+
Attachment #8421078 - Flags: checkin?
Attachment #8401456 - Flags: checkin+ → checkin-
Attachment #8421078 - Flags: checkin? → checkin+
Not sure if I'm using the checkin+/- flags correctly here: I would like to get the wait_for_close_in_tests.patch checked in. The "Adding missing close() calls to two tests (without test case)" patch is landed already.
Keywords: checkin-needed
Attached patch close_datachannel.patch (obsolete) (deleted) — — Splinter Review
This patch goes through all open data channels and closes any remaining open data channel (if needed from both sides). It also adds an overall timeout in case the closing does not work to prevent a test framework timeout. It also tries to prepare for running the same test case on two separate machines with steeplechase, although this has not been tested.
Attachment #8422892 - Flags: review?(rjesup)
Comment on attachment 8422892 [details] [diff] [review] close_datachannel.patch Review of attachment 8422892 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/tests/mochitest/pc.js @@ +547,5 @@ > self.waitingForRemote = true; > self.pcRemote.close(); > } > + // give the signals handlers time to fire > + setTimeout(verifyClosed, 100); Very concerned about such a "short" timeout when we have to run on b2g emulators, etc. In general, most "wait for event" timeouts should be LONG (within a factor of say 2 or 3 of the overall mochitest timeout time; if they fail taking a long time to fail (and orange) is ok) @@ +822,5 @@ > + _closePeerConnection(); > + } > + else { > + closeTimeout = setTimeout(function() { > + ok(false, "Failed to properly close data channels: " + trailing space @@ +918,5 @@ > + verifyClosedChannels(); > + }; > + > + // we both channels available so wait for the remote even by default > + // apparently this is need for Linux debug builds?!? Rewrite comment; confusing. Is there a bug on Linux debug builds? Bug # should be here if so. If there is a bug but it isn't filed; file and include STR and pointers to Try/etc logs @@ +1834,1 @@ > clearTimeout(self.addStreamTimeout); Yup, that looks like it was a bug!
Attachment #8422892 - Flags: review?(rjesup) → review+
Summary: Intermittent TEST-UNEXPECTED-FAIL | test_peerConnection_bug834153.html | Test timed out. → Intermittent test_peerConnection_bug834153.html | Test timed out.
Target Milestone: --- → mozilla33
Attached patch bug_965656_close_datachannel.patch (deleted) — — Splinter Review
Adressed jesup's concerns. Carrying forward r+=jesup
Attachment #8422892 - Attachment is obsolete: true
Attachment #8432989 - Flags: checkin?
Attachment #8421078 - Flags: checkin+ → checkin-
Requesting checkin of attachment 8432989 [details] [diff] [review]
Keywords: checkin-needed
Attachment #8432989 - Flags: checkin? → checkin+
A completely new idea: Bug 950317 fixed an issue where setLocalDescription would never fire the success callback on B2G Emu, because the network interfaces were not up/ready. Unfortunately the fix in Bug 950317 was placed into the constructor of the PeerConnectionWrapper.js, which is NOT used in this test case!
Attached patch bug_965656_part1_add_network_startup.patch (obsolete) (deleted) — — Splinter Review
This patch moves out the B2G network setup code from pc.js into a new file. Note: only works together with part 2.
Attachment #8435397 - Flags: review?(rjesup)
Attached patch bug_965656_part_2_use_b2g_network_start_everywhere.patch (obsolete) (deleted) — — Splinter Review
This patch applies the new common code in tests which need it. Try run: https://tbpl.mozilla.org/?tree=Try&rev=ffd709e59c6e
Attachment #8435400 - Flags: review?(rjesup)
Attachment #8435400 - Flags: review?(jib)
Attachment #8435397 - Flags: review?(jib)
Comment on attachment 8435397 [details] [diff] [review] bug_965656_part1_add_network_startup.patch Review of attachment 8435397 [details] [diff] [review]: ----------------------------------------------------------------- r+ with nits and passing networkTeardown in to croak(). ::: dom/media/tests/mochitest/b2gNetworkHelper.js @@ +91,5 @@ > + }); > + > + return utils.tearDownNetwork; > + } else { > + return null; Why not return an empty function instead, then you wont have to test for it? ::: dom/media/tests/mochitest/test_peerConnection_bug834153.html @@ +17,5 @@ > function croak(msg) { > ok(0, msg); > pc1.close(); > pc2.close(); > + if (networkTeardown) { networkTeardown is undefined here, so you'll have to pass it in to croak (closures are to declaration environment not calling environment). @@ +39,5 @@ > pc2.createAnswer(function (d) { > is(d.type,"answer","CreateAnswer created an answer"); > pc1.close(); > pc2.close(); > + if (networkTeardown) { This is the test that could be made unnecessary.
Attachment #8435397 - Flags: review?(jib) → review+
Comment on attachment 8435400 [details] [diff] [review] bug_965656_part_2_use_b2g_network_start_everywhere.patch Review of attachment 8435400 [details] [diff] [review]: ----------------------------------------------------------------- Yowsa! That's a lot of files. I'm not sure about this. Involving every test in what's essentially an environment problem is unfortunate, and can lead to bugs by omission. Can this perhaps be encapsulated better? How about: 1. Leave this in pc.js and include pc.js in the 6 remaining places that need this? (They still don't need to use PeerConnectionTest). 2. Add a runNetworkTest() wrapper around runTest() or something that takes care of this at a higher level somehow (for tests that aren't already using PeerConnectionTest). See concerns below. ::: dom/media/tests/mochitest/test_dataChannel_noOffer.html @@ +32,2 @@ > SimpleTest.finish(); > }, generateErrorCallback(), options); Don't you need to call networkTeardown in failure callback as well? This is the kind of bugs I worry about, which makes me think this ideally should be handled transparently a level up if at all possible. ::: dom/media/tests/mochitest/test_peerConnection_errorCallbacks.html @@ +54,5 @@ > info ("Testing addIceCandidate error callback"); > pc.addIceCandidate(new mozRTCIceCandidate({ candidate: "Pony Lords, jump!", > sdpMid: "whee", > sdpMLineIndex: 1 }), > generateErrorCallback("addIceCandidate with nonsense candidate should fail"), Same here. ::: dom/media/tests/mochitest/test_peerConnection_throwInCallbacks.html @@ +82,5 @@ > > function getFail() { > return function (err) { > window.onerror = oldOnError; > generateErrorCallback()(err); And here.
Attachment #8435400 - Flags: review?(jib) → review-
Comment on attachment 8435397 [details] [diff] [review] bug_965656_part1_add_network_startup.patch Review of attachment 8435397 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/tests/mochitest/b2gNetworkHelper.js @@ +25,5 @@ > + var length = itfList.getInterface(i).getAddresses(ips, prefixLengths); > + > + for (var j = 0; j < length; j++) { > + var ip = ips.value[j]; > + // skip IPv6 address Make a comment linking to this in the bug about implementing IPV6 for webrtc
Attachment #8435397 - Flags: review?(rjesup) → review+
Comment on attachment 8435400 [details] [diff] [review] bug_965656_part_2_use_b2g_network_start_everywhere.patch Review of attachment 8435400 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/tests/mochitest/test_peerConnection_bug825703.html @@ +90,5 @@ > pcs = null; > > + if (networkTeardown) { > + networkTeardown(); > + } I presume job's comment applies here too ::: dom/media/tests/mochitest/test_peerConnection_bug835370.html @@ +57,5 @@ > pconnect = null; > pconnects = null; > + if (networkTeardown) { > + networkTeardown(); > + } ditto
Attachment #8435400 - Flags: review?(rjesup)
Attachment #8435397 - Attachment is obsolete: true
Attachment #8435400 - Attachment is obsolete: true
Attachment #8436126 - Flags: review?(rjesup)
Attachment #8436126 - Flags: review?(jib)
Attached patch bug_965656_add_network_startup.patch (obsolete) (deleted) — — Splinter Review
Attachment #8436126 - Attachment is obsolete: true
Attachment #8436126 - Flags: review?(rjesup)
Attachment #8436126 - Flags: review?(jib)
Attachment #8436563 - Flags: review?(rjesup)
Attachment #8436563 - Flags: review?(jib)
Attached patch bug_965656_add_network_startup.patch (deleted) — — Splinter Review
Removed commented code from development cycle.
Attachment #8436563 - Attachment is obsolete: true
Attachment #8436563 - Flags: review?(rjesup)
Attachment #8436563 - Flags: review?(jib)
Attachment #8436566 - Flags: review?(rjesup)
Attachment #8436566 - Flags: review?(jib)
Attachment #8436566 - Flags: review?(rjesup) → review+
Comment on attachment 8436566 [details] [diff] [review] bug_965656_add_network_startup.patch Review of attachment 8436566 [details] [diff] [review]: ----------------------------------------------------------------- r+ but see question. ::: dom/media/tests/mochitest/test_peerConnection_basicAudio.html @@ +4,5 @@ > <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/> > <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script> > <script type="application/javascript" src="head.js"></script> > <script type="application/javascript" src="mediaStreamPlayback.js"></script> > + <script type="application/javascript" src="b2gNetworkHelper.js"></script> Not needed anymore, right? ::: dom/media/tests/mochitest/test_peerConnection_bug834153.html @@ +35,5 @@ > pc2.createAnswer(function (d) { > is(d.type,"answer","CreateAnswer created an answer"); > pc1.close(); > pc2.close(); > + //networkTeardown(); What does this mean? Perhaps add a comment?
Attachment #8436566 - Flags: review?(jib) → review+
(In reply to Jan-Ivar Bruaroey [:jib] from comment #306) > Yowsa! That's a lot of files. > > I'm not sure about this. Involving every test in what's essentially an > environment problem is unfortunate, and can lead to bugs by omission. > > Can this perhaps be encapsulated better? How about: > > 1. Leave this in pc.js and include pc.js in the 6 remaining places that need > this? > (They still don't need to use PeerConnectionTest). > > 2. Add a runNetworkTest() wrapper around runTest() or something that takes > care of > this at a higher level somehow (for tests that aren't already using > PeerConnectionTest). As you saw I actually went ahead and implemented your suggestion of a wrapper around runTest(). BUT as it turns out this is not so easily possible as runTest() itself is not a blocking function AND it currently does not take any success or failure callbacks. Instead each test starts everything with runTest() which is part of head.js, but then calls SimpleTest.finish() in success or error case. While digging into this problem I learned that our current implementation for starting the B2G network is broken (it does not wait for the network initialization to finish, but just proceeds). My current thinking is to change all calls to runTest() into runNetworkTest() to ensure proper network initialization. But the bigger issue is the network teardown. I don't want to change SimpleTest.finish() as it is part of the mochitest framework. So my current thinking is to provide another wrapper around SimpleTest.finish() which handles the network teardown. But again I then need to change all files. 1) Does this (the two wrappers) sound like a feasible plan? 2) Should I do this in hear, or create a new ticket?
Flags: needinfo?(rjesup)
Flags: needinfo?(jib)
Clearing needinfo since I think we covered this in irc. To recap, I suggested that since we can't change SimpleTest.finish, that we could pass your wrapper as 'simpleTest': runNetworkTest(function (simpleTest) { to make it a one-line change at least (and who knows this might be a good idea in general).
Flags: needinfo?(jib)
Priority: -- → P1
Since Bug 1022896 landed on 2014-06-16 this seems to have popped up on branches which don't have the fix from Bug 1022896. If it stays like this I would close this at some point in the future.
Flags: needinfo?(rjesup)
Any reason not to backport bug 1022896 to affected branches?
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #334) > Any reason not to backport bug 1022896 to affected branches? Non, besides the fact that I have never backported any of my tests or fixes yet and therefore I'm not familiar with I have to do :-)
Bug has been uplifted around to most B2G branches now. Let's call this fixed.
Status: NEW → RESOLVED
Closed: 10 years ago
status-b2g-v1.4: --- → fixed
status-b2g-v2.0: --- → fixed
status-b2g-v2.1: --- → fixed
status-firefox31: --- → wontfix
status-firefox32: --- → fixed
status-firefox33: --- → fixed
status-firefox-esr24: --- → unaffected
Keywords: leave-open
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: