Issue metadata
Sign in to add a comment
|
WebRtcGetUserMediaBrowserTest.TwoGetUserMediaAndVerifyFrameRate doesn't do anything |
||||||||||||||||||||||
Issue description
The problem is that TwoGetUserMediaAndVerifyFrameRate does not verify anything at all, since its callback had
var validateFrameRateCallback = function (success) {
if (!success)
failTest("Failed to validate frameRate.");
eventOccured();
};
However, validate framerate does not call back with false if it fails, it calls back with an error message, i.e. an nonempty string. !"nonempty string" always evaluates false, so failTest can never occur.
,
Nov 28 2017
I doubt we can salvage this test unfortunately. This is inherently a performance test, and asserting on performance values almost inevitably goes flaky. I think that's what we saw here as well. I'd recommend just deleting it. I do think frame rate measurements are well covered by our performance tests, like https://chromeperf.appspot.com/report?sid=dacecead3bcc3440bb380e33e85710c16d285bed6bcc5fa25dccf882b161d657.
,
Nov 28 2017
,
Nov 28 2017
All right, fine, I'll do it.
,
Nov 28 2017
,
Nov 29 2017
The NextAction date has arrived: 2017-11-29
,
Dec 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9db327358e303e9900eef58036c31cdcb8512845 commit 9db327358e303e9900eef58036c31cdcb8512845 Author: Patrik Höglund <phoglund@chromium.org> Date: Tue Dec 05 12:41:34 2017 Remove WebRtcGetUserMediaBrowserTest.TwoGetUserMediaAndVerifyFrameRate. The test doesn't do anything and hasn't for a while. I don't think it can be salvaged because it's inherently a performance test. It will inevitably be flaky. We do have performance tests that cover frame rates, maybe not in the "two getusermedia" case though. Bug: 789121 Change-Id: I483096ed8ac18f4572245f93d1f2b2ccb2c473f2 Reviewed-on: https://chromium-review.googlesource.com/803348 Reviewed-by: Guido Urdaneta <guidou@chromium.org> Commit-Queue: Patrik Höglund <phoglund@chromium.org> Cr-Commit-Position: refs/heads/master@{#521678} [modify] https://crrev.com/9db327358e303e9900eef58036c31cdcb8512845/content/browser/webrtc/webrtc_getusermedia_browsertest.cc [modify] https://crrev.com/9db327358e303e9900eef58036c31cdcb8512845/content/test/data/media/getusermedia.html [modify] https://crrev.com/9db327358e303e9900eef58036c31cdcb8512845/content/test/data/media/webrtc_test_utilities.js
,
Dec 5 2017
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by phoglund@chromium.org
, Nov 28 2017