Open
Bug 1020436
Opened 10 years ago
Updated 2 years ago
WebRTC mochitests orchestrate two participants, but share one test/event queue
Categories
(Core :: WebRTC, defect, P4)
Core
WebRTC
Tracking
()
NEW
backlog | webrtc/webaudio+ |
People
(Reporter: drno, Unassigned)
References
Details
(Whiteboard: [webrtc-mochitest])
As of now the majority of the data channel and peer connection mochitests use the command chain from template.js and pc.js to describe the list of actions and expectations. But even though the tests are orchestrating two instances, pcLocal and pcRemote, all of the actions and expectations are sharing just one queue/chain.
Especially if one element in the chain is waiting for network events to finish before proceeding (because sub-sequent steps depend on the network to be ready) and the same step needs to happen on both ends, the step for the second party usually does no longer verifies if the network events coming in, but merely that the expected status has already been reached.
To support steeplechase (which relies on filtering the chain into two separate chains for both parties) we can not have steps in the chain which does things in parallel on both sides of a call.
It would be ideal if we could split the one chain into two separate chains, which can be processed in parallel. Even with the limits of Javascript we could at least get in non steeplechase cases both sides into the waiting state. And which side receives its network events earlier proceeds first.
That would also help in making sure that our tests are properly written so they do not depend on the second party.
Reporter | ||
Comment 1•10 years ago
|
||
Here is an idea: replace the long sequential test steps in template.js with Promises and use their all() and race() functions to implement AND's and OR's.
Comment 2•10 years ago
|
||
I'm not a big fan of that because it makes it hard to insert new stuff into the chain.
Comment 3•10 years ago
|
||
See attachment 8492552 [details] for an example that orchestrates two participants in parallel using Promise.all.
We should be able to use promises behind the scenes without altering the replaceable nature of the queue.
Reporter | ||
Comment 4•10 years ago
|
||
Now we have everything converted to Promises. But I think all the steps in the chain are still executed sequentially, so ICE connection (as one example) still gets establish on pcLocal first and then the chain executes the next step for pcRemote and just returns immediately because the ICE connection has been established already, or?
Martin does the new structure allow us to let two steps of the chain run in parallel (if the function names start with PC_LOCAL and PC_REMOTE)? Or could we filter the one chain a test start into two separate chains - that should at least be possible with tests which work under steeplechase?
Flags: needinfo?(martin.thomson)
Whiteboard: [webrtc-mochitest]
Comment 5•10 years ago
|
||
I think the cleanest approach would be to have two chains to begin with, e.g. a local and remote chain to use the current nomenclature. We could then prune away *_LOCAL* and *_REMOTE* from all names, or leave them for logging.
Comment 6•10 years ago
|
||
The new structure would require some significant changes to work. But as Jan-Ivar says, two separate chains makes a great deal of sense. The signaling portion used by steeplechase would need to be factored out so that part of it could be used in local testing.
Note that any benefit is probably only structural. I don't think that there is any performance advantage to this - we don't actually hold any of the async calls back. If the intent is to be able to run more of the tests on steeplechase (at least in theory) the exercise might be worthwhile. If the intent is to try to make the tests finish quicker, I think that we could spend our time more productively on certificate generation.
Flags: needinfo?(martin.thomson)
Reporter | ||
Comment 7•10 years ago
|
||
The intent is more to get our tests to closer to reality execution. As bug 1087551 shows we are still have strange obstacles in our tests.
Instead of factoring out the steeplechase signaling we should have a common interface for sending messages between PC objects. In case of steeplechase that interface really relays signaling messages over an external signaling server. In case of TBPL it would probably just use the global state/memory of the PeerConnectionTest as a relay and pretend to the receiver side that a signaling event was received.
Reporter | ||
Updated•10 years ago
|
Reporter | ||
Comment 8•10 years ago
|
||
Another thing which could be optimized as part of this bug is to merge setupEnvironment() and run_test() in head.js.
Updated•9 years ago
|
backlog: --- → webRTC+
Rank: 33
Priority: -- → P3
Comment 9•7 years ago
|
||
Mass change P3->P4 to align with new Mozilla triage process.
Priority: P3 → P4
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•