New issue
Advanced search Search tips

Issue 773472 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 706359



Sign in to add a comment

Migrate browser_tests to wpt that are not browser tests

Project Member Reported by hbos@chromium.org, Oct 10 2017

Issue description

Due to mocking limitations ( https://crbug.com/706359 ) some WebRTC tests were written as browser tests instead of layout tests.

Some that come to mind:
createReceiverWithSetRemoteDescription(), switchRemoteStreamAndBackAgain(), switchRemoteStreamAndBackAgain(), switchRemoteStreamWithoutWaitingForPromisesToResolve()
https://cs.chromium.org/chromium/src/chrome/test/data/webrtc/peerconnection_rtp.js?q=peerconnection_rtp.js&sq=package:chromium&dr&l=230

But anything that isn't "establish a call between two tabs" could be moved, and if written as a Web Platform Test, we get the benefits of WPT (listed on http://wpt.fyi/, shared tests and collaboration between browsers, etc).

Tests that requires two RTCPeerConnection can typically be more concisely written as layout tests in the same tab.

1. List tests that should be moved here.
2. ?
3. Profit.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 14 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/199c6b4edf10033bed38333393e769018d7f1058

commit 199c6b4edf10033bed38333393e769018d7f1058
Author: Henrik Boström <hbos@chromium.org>
Date: Thu Jun 14 15:40:47 2018

Delete WebRtcRtpBrowserTest.

Tests that can be expressed as LayoutTests (preferreably Web Platform
Tests) rather than browser tests should. Browser tests are slow to run
and a maintenance burden.

The WebRtcRtpBrowserTests are mainly testing behaviors of adding and
removing tracks/streams and asserting that the expected outcomes.

- The essentials of these tests are already covered in
  external/wpt/webrtc/ which we are actively maintaining and updating in
  preparation for Unified Plan and RTCRtpTransceiver support. Almost all
  of it is overlapping.
- The WebRtcRtpBrowserTests were not written to test behaviors, but
  functions. Each test is asserting a lot of unrelated behaviors.
- When running with Unified Plan/RTCRtpTransceiver (WIP CL:
  https://chromium-review.googlesource.com/c/chromium/src/+/1025771)
  these tests start failing. I started updating these tests but the
  failures were due to Plan B assumptions (problems in the tests, not in
  the implementation).
- Debugging this part of the code is slow and cumbersome.
- I won't miss any of these tests.

Conclusion: Nuke it.

Bug: 773472,  777617 
Change-Id: I099992f1783914d4fb1d5c2d952ff977ec4cc513
Reviewed-on: https://chromium-review.googlesource.com/1100891
Reviewed-by: Patrik Höglund <phoglund@chromium.org>
Commit-Queue: Henrik Boström <hbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567283}
[modify] https://crrev.com/199c6b4edf10033bed38333393e769018d7f1058/chrome/browser/media/webrtc/webrtc_browsertest_base.cc
[modify] https://crrev.com/199c6b4edf10033bed38333393e769018d7f1058/chrome/browser/media/webrtc/webrtc_browsertest_base.h
[delete] https://crrev.com/be8344a8ca77a9e0f9e7af0eab9d4a85ead9c36b/chrome/browser/media/webrtc/webrtc_rtp_browsertest.cc
[modify] https://crrev.com/199c6b4edf10033bed38333393e769018d7f1058/chrome/test/BUILD.gn
[delete] https://crrev.com/be8344a8ca77a9e0f9e7af0eab9d4a85ead9c36b/chrome/test/data/webrtc/peerconnection_rtp.js
[modify] https://crrev.com/199c6b4edf10033bed38333393e769018d7f1058/chrome/test/data/webrtc/webrtc_jsep01_test.html

Sign in to add a comment