From ce73ec4a9a081350d240c403c89512c309219e41 Mon Sep 17 00:00:00 2001 From: Artem Titov Date: Fri, 12 Jun 2020 08:42:34 +0000 Subject: [PATCH] Revert "Generalize NetworkQualityMetricsReporter to support multiple peers in test" This reverts commit 33c0c342f60b4365b2c7773c73ae489d4e32149b. Reason for revert: Break packet loss metric Original change's description: > Generalize NetworkQualityMetricsReporter to support multiple peers in test > > Bug: webrtc:11479 > Change-Id: I80a6633b0edbb02274aff1f3a596908ee6a7497e > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/177008 > Reviewed-by: Andrey Logvin > Commit-Queue: Artem Titov > Cr-Commit-Position: refs/heads/master@{#31506} TBR=titovartem@webrtc.org,landrey@webrtc.org Change-Id: Ic428e8a7e016bcbfd35f8fca8468ed26f58e5800 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: webrtc:11479 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/177010 Reviewed-by: Artem Titov Commit-Queue: Artem Titov Cr-Commit-Position: refs/heads/master@{#31511} --- .../e2e/network_quality_metrics_reporter.cc | 30 ++++++++----------- .../pc/e2e/network_quality_metrics_reporter.h | 18 +++-------- 2 files changed, 16 insertions(+), 32 deletions(-) diff --git a/test/pc/e2e/network_quality_metrics_reporter.cc b/test/pc/e2e/network_quality_metrics_reporter.cc index 6cd5efc678..56f0337037 100644 --- a/test/pc/e2e/network_quality_metrics_reporter.cc +++ b/test/pc/e2e/network_quality_metrics_reporter.cc @@ -28,22 +28,15 @@ constexpr int kStatsWaitTimeoutMs = 1000; constexpr char kUseStandardBytesStats[] = "WebRTC-UseStandardBytesStats"; } -NetworkQualityMetricsReporter::NetworkQualityMetricsReporter( - EmulatedNetworkManagerInterface* alice_network, - EmulatedNetworkManagerInterface* bob_network) - : networks_by_peer_({{"alice", alice_network}, {"bob", bob_network}}) {} - void NetworkQualityMetricsReporter::Start(absl::string_view test_case_name) { test_case_name_ = std::string(test_case_name); // Check that network stats are clean before test execution. - for (const auto& entry : networks_by_peer_) { - EmulatedNetworkStats stats = PopulateStats(entry.second); - RTC_CHECK_EQ(stats.packets_sent, 0) - << "|packets_sent| for " << entry.first << " have to be 0 before test"; - RTC_CHECK_EQ(stats.packets_received, 0) - << "|packets_received| for " << entry.first - << " have to be 0 before test"; - } + EmulatedNetworkStats alice_stats = PopulateStats(alice_network_); + RTC_CHECK_EQ(alice_stats.packets_sent, 0); + RTC_CHECK_EQ(alice_stats.packets_received, 0); + EmulatedNetworkStats bob_stats = PopulateStats(bob_network_); + RTC_CHECK_EQ(bob_stats.packets_sent, 0); + RTC_CHECK_EQ(bob_stats.packets_received, 0); } void NetworkQualityMetricsReporter::OnStatsReports( @@ -72,11 +65,12 @@ void NetworkQualityMetricsReporter::OnStatsReports( } void NetworkQualityMetricsReporter::StopAndReportResults() { - for (const auto& entry : networks_by_peer_) { - EmulatedNetworkStats stats = PopulateStats(entry.second); - ReportStats(entry.first, stats, - stats.packets_sent - stats.packets_received); - } + EmulatedNetworkStats alice_stats = PopulateStats(alice_network_); + EmulatedNetworkStats bob_stats = PopulateStats(bob_network_); + ReportStats("alice", alice_stats, + alice_stats.packets_sent - bob_stats.packets_received); + ReportStats("bob", bob_stats, + bob_stats.packets_sent - alice_stats.packets_received); if (!webrtc::field_trial::IsEnabled(kUseStandardBytesStats)) { RTC_LOG(LS_ERROR) diff --git a/test/pc/e2e/network_quality_metrics_reporter.h b/test/pc/e2e/network_quality_metrics_reporter.h index abd89b9449..6454f17526 100644 --- a/test/pc/e2e/network_quality_metrics_reporter.h +++ b/test/pc/e2e/network_quality_metrics_reporter.h @@ -11,7 +11,6 @@ #ifndef TEST_PC_E2E_NETWORK_QUALITY_METRICS_REPORTER_H_ #define TEST_PC_E2E_NETWORK_QUALITY_METRICS_REPORTER_H_ -#include #include #include "api/test/network_emulation_manager.h" @@ -24,19 +23,9 @@ namespace webrtc_pc_e2e { class NetworkQualityMetricsReporter : public PeerConnectionE2EQualityTestFixture::QualityMetricsReporter { public: - // Creates a network quality metrics reporter on specified - // EmulatedNetworkManagerInterface instances index by the labels. These labels - // will be used as prefix for the metric name. - // Instances of |EmulatedNetworkManagerInterface*| have to outlive - // NetworkQualityMetricsReporter. - explicit NetworkQualityMetricsReporter( - std::map networks_by_peer) - : networks_by_peer_(std::move(networks_by_peer)) {} - // Creates a network quality metrics reporter on specified for two network - // which will be labeled "alice" and "bob" respectively. Bot |alice_network| - // and |bob_network| have to outlive NetworkQualityMetricsReporter. NetworkQualityMetricsReporter(EmulatedNetworkManagerInterface* alice_network, - EmulatedNetworkManagerInterface* bob_network); + EmulatedNetworkManagerInterface* bob_network) + : alice_network_(alice_network), bob_network_(bob_network) {} ~NetworkQualityMetricsReporter() override = default; // Network stats must be empty when this method will be invoked. @@ -67,7 +56,8 @@ class NetworkQualityMetricsReporter std::string test_case_name_; - std::map networks_by_peer_; + EmulatedNetworkManagerInterface* alice_network_; + EmulatedNetworkManagerInterface* bob_network_; rtc::CriticalSection lock_; std::map pc_stats_ RTC_GUARDED_BY(lock_); };