1167443 - mochitest end-of-candidate handling is flawed

Status: RESOLVED FIXED

(Core :: WebRTC, defect, P2)

Description

[Nils Ohlmeier [:drno]]

Evidence: https://treeherder.mozilla.org/logviewer.html#?job_id=10041547&repo=mozilla-inbound

These type error happened because the end-of-candidate promise got fulfilled after the PeerConnetions had been closed already.

Plan to fix it:
- Setup the promise variable for end-of-candidate (EOC) when sLD gets called, to ensure that we actually catch multiple EOC's in case of renegotiation
- add an extra test step at the very end of the regular chain to check for EOC
- move the code which verifies the SDP at EOC into the test step function, so it can't get executed any more after test shutdown
- carefully verify that renegotiation test cases actually wait multiple times for EOC

Comment 1

[Nils Ohlmeier [:drno]]

(In reply to Nils Ohlmeier [:drno] from comment #0)
> - carefully verify that renegotiation test cases actually wait multiple
> times for EOC

After further discussion we want renegotiation by default to not wait for EOC for speed reason and to try to get to EOC's overlaps. But then we need a new dedicated test which explicitly waits for each EOC between renegotiation's to ensure that we receive an EOC for each renegotiation.

Comment 2

[Nils Ohlmeier [:drno]]

Attachment: MozReview Request: bz://1167443/drno

/r/9319 - Bug 1167443: fix the SDP verification after end-of-candidates

Pull down this commit:

hg pull -r 193ce0b94e3761b87825927672bc69089c8b1bd0 https://reviewboard-hg.mozilla.org/gecko/

Comment 3

[Nils Ohlmeier [:drno]]

(In reply to Nils Ohlmeier [:drno] from comment #0)
> These type error happened because the end-of-candidate promise got fulfilled
> after the PeerConnetions had been closed already.

Turned out that is actually not the case, as the PeerConnections are kept around even after everything got closed. And even pulling the last local SDP from a closed PeerConnection works.
The problem was simply that the code in the Promise was full of errors and never worked as expected, and the JS errors from that code never got caught, because the Promise had not catch on it.

Comment 4

[Nils Ohlmeier [:drno]]

Note: this is still missing the dedicated test case which waits for EOC and renegotiates afterwards and verifies the second EOC.

Comment 5

[Byron Campen [:bwc]]

Comment on attachment 8609794 [details]
MozReview Request: bz://1167443/drno

https://reviewboard.mozilla.org/r/9317/#review8085

::: dom/media/tests/mochitest/templates.js:277
(Diff revision 1)
> +    if (test.steeplechase) {
> +      test.pcLocal.endOfTrickleIce.then(() => {
> +        send_message({"type": "end_of_trickle_ice"});
> +      });
> +    } else {
> +      test.pcLocal.endOfTrickleIce.then(() => {
> +        test.pcLocal._pc.onicecandidate = () =>
> +          ok(false, test.pcLocal.label + " received ICE candidate after end of trickle");
> +        test.pcLocal.eocSDP =
> +          JSON.parse(JSON.stringify(test.pcLocal._pc.localDescription));
> +      })
> +      .catch(e => ok(false, "end-of-trickel-ICE error: "+ e));
> +    }

Could this be moved to |setLocalDescription|? Or does that upset some test-cases that call |setLocalDescription| directly? If this does break things, we could probably move it to a separate function.

::: dom/media/tests/mochitest/templates.js:288
(Diff revision 1)
> +      .catch(e => ok(false, "end-of-trickel-ICE error: "+ e));

Nit: s/trickel/trickle/

Also, let's put a label here.

::: dom/media/tests/mochitest/templates.js:528
(Diff revision 1)
> -    return test.pcLocal.endOfTrickleIce;
> +    if (test.pcLocal.eocSDP) {
> +        var localSdp = test.pcLocal.eocSDP;
> +        ok(localSdp.sdp.includes("a=end-of-candidates"), "SDP contains end-of-candidates");
> +        if (localSdp.type === "offer") {
> +          ok(localSdp.sdp.includes("a=rtcp:"), "SDP contains rtcp");
> +        }
> +        ok(!localSdp.sdp.includes("c=IN IP4 0.0.0.0"), "SDP contains non-zero IP c line");
> +    } else {
> +      info(test.pcRemote.label + " no end-of-candidates SDP stored to verify");
> +    }

Let's move this into a single function.

Comment 6

[Martin Thomson [:mt:]]

Comment on attachment 8609794 [details]
MozReview Request: bz://1167443/drno

https://reviewboard.mozilla.org/r/9317/#review8087

::: dom/media/tests/mochitest/templates.js:277
(Diff revision 1)
> +    if (test.steeplechase) {
> +      test.pcLocal.endOfTrickleIce.then(() => {
> +        send_message({"type": "end_of_trickle_ice"});
> +      });
> +    } else {
> +      test.pcLocal.endOfTrickleIce.then(() => {
> +        test.pcLocal._pc.onicecandidate = () =>
> +          ok(false, test.pcLocal.label + " received ICE candidate after end of trickle");
> +        test.pcLocal.eocSDP =
> +          JSON.parse(JSON.stringify(test.pcLocal._pc.localDescription));
> +      })
> +      .catch(e => ok(false, "end-of-trickel-ICE error: "+ e));
> +    }

I have similar concerns to Byron here.

I imagined that this fix would have test.setLocalDescription (or test.pc.setLocalDescription) create a promise that resolves with the end-of-candidates SDP.

Then the final test step could just await that and do the checks.

::: dom/media/tests/mochitest/templates.js:531
(Diff revision 1)
> +        if (localSdp.type === "offer") {
> +          ok(localSdp.sdp.includes("a=rtcp:"), "SDP contains rtcp");
> +        }

Why do we only check for a=rtcp: in an offer?

Comment 7

[Nils Ohlmeier [:drno]]

https://reviewboard.mozilla.org/r/9317/#review8153

> Why do we only check for a=rtcp: in an offer?

Because the answer doesn't have a rtcp port if we agree on rtcp-mux, which all of our tests seem to do.

> I have similar concerns to Byron here.
> 
> I imagined that this fix would have test.setLocalDescription (or test.pc.setLocalDescription) create a promise that resolves with the end-of-candidates SDP.
> 
> Then the final test step could just await that and do the checks.

According to the records of our discussion https://bugzilla.mozilla.org/show_bug.cgi?id=1167443#c1 by default we do not want to wait for EOC (but only have a single test case which explicitly waits for EOC before shutting down). So I'm not sure how to accomplish that easily with a promise as it needs to be optional.

Comment 8

[Martin Thomson [:mt:]]

https://reviewboard.mozilla.org/r/9317/#review8193

> According to the records of our discussion https://bugzilla.mozilla.org/show_bug.cgi?id=1167443#c1 by default we do not want to wait for EOC (but only have a single test case which explicitly waits for EOC before shutting down). So I'm not sure how to accomplish that easily with a promise as it needs to be optional.

Make the promise every time.  Only use the promise when you are adding the special test step.

> Because the answer doesn't have a rtcp port if we agree on rtcp-mux, which all of our tests seem to do.

That sounds like an opportunity for more test cases, though that might be covered by the signaling unit tests.

I would suggest that the test check for either a=rtcpmux or a=rtcp in the answer then.

I.e., localSdp.sdp.includes("a=rtcp:") || (localSdp.type === "answer" && localSdp.sdp.includes("a=rtcpmux\r"))

Comment 9

[Nils Ohlmeier [:drno]]

Attachment: MozReview Request: Bug 1167443: fix verification of end-of-candidates in mochitests. r=mt

Bug 1167443: fix the SDP verification after end-of-candidates

Comment 10

[Nils Ohlmeier [:drno]]

Comment on attachment 8612758 [details]
MozReview Request: Bug 1167443: fix verification of end-of-candidates in mochitests. r=mt

Bug 1167443: fix the SDP verification after end-of-candidates

Comment 11

[Martin Thomson [:mt:]]

https://reviewboard.mozilla.org/r/9317/#review8437

::: dom/media/tests/mochitest/pc.js:405
(Diff revision 2)
> +        ok(false, peer.label + " received ICE candidate after end of trickle");
> +      resolveEndOfTrickleSdp(peer._pc.localDescription);

I don't understand this code.

Given that we already have endOfTrickleIce, why don't we use that.  As for the check that you have added here, I think that could just as easily go into setupIceCandidateHandler.

::: dom/media/tests/mochitest/sdpUtils.js:7
(Diff revision 2)
> +    info(label + " no end-of-candidates SDP stored to verify");

Why isn't this a test failure?

::: dom/media/tests/mochitest/sdpUtils.js:1
(Diff revision 2)
> +function checkSdpAfterEndOfTrickle(label, sdp) {

This file needs a copyright statement.

I also wonder whether the functions might be grouped to avoid adding so much to the global namespace:

var sdputil = {
  checkSdpAfterEndOfTrickle: function(...) {...},
};

Finally, I assume that this is a direct transplant of code, so I didn't review it thoroughly.

::: dom/media/tests/mochitest/templates.js
(Diff revision 2)
> -  function PC_REMOTE_CHECK_FOR_DUPLICATED_PORTS_IN_SDP(test) {

I see that you factored this out, but I can't see a call to the new function.

::: dom/media/tests/mochitest/templates.js:466
(Diff revision 2)
> +    .then(sdp => checkSdpAfterEndOfTrickle(test.pcLocal.label, sdp),
> +          () => info("pcLocal: SDP after end-of-candidates missing"));
> +  },

Same question as before: this doesn't guarantee that the step is run.  I think that we need at least one test that waits and fails if we don't see EOC.

::: dom/media/tests/mochitest/sdpUtils.js:157
(Diff revision 2)
> +  if (offerToReceiveAudio) {
> +    return 1;
> +  } else {
> +    return 0;
> +  }

I know that this isn't your fault, but this function can return true or false given how it is used.

Comment 12

[Byron Campen [:bwc]]

Comment on attachment 8612758 [details]
MozReview Request: Bug 1167443: fix verification of end-of-candidates in mochitests. r=mt

https://reviewboard.mozilla.org/r/9319/#review8439

::: dom/media/tests/mochitest/templates.js:467
(Diff revisions 1 - 2)
> -        }
> +          () => info("pcLocal: SDP after end-of-candidates missing"));

This is going to be misleading logging. Suggest something more like "pcLocal: Gathering is not complete, skipping post-gathering SDP check."

::: dom/media/tests/mochitest/sdpUtils.js:82
(Diff revision 2)
> +function checkForDuplicatedPortsInSdp(offer, answer) {

I don't see anything calling this.

::: dom/media/tests/mochitest/templates.js:365
(Diff revisions 1 - 2)
> -    test.pcRemote.verifySdp(test._remote_answer, "answer",
> +    verifySdp(test._remote_answer, "answer",
>                              test._offer_constraints, test._offer_options,
>                              true);
>    },
>    function PC_LOCAL_SANE_REMOTE_SDP(test) {
> -    test.pcLocal.verifySdp(test._remote_answer, "answer",
> +    verifySdp(test._remote_answer, "answer",
>                             test._offer_constraints, test._offer_options,
>                             false);

Nit: whitespace

::: dom/media/tests/mochitest/sdpUtils.js:2
(Diff revision 2)
> +  if (sdp) {
> +    ok(sdp.sdp.includes("a=end-of-candidates"), label + ": SDP contains end-of-candidates");
> +    ok((sdp.sdp.includes("a=rtcp:") || (sdp.type === "answer" && sdp.sdp.includes("a=rtcp-mux"))), label + ": SDP contains rtcp port or rtcp-mux");
> +    ok(!sdp.sdp.includes("c=IN IP4 0.0.0.0"), label + ": SDP contains non-zero IP c line");
> +  } else {
> +    info(label + " no end-of-candidates SDP stored to verify");
> +  }

I don't think we call this with a null or undefined SDP anymore.

::: dom/media/tests/mochitest/templates.js:475
(Diff revisions 1 - 2)
> -        }
> +          () => info("pcRemote: SDP after end-of-candidates missing"));

Same here.

Comment 13

[Nils Ohlmeier [:drno]]

https://reviewboard.mozilla.org/r/9317/#review8517

> I see that you factored this out, but I can't see a call to the new function.

Yes I took that out for two reasons:
- this was very specifically put there to track down an issue we use to have on IPC scenarios
- this test is broken, as it does not work with trickle ICE

> I know that this isn't your fault, but this function can return true or false given how it is used.

Well countTracksInConstraints returns an integer, and I would prefer to not rely on fancy language features which are suppose to compare integers with booleans.

> Why isn't this a test failure?

It was optional in previous revision. Turned it into a failure now.

Comment 14

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

https://reviewboard.mozilla.org/r/9317/#review8649

Drive-by feedback.

::: dom/media/tests/mochitest/pc.js:395
(Diff revision 2)
> +  var resolveEndOfTrickleSdp;
> +  peer.endOfTrickleSdp = new Promise(r => resolveEndOfTrickleSdp = r);
> +
> +  if (this.steeplechase) {
> +    peer.endOfTrickleIce.then(() => {
> +      send_message({"type": "end_of_trickle_ice"});
> +    });
> +  } else {
> +    peer.endOfTrickleIce.then(() => {
> +      peer._pc.onicecandidate = () =>
> +        ok(false, peer.label + " received ICE candidate after end of trickle");
> +      resolveEndOfTrickleSdp(peer._pc.localDescription);
> +    })
> +    .catch(e => ok(false, peer.label + ": end-of-trickle-ICE error: "+ e));
> +  }

This never resolves peer.endOfTrickleSdp on steeplechase, is that intentional?

I also find it hard to reason about errors here, can this can be simplified to avoid the new Promise(r => p.then(r())) anti-pattern?

::: dom/media/tests/mochitest/templates.js:462
(Diff revision 2)
> -    return test.pcLocal.endOfTrickleIce;
> -  },
> -  function PC_REMOTE_WAIT_FOR_END_OF_TRICKLE(test) {
> -    return test.pcRemote.endOfTrickleIce;
> +    return Promise.race([
> +      test.pcLocal.endOfTrickleSdp,
> +      Promise.reject("No SDP")
> +    ])
> +    .then(sdp => checkSdpAfterEndOfTrickle(test.pcLocal.label, sdp),

This will always reject and never run the .then() below.

::: dom/media/tests/mochitest/pc.js:399
(Diff revision 2)
> +    peer.endOfTrickleIce.then(() => {
> +      send_message({"type": "end_of_trickle_ice"});
> +    });

Missing catch here will hide errors.

Comment 15

[Nils Ohlmeier [:drno]]

It will be easier to wait to proceed with the bug after the new dependencies are fixed.

Comment 16

[Nils Ohlmeier [:drno]]

https://reviewboard.mozilla.org/r/9317/#review8713

> This never resolves peer.endOfTrickleSdp on steeplechase, is that intentional?
> 
> I also find it hard to reason about errors here, can this can be simplified to avoid the new Promise(r => p.then(r())) anti-pattern?

After Martin's suggestion the re-written section of this looks quite different, so this should no longer be an issue.

> Missing catch here will hide errors.

Thanks for catching that.

> This will always reject and never run the .then() below.

This is actually not correct. As the first promise endOfTrickleSdp is resolved already it wins (as long as you don't have .then code depending on that promise).

It's ugly, but try it yourself, it works :-)

Comment 17

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

(In reply to Nils Ohlmeier [:drno] from comment #16)
> This is actually not correct. As the first promise endOfTrickleSdp is
> resolved already it wins (as long as you don't have .then code depending on
> that promise).
> 
> It's ugly, but try it yourself, it works :-)

mind = blown.

Cool! But if I flip the order in the array then it doesn't work. Does Promise.race guarantee the order in which it checks to be first-to-last?

Comment 19

[Nils Ohlmeier [:drno]]

Comment on attachment 8612758 [details]
MozReview Request: Bug 1167443: fix verification of end-of-candidates in mochitests. r=mt

Bug 1167443: fix verification of end-of-candidates in mochitests

Comment 20

[Nils Ohlmeier [:drno]]

Comment on attachment 8612758 [details]
MozReview Request: Bug 1167443: fix verification of end-of-candidates in mochitests. r=mt

Bug 1167443: fix verification of end-of-candidates in mochitests

Comment 21

[Martin Thomson [:mt:]]

https://reviewboard.mozilla.org/r/9319/#review15191

sdpUtils.js is missing...

::: dom/media/tests/mochitest/pc.js:1220
(Diff revision 3)
> +        todo(this._pc.iceGatheringState === 'completed',
> +           "ICE gathering state has reached completed");

Is this a race condition between updating iceGatheringState and the event, or do we have something else going on here?

Comment please.

::: dom/media/tests/mochitest/pc.js
(Diff revision 3)
> -  audioInOfferOptions : function(options) {

jib removed this function with his recent patch, so you might want to check that...https://bugzilla.mozilla.org/show_bug.cgi?id=1064223

I guess that you can race each other.

::: dom/media/tests/mochitest/templates.js:467
(Diff revision 3)
> -    return test.pcLocal.endOfTrickleIce;
> -  },
> -  function PC_REMOTE_WAIT_FOR_END_OF_TRICKLE(test) {
> -    return test.pcRemote.endOfTrickleIce;
> +    return Promise.race([
> +      test.pcLocal.endOfTrickleSdp,
> +      Promise.reject("No SDP")
> +    ])

Maybe a comment here explaining the use of `Promise.race()` for those who might be as surprised about this as jib was.

::: dom/media/tests/mochitest/test_peerConnection_basicAudio.html:19
(Diff revision 3)
> +        return test.pcLocal.endOfTrickleSdp .then(sdp =>

extra ws before `.then`

Comment 22

[Nils Ohlmeier [:drno]]

Comment on attachment 8612758 [details]
MozReview Request: Bug 1167443: fix verification of end-of-candidates in mochitests. r=mt

Bug 1167443: fix verification of end-of-candidates in mochitests. r?mt,r?bwc,r?jib

Comment 23

[Nils Ohlmeier [:drno]]

Comment on attachment 8612758 [details]
MozReview Request: Bug 1167443: fix verification of end-of-candidates in mochitests. r=mt

Bug 1167443: fix verification of end-of-candidates in mochitests. r?mt,r?bwc,r?jib

Comment 24

[Martin Thomson [:mt:]]

Comment on attachment 8612758 [details]
MozReview Request: Bug 1167443: fix verification of end-of-candidates in mochitests. r=mt

https://reviewboard.mozilla.org/r/9319/#review15473

::: dom/media/tests/mochitest/pc.js:85
(Diff revision 4)
>    if (options.is_local)
> -    this.pcLocal = new PeerConnectionWrapper('pcLocal', options.config_local, options.h264);
> +    this.pcLocal = new PeerConnectionWrapper('pcLocal', options.config_local);
>    else
>      this.pcLocal = null;

Braces please.

::: dom/media/tests/mochitest/pc.js:95
(Diff revision 4)
> -  this.steeplechase = this.pcLocal === null || this.pcRemote === null;
> +  options.steeplechase = this.pcLocal === null || this.pcRemote === null;

options.steeplechase = !options.is_local || !options.is_remote;

::: dom/media/tests/mochitest/pc.js:398
(Diff revision 4)
> +  /* We have to modify the chain here to allow tests which modify the default
> +   * test chain instantiating a PeerConnectionTest() */
> +  if(this.testOptions.h264) {
> +    this.chain.insertAfterEach(
> +      'PC_LOCAL_CREATE_OFFER',
> +      [PC_LOCAL_REMOVE_VP8_FROM_OFFER]);
> +  }
> +  if(!this.testOptions.bundle) {
> +    this.chain.insertAfterEach(
> +      'PC_LOCAL_CREATE_OFFER',
> +      [PC_LOCAL_REMOVE_BUNDLE_FROM_OFFER]);
> +  }
> +  if(!this.testOptions.rtcpmux) {
> +    this.chain.insertAfterEach(
> +      'PC_LOCAL_CREATE_OFFER',
> +      [PC_LOCAL_REMOVE_RTCPMUX_FROM_OFFER]);
> +  }
> +  if (!this.testOptions.is_local) {
> +    this.chain.filterOut(/^PC_LOCAL/);
> +  }
> +  if (!this.testOptions.is_remote) {
> +    this.chain.filterOut(/^PC_REMOTE/);
> +  }

Please move this stuff to `PeerConnectionTest.prototype.filterOutUnwantedSteps()` or something like that.

::: dom/media/tests/mochitest/pc.js:400
(Diff revision 4)
> +  if(this.testOptions.h264) {

space between if and (

::: dom/media/tests/mochitest/pc.js:1665
(Diff revision 4)
>        var numDataTracks = this.dataChannels.length;
>  
> -      var numAudioVideoDataTracks = numAudioTracks + numVideoTracks + numDataTracks;
> +      var numAudioVideoDataTracks = numAvTracks + numDataTracks;

This is wrong, you need to set numDataTracks to this.dataChannels.length ? 1 : 0.

I would just do this though:
    numTracks = numAudioTracks + numVideoTracks;
    if (!testOptions.rtcpmux) {
      numTracks *= 2;
    }
    if (this.dataChannels.length) {
      ++numTracks;
    }

::: dom/media/tests/mochitest/sdpUtils.js:38
(Diff revision 4)
> +  var updated_sdp = sdp.replace(/a=rtcp-mux\r\n/g,"");
> +  return updated_sdp;

See below.

::: dom/media/tests/mochitest/sdpUtils.js:43
(Diff revision 4)
> +  var updated_sdp = sdp.replace(/a=group:BUNDLE .*\r\n/g,
> +                                "");

Unwrap this.

::: dom/media/tests/mochitest/sdpUtils.js:43
(Diff revision 4)
> +  var updated_sdp = sdp.replace(/a=group:BUNDLE .*\r\n/g,
> +                                "");
> +  return updated_sdp;

I would just:

return sdp.replace(/.../, "");

::: dom/media/tests/mochitest/sdpUtils.js:71
(Diff revision 4)
> +      sdputils.countTracksInConstraint('audio', offerConstraintsList) ||
> +      ((offerOptions && offerOptions.offerToReceiveAudio) ? 1 : 0);

Do we even test with offerToReceiveAudio of 2 or more?

::: dom/media/tests/mochitest/test_peerConnection_basicAudioVideoNoRtcpMux.html:21
(Diff revision 4)
> +        return test.pcLocal.endOfTrickleSdp .then(sdp =>

Extra space here.

Comment 25

[Nils Ohlmeier [:drno]]

https://reviewboard.mozilla.org/r/9319/#review8439

> I don't see anything calling this.

Yeah we removed the call to this some time ago from templates.js. But I guess this is a good time to leave this ugly piece of code to versioning tools if we ever need it again.

Comment 26

[Nils Ohlmeier [:drno]]

https://reviewboard.mozilla.org/r/9319/#review15473

> Do we even test with offerToReceiveAudio of 2 or more?

I don't think so.

It would anyhow only work with a re-negotiation, because the attribute name exists and is only a Boolean.

Comment 27

[Nils Ohlmeier [:drno]]

Comment on attachment 8612758 [details]
MozReview Request: Bug 1167443: fix verification of end-of-candidates in mochitests. r=mt

Bug 1167443: fix verification of end-of-candidates in mochitests. r=mt,r?bwc,r?jib

Comment 28

[Byron Campen [:bwc]]

Comment on attachment 8612758 [details]
MozReview Request: Bug 1167443: fix verification of end-of-candidates in mochitests. r=mt

https://reviewboard.mozilla.org/r/9319/#review15595

::: dom/media/tests/mochitest/pc.js:711
(Diff revision 5)
> -  this.h264 = typeof h264 !== "undefined" ? true : false;
> +  this.offerAnswerCounter = 0;

Not seeing anything that uses this.

::: dom/media/tests/mochitest/pc.js:1674
(Diff revision 5)
> -      var numDataTracks = this.dataChannels.length;
> +      var numTracks = numAudioTracks + numVideoTracks;

Suggest s/numTracks/numExpectedTransports/

::: dom/media/tests/mochitest/pc.js:1683
(Diff revision 5)
> -      var numAudioVideoDataTracks = numAudioTracks + numVideoTracks + numDataTracks;
> -      info("expected audio + video + data tracks: " + numAudioVideoDataTracks);
> +      info("expected audio + video + data tracks: " + numTracks);
> +      is(numIceConnections, numTracks, "stats ICE connections matches expected A/V tracks");

s/tracks/transports/

::: dom/media/tests/mochitest/sdpUtils.js:1
(Diff revision 5)
> +/* This Source Code Form is subject to the terms of the Mozilla Public

I'm assuming that all of this is strictly copied from elsewhere.

Comment 29

[Nils Ohlmeier [:drno]]

https://reviewboard.mozilla.org/r/9319/#review15595

> I'm assuming that all of this is strictly copied from elsewhere.

Correct, only copies from various location in the same directory.

Comment 30

[Nils Ohlmeier [:drno]]

Comment on attachment 8612758 [details]
MozReview Request: Bug 1167443: fix verification of end-of-candidates in mochitests. r=mt

Bug 1167443: fix verification of end-of-candidates in mochitests. r=mt

Comment 31

[Nils Ohlmeier [:drno]]

Try run looks green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=300f70c9e52c

Comment 32

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/30f65371e36c

Comment 33

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/30f65371e36c