Commit 199c6b4e authored by Henrik Boström's avatar Henrik Boström Committed by Commit Bot

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/1100891Reviewed-by: default avatarPatrik Höglund <phoglund@chromium.org>
Commit-Queue: Henrik Boström <hbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567283}
parent be8344a8
...@@ -154,13 +154,6 @@ std::vector<std::string> JsonArrayToVectorOfStrings( ...@@ -154,13 +154,6 @@ std::vector<std::string> JsonArrayToVectorOfStrings(
} // namespace } // namespace
WebRtcTestBase::TrackEvent::TrackEvent(const std::string& track_id)
: track_id(track_id) {}
WebRtcTestBase::TrackEvent::TrackEvent(const TrackEvent&) = default;
WebRtcTestBase::TrackEvent::~TrackEvent() = default;
WebRtcTestBase::WebRtcTestBase(): detect_errors_in_javascript_(false) { WebRtcTestBase::WebRtcTestBase(): detect_errors_in_javascript_(false) {
// The handler gets set for each test method, but that's fine since this // The handler gets set for each test method, but that's fine since this
// set operation is idempotent. // set operation is idempotent.
...@@ -607,151 +600,6 @@ void WebRtcTestBase::EnableOpusDtx(content::WebContents* tab) const { ...@@ -607,151 +600,6 @@ void WebRtcTestBase::EnableOpusDtx(content::WebContents* tab) const {
EXPECT_EQ("ok-forced", ExecuteJavascript("forceOpusDtx()", tab)); EXPECT_EQ("ok-forced", ExecuteJavascript("forceOpusDtx()", tab));
} }
void WebRtcTestBase::CreateAndAddStreams(content::WebContents* tab,
size_t count) const {
EXPECT_EQ(
"ok-streams-created-and-added",
ExecuteJavascript(
"createAndAddStreams(" + base::NumberToString(count) + ")", tab));
}
void WebRtcTestBase::VerifyRtpSenders(
content::WebContents* tab,
base::Optional<size_t> expected_num_tracks) const {
std::string javascript =
expected_num_tracks ? "verifyRtpSenders(" +
base::NumberToString(*expected_num_tracks) + ")"
: "verifyRtpSenders()";
EXPECT_EQ("ok-senders-verified", ExecuteJavascript(javascript, tab));
}
void WebRtcTestBase::VerifyRtpReceivers(
content::WebContents* tab,
base::Optional<size_t> expected_num_tracks) const {
std::string javascript =
expected_num_tracks ? "verifyRtpReceivers(" +
base::NumberToString(*expected_num_tracks) + ")"
: "verifyRtpReceivers()";
EXPECT_EQ("ok-receivers-verified", ExecuteJavascript(javascript, tab));
}
std::vector<std::string> WebRtcTestBase::CreateAndAddAudioAndVideoTrack(
content::WebContents* tab,
StreamArgumentType stream_argument_type) const {
const char* string_argument_type_str = nullptr;
switch (stream_argument_type) {
case StreamArgumentType::NO_STREAM:
string_argument_type_str = "'no-stream'";
break;
case StreamArgumentType::SHARED_STREAM:
string_argument_type_str = "'shared-stream'";
break;
case StreamArgumentType::INDIVIDUAL_STREAMS:
string_argument_type_str = "'individual-streams'";
break;
}
std::string result =
ExecuteJavascript(base::StringPrintf("createAndAddAudioAndVideoTrack(%s)",
string_argument_type_str),
tab);
EXPECT_TRUE(base::StartsWith(result, "ok-", base::CompareCase::SENSITIVE));
std::vector<std::string> ids = base::SplitString(
result.substr(3), " ", base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL);
EXPECT_EQ(4u, ids.size());
return ids;
}
void WebRtcTestBase::RemoveTrack(content::WebContents* tab,
const std::string& track_id) const {
EXPECT_EQ(
"ok-sender-removed",
ExecuteJavascript(
base::StringPrintf("removeTrack('%s')", track_id.c_str()), tab));
}
bool WebRtcTestBase::HasLocalStreamWithTrack(
content::WebContents* tab,
const std::string& stream_id,
const std::string& track_id) const {
return HasStreamWithTrack(tab, "hasLocalStreamWithTrack", stream_id,
track_id);
}
bool WebRtcTestBase::HasRemoteStreamWithTrack(
content::WebContents* tab,
const std::string& stream_id,
const std::string& track_id) const {
return HasStreamWithTrack(tab, "hasRemoteStreamWithTrack", stream_id,
track_id);
}
bool WebRtcTestBase::HasStreamWithTrack(content::WebContents* tab,
const char* function_name,
std::string stream_id,
std::string track_id) const {
if (stream_id != kUndefined)
stream_id = "'" + stream_id + "'";
std::string javascript = base::StringPrintf(
"%s(%s, '%s')", function_name, stream_id.c_str(), track_id.c_str());
std::string result = ExecuteJavascript(javascript, tab);
EXPECT_TRUE(result == "ok-stream-with-track-found" ||
result == "ok-stream-with-track-not-found");
return result == "ok-stream-with-track-found";
}
bool WebRtcTestBase::HasSenderWithTrack(content::WebContents* tab,
std::string track_id) const {
std::string javascript =
base::StringPrintf("hasSenderWithTrack('%s')", track_id.c_str());
std::string result = ExecuteJavascript(javascript, tab);
EXPECT_TRUE(result == "ok-sender-with-track-found" ||
result == "ok-sender-with-track-not-found");
return result == "ok-sender-with-track-found";
}
bool WebRtcTestBase::HasReceiverWithTrack(content::WebContents* tab,
std::string track_id) const {
std::string javascript =
base::StringPrintf("hasReceiverWithTrack('%s')", track_id.c_str());
std::string result = ExecuteJavascript(javascript, tab);
EXPECT_TRUE(result == "ok-receiver-with-track-found" ||
result == "ok-receiver-with-track-not-found");
return result == "ok-receiver-with-track-found";
}
size_t WebRtcTestBase::GetNegotiationNeededCount(
content::WebContents* tab) const {
std::string result = ExecuteJavascript("getNegotiationNeededCount()", tab);
EXPECT_TRUE(base::StartsWith(result, "ok-negotiation-count-is-",
base::CompareCase::SENSITIVE));
size_t count = 0;
EXPECT_TRUE(base::StringToSizeT(result.substr(24), &count));
return count;
}
std::vector<WebRtcTestBase::TrackEvent> WebRtcTestBase::GetTrackEvents(
content::WebContents* tab) const {
std::string result = ExecuteJavascript("getTrackEvents()", tab);
EXPECT_TRUE(base::StartsWith(result, "ok-", base::CompareCase::SENSITIVE));
std::vector<std::string> tokens = base::SplitString(
result.substr(3), " ", base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL);
std::vector<TrackEvent> events;
for (size_t i = 0; i < tokens.size(); ++i) {
if (tokens[i] == "RTCTrackEvent") {
DCHECK_LT(i + 1, tokens.size());
events.push_back(TrackEvent(tokens[++i]));
} else {
DCHECK(!events.empty());
events[events.size() - 1].stream_ids.push_back(tokens[i]);
}
}
return events;
}
void WebRtcTestBase::CollectGarbage(content::WebContents* tab) const {
EXPECT_EQ("ok-gc", ExecuteJavascript("collectGarbage()", tab));
}
std::string WebRtcTestBase::GetDesktopMediaStream(content::WebContents* tab) { std::string WebRtcTestBase::GetDesktopMediaStream(content::WebContents* tab) {
DCHECK(static_cast<bool>(LoadDesktopCaptureExtension())); DCHECK(static_cast<bool>(LoadDesktopCaptureExtension()));
......
...@@ -58,15 +58,6 @@ class WebRtcTestBase : public InProcessBrowserTest { ...@@ -58,15 +58,6 @@ class WebRtcTestBase : public InProcessBrowserTest {
INDIVIDUAL_STREAMS INDIVIDUAL_STREAMS
}; };
struct TrackEvent {
explicit TrackEvent(const std::string& track_id);
TrackEvent(const TrackEvent&);
~TrackEvent();
std::string track_id;
std::vector<std::string> stream_ids;
};
protected: protected:
WebRtcTestBase(); WebRtcTestBase();
~WebRtcTestBase() override; ~WebRtcTestBase() override;
...@@ -223,33 +214,6 @@ class WebRtcTestBase : public InProcessBrowserTest { ...@@ -223,33 +214,6 @@ class WebRtcTestBase : public InProcessBrowserTest {
// Add 'usedtx=1' to the offer SDP. // Add 'usedtx=1' to the offer SDP.
void EnableOpusDtx(content::WebContents* tab) const; void EnableOpusDtx(content::WebContents* tab) const;
void CreateAndAddStreams(content::WebContents* tab, size_t count) const;
void VerifyRtpSenders(content::WebContents* tab,
base::Optional<size_t> expected_num_tracks =
base::Optional<size_t>()) const;
void VerifyRtpReceivers(content::WebContents* tab,
base::Optional<size_t> expected_num_tracks =
base::Optional<size_t>()) const;
std::vector<std::string> CreateAndAddAudioAndVideoTrack(
content::WebContents* tab,
StreamArgumentType stream_argument_type) const;
void RemoveTrack(content::WebContents* tab,
const std::string& track_id) const;
bool HasLocalStreamWithTrack(content::WebContents* tab,
const std::string& stream_id,
const std::string& track_id) const;
bool HasRemoteStreamWithTrack(content::WebContents* tab,
const std::string& stream_id,
const std::string& track_id) const;
bool HasSenderWithTrack(content::WebContents* tab,
std::string track_id) const;
bool HasReceiverWithTrack(content::WebContents* tab,
std::string track_id) const;
size_t GetNegotiationNeededCount(content::WebContents* tab) const;
std::vector<TrackEvent> GetTrackEvents(content::WebContents* tab) const;
// Performs garbage collection with "gc()". Requires command line switch
// |kJavaScriptFlags| with "--expose-gc".
void CollectGarbage(content::WebContents* tab) const;
// Try to open a dekstop media stream, and return the stream id. // Try to open a dekstop media stream, and return the stream id.
// On failure, will return empty string. // On failure, will return empty string.
std::string GetDesktopMediaStream(content::WebContents* tab); std::string GetDesktopMediaStream(content::WebContents* tab);
......
This diff is collapsed.
...@@ -594,7 +594,6 @@ test("browser_tests") { ...@@ -594,7 +594,6 @@ test("browser_tests") {
"../browser/media/webrtc/webrtc_getmediadevices_browsertest.cc", "../browser/media/webrtc/webrtc_getmediadevices_browsertest.cc",
"../browser/media/webrtc/webrtc_internals_integration_browsertest.cc", "../browser/media/webrtc/webrtc_internals_integration_browsertest.cc",
"../browser/media/webrtc/webrtc_internals_perf_browsertest.cc", "../browser/media/webrtc/webrtc_internals_perf_browsertest.cc",
"../browser/media/webrtc/webrtc_rtp_browsertest.cc",
"../browser/media/webrtc/webrtc_simulcast_browsertest.cc", "../browser/media/webrtc/webrtc_simulcast_browsertest.cc",
"../browser/media/webrtc/webrtc_stats_perf_browsertest.cc", "../browser/media/webrtc/webrtc_stats_perf_browsertest.cc",
"../browser/media/webrtc/webrtc_video_display_perf_browsertest.cc", "../browser/media/webrtc/webrtc_video_display_perf_browsertest.cc",
......
This diff is collapsed.
...@@ -10,7 +10,6 @@ ...@@ -10,7 +10,6 @@
<script type="text/javascript" src="media_devices.js"></script> <script type="text/javascript" src="media_devices.js"></script>
<script type="text/javascript" src="indexeddb.js"></script> <script type="text/javascript" src="indexeddb.js"></script>
<script type="text/javascript" src="peerconnection_getstats.js"></script> <script type="text/javascript" src="peerconnection_getstats.js"></script>
<script type="text/javascript" src="peerconnection_rtp.js"></script>
<script type="text/javascript" src="peerconnection_replacetrack.js"></script> <script type="text/javascript" src="peerconnection_replacetrack.js"></script>
</head> </head>
<body> <body>
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment