Add metrics::Samples to facilitate easier testing
Currently, tests that verify metrics use a combination of
metrics::NumSamples and metrics::NumEvents to assert which samples
were recorded and how many times they were recorded. This means
that a comprehensive tests has n + 1 assertions for n distinct
samples.
The new metrics::Samples function returns a map of sample --> num
events which can be asserted against using gmock matchers,
achieving better coverage and better test failure messages in just
one line.
Bug: None
Change-Id: I07d4a766654cfc04e414b77b6de02927683a361f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/125486
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26974}diff --git a/pc/peer_connection_rtp_unittest.cc b/pc/peer_connection_rtp_unittest.cc
index cf00138..48c14e3 100644
--- a/pc/peer_connection_rtp_unittest.cc
+++ b/pc/peer_connection_rtp_unittest.cc
@@ -64,6 +64,7 @@
using RTCConfiguration = PeerConnectionInterface::RTCConfiguration;
using ::testing::ElementsAre;
+using ::testing::Pair;
using ::testing::UnorderedElementsAre;
using ::testing::Values;
@@ -1606,11 +1607,8 @@
EXPECT_EQ(cricket::kMsidSignalingMediaSection,
answer->description()->msid_signaling());
// Check that this is counted correctly
- EXPECT_EQ(2, webrtc::metrics::NumSamples(
- "WebRTC.PeerConnection.SdpSemanticNegotiated"));
- EXPECT_EQ(2, webrtc::metrics::NumEvents(
- "WebRTC.PeerConnection.SdpSemanticNegotiated",
- kSdpSemanticNegotiatedUnifiedPlan));
+ EXPECT_THAT(metrics::Samples("WebRTC.PeerConnection.SdpSemanticNegotiated"),
+ ElementsAre(Pair(kSdpSemanticNegotiatedUnifiedPlan, 2)));
}
TEST_F(PeerConnectionMsidSignalingTest, PlanBOfferToUnifiedPlanAnswer) {
@@ -1698,11 +1696,8 @@
ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
// Note that only the callee does ReportSdpFormatReceived.
- EXPECT_EQ(1, webrtc::metrics::NumSamples(
- "WebRTC.PeerConnection.SdpFormatReceived"));
- EXPECT_EQ(
- 1, webrtc::metrics::NumEvents("WebRTC.PeerConnection.SdpFormatReceived",
- kSdpFormatReceivedNoTracks));
+ EXPECT_THAT(metrics::Samples("WebRTC.PeerConnection.SdpFormatReceived"),
+ ElementsAre(Pair(kSdpFormatReceivedNoTracks, 1)));
}
#endif // HAVE_SCTP
@@ -1714,11 +1709,8 @@
ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
// Note that only the callee does ReportSdpFormatReceived.
- EXPECT_EQ(1, webrtc::metrics::NumSamples(
- "WebRTC.PeerConnection.SdpFormatReceived"));
- EXPECT_EQ(
- 1, webrtc::metrics::NumEvents("WebRTC.PeerConnection.SdpFormatReceived",
- kSdpFormatReceivedSimple));
+ EXPECT_THAT(metrics::Samples("WebRTC.PeerConnection.SdpFormatReceived"),
+ ElementsAre(Pair(kSdpFormatReceivedSimple, 1)));
}
TEST_F(SdpFormatReceivedTest, SimplePlanBIsReportedAsSimple) {
@@ -1727,12 +1719,9 @@
auto callee = CreatePeerConnectionWithUnifiedPlan();
ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
-
- EXPECT_EQ(1, webrtc::metrics::NumSamples(
- "WebRTC.PeerConnection.SdpFormatReceived"));
- EXPECT_EQ(
- 1, webrtc::metrics::NumEvents("WebRTC.PeerConnection.SdpFormatReceived",
- kSdpFormatReceivedSimple));
+ // Note that only the callee does ReportSdpFormatReceived.
+ EXPECT_THAT(metrics::Samples("WebRTC.PeerConnection.SdpFormatReceived"),
+ ElementsAre(Pair(kSdpFormatReceivedSimple, 1)));
}
TEST_F(SdpFormatReceivedTest, ComplexUnifiedIsReportedAsComplexUnifiedPlan) {
@@ -1744,11 +1733,8 @@
ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
// Note that only the callee does ReportSdpFormatReceived.
- EXPECT_EQ(1, webrtc::metrics::NumSamples(
- "WebRTC.PeerConnection.SdpFormatReceived"));
- EXPECT_EQ(
- 1, webrtc::metrics::NumEvents("WebRTC.PeerConnection.SdpFormatReceived",
- kSdpFormatReceivedComplexUnifiedPlan));
+ EXPECT_THAT(metrics::Samples("WebRTC.PeerConnection.SdpFormatReceived"),
+ ElementsAre(Pair(kSdpFormatReceivedComplexUnifiedPlan, 1)));
}
TEST_F(SdpFormatReceivedTest, ComplexPlanBIsReportedAsComplexPlanB) {
@@ -1762,11 +1748,8 @@
// SDP Format to be recorded.
ASSERT_FALSE(callee->SetRemoteDescription(caller->CreateOffer()));
// Note that only the callee does ReportSdpFormatReceived.
- EXPECT_EQ(1, webrtc::metrics::NumSamples(
- "WebRTC.PeerConnection.SdpFormatReceived"));
- EXPECT_EQ(
- 1, webrtc::metrics::NumEvents("WebRTC.PeerConnection.SdpFormatReceived",
- kSdpFormatReceivedComplexPlanB));
+ EXPECT_THAT(metrics::Samples("WebRTC.PeerConnection.SdpFormatReceived"),
+ ElementsAre(Pair(kSdpFormatReceivedComplexPlanB, 1)));
}
// Sender setups in a call.
diff --git a/system_wrappers/include/metrics.h b/system_wrappers/include/metrics.h
index bad0f65..62dc6c6 100644
--- a/system_wrappers/include/metrics.h
+++ b/system_wrappers/include/metrics.h
@@ -309,6 +309,10 @@
// Returns the minimum sample value (or -1 if the histogram has no samples).
int MinSample(const std::string& name);
+// Returns a map with keys the samples with at least one event and values the
+// number of events for that sample.
+std::map<int, int> Samples(const std::string& name);
+
} // namespace metrics
} // namespace webrtc
diff --git a/system_wrappers/source/metrics.cc b/system_wrappers/source/metrics.cc
index f98b4cd..2383272 100644
--- a/system_wrappers/source/metrics.cc
+++ b/system_wrappers/source/metrics.cc
@@ -88,6 +88,11 @@
return (info_.samples.empty()) ? -1 : info_.samples.begin()->first;
}
+ std::map<int, int> Samples() const {
+ rtc::CritScope cs(&crit_);
+ return info_.samples;
+ }
+
private:
rtc::CriticalSection crit_;
const int min_;
@@ -162,6 +167,12 @@
return (it == map_.end()) ? -1 : it->second->MinSample();
}
+ std::map<int, int> Samples(const std::string& name) const {
+ rtc::CritScope cs(&crit_);
+ const auto& it = map_.find(name);
+ return (it == map_.end()) ? std::map<int, int>() : it->second->Samples();
+ }
+
private:
rtc::CriticalSection crit_;
std::map<std::string, std::unique_ptr<RtcHistogram>> map_
@@ -307,5 +318,10 @@
return map ? map->MinSample(name) : -1;
}
+std::map<int, int> Samples(const std::string& name) {
+ RtcHistogramMap* map = GetMap();
+ return map ? map->Samples(name) : std::map<int, int>();
+}
+
} // namespace metrics
} // namespace webrtc
diff --git a/system_wrappers/source/metrics_unittest.cc b/system_wrappers/source/metrics_unittest.cc
index dac8177..9c96ba0 100644
--- a/system_wrappers/source/metrics_unittest.cc
+++ b/system_wrappers/source/metrics_unittest.cc
@@ -9,8 +9,13 @@
*/
#include "system_wrappers/include/metrics.h"
+#include "test/gmock.h"
#include "test/gtest.h"
+using ::testing::ElementsAre;
+using ::testing::IsEmpty;
+using ::testing::Pair;
+
namespace webrtc {
namespace {
const int kSample = 22;
@@ -34,6 +39,7 @@
TEST_F(MetricsTest, InitiallyNoSamples) {
EXPECT_EQ(0, metrics::NumSamples("NonExisting"));
EXPECT_EQ(0, metrics::NumEvents("NonExisting", kSample));
+ EXPECT_THAT(metrics::Samples("NonExisting"), IsEmpty());
}
TEST_F(MetricsTest, RtcHistogramPercent_AddSample) {
@@ -41,6 +47,7 @@
RTC_HISTOGRAM_PERCENTAGE(kName, kSample);
EXPECT_EQ(1, metrics::NumSamples(kName));
EXPECT_EQ(1, metrics::NumEvents(kName, kSample));
+ EXPECT_THAT(metrics::Samples(kName), ElementsAre(Pair(kSample, 1)));
}
TEST_F(MetricsTest, RtcHistogramEnumeration_AddSample) {
@@ -48,6 +55,7 @@
RTC_HISTOGRAM_ENUMERATION(kName, kSample, kSample + 1);
EXPECT_EQ(1, metrics::NumSamples(kName));
EXPECT_EQ(1, metrics::NumEvents(kName, kSample));
+ EXPECT_THAT(metrics::Samples(kName), ElementsAre(Pair(kSample, 1)));
}
TEST_F(MetricsTest, RtcHistogramBoolean_AddSample) {
@@ -56,6 +64,7 @@
RTC_HISTOGRAM_BOOLEAN(kName, kSample);
EXPECT_EQ(1, metrics::NumSamples(kName));
EXPECT_EQ(1, metrics::NumEvents(kName, kSample));
+ EXPECT_THAT(metrics::Samples(kName), ElementsAre(Pair(kSample, 1)));
}
TEST_F(MetricsTest, RtcHistogramCountsSparse_AddSample) {
@@ -63,6 +72,7 @@
RTC_HISTOGRAM_COUNTS_SPARSE_100(kName, kSample);
EXPECT_EQ(1, metrics::NumSamples(kName));
EXPECT_EQ(1, metrics::NumEvents(kName, kSample));
+ EXPECT_THAT(metrics::Samples(kName), ElementsAre(Pair(kSample, 1)));
}
TEST_F(MetricsTest, RtcHistogramCounts_AddSample) {
@@ -70,16 +80,20 @@
RTC_HISTOGRAM_COUNTS_100(kName, kSample);
EXPECT_EQ(1, metrics::NumSamples(kName));
EXPECT_EQ(1, metrics::NumEvents(kName, kSample));
+ EXPECT_THAT(metrics::Samples(kName), ElementsAre(Pair(kSample, 1)));
}
TEST_F(MetricsTest, RtcHistogramCounts_AddMultipleSamples) {
const std::string kName = "Counts200";
const int kNumSamples = 10;
+ std::map<int, int> samples;
for (int i = 1; i <= kNumSamples; ++i) {
RTC_HISTOGRAM_COUNTS_200(kName, i);
EXPECT_EQ(1, metrics::NumEvents(kName, i));
EXPECT_EQ(i, metrics::NumSamples(kName));
+ samples[i] = 1;
}
+ EXPECT_EQ(samples, metrics::Samples(kName));
}
TEST_F(MetricsTest, RtcHistogramsCounts_AddSample) {
@@ -92,6 +106,9 @@
EXPECT_EQ(1, metrics::NumEvents("Name1", kSample + 0));
EXPECT_EQ(1, metrics::NumEvents("Name2", kSample + 1));
EXPECT_EQ(1, metrics::NumEvents("Name3", kSample + 2));
+ EXPECT_THAT(metrics::Samples("Name1"), ElementsAre(Pair(kSample + 0, 1)));
+ EXPECT_THAT(metrics::Samples("Name2"), ElementsAre(Pair(kSample + 1, 1)));
+ EXPECT_THAT(metrics::Samples("Name3"), ElementsAre(Pair(kSample + 2, 1)));
}
#if RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
@@ -107,6 +124,8 @@
AddSparseSample("Sparse2", kSample);
EXPECT_EQ(1, metrics::NumSamples("Sparse1"));
EXPECT_EQ(1, metrics::NumSamples("Sparse2"));
+ EXPECT_THAT(metrics::Samples("Sparse1"), ElementsAre(Pair(kSample, 1)));
+ EXPECT_THAT(metrics::Samples("Sparse2"), ElementsAre(Pair(kSample, 1)));
}
} // namespace webrtc
