diff --git a/api/BUILD.gn b/api/BUILD.gn index 9cab2b0f5b..02dbe8da60 100644 --- a/api/BUILD.gn +++ b/api/BUILD.gn @@ -427,10 +427,23 @@ rtc_library("rtc_error") { "../rtc_base:logging", "../rtc_base:macromagic", "../rtc_base/system:rtc_export", + "//third_party/abseil-cpp/absl/meta:type_traits", + "//third_party/abseil-cpp/absl/strings", + "//third_party/abseil-cpp/absl/strings:str_format", "//third_party/abseil-cpp/absl/strings:string_view", ] } +rtc_source_set("rtc_error_matchers") { + testonly = true + sources = [ "test/rtc_error_matchers.h" ] + deps = [ + ":rtc_error", + "../test:test_support", + "//third_party/abseil-cpp/absl/strings", + ] +} + rtc_source_set("packet_socket_factory") { visibility = [ "*" ] sources = [ "packet_socket_factory.h" ] diff --git a/api/DEPS b/api/DEPS index a03410297b..e4c17a3a39 100644 --- a/api/DEPS +++ b/api/DEPS @@ -135,6 +135,8 @@ specific_include_rules = { "rtc_error\.h": [ "+rtc_base/logging.h", + "+absl/strings/has_absl_stringify.h", + "+absl/strings/str_format.h", ], "rtc_event_log_output_file.h": [ # For private member and constructor. diff --git a/api/rtc_error.h b/api/rtc_error.h index 9de3b1c649..775f6de038 100644 --- a/api/rtc_error.h +++ b/api/rtc_error.h @@ -15,8 +15,10 @@ #include #include +#include #include // For std::move. +#include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" @@ -95,6 +97,14 @@ enum class RTCErrorDetailType { HARDWARE_ENCODER_ERROR, }; +// Outputs the error as a friendly string. Update this method when adding a new +// error type. +// +// Only intended to be used for logging/diagnostics. The returned char* points +// to literal strings that live for the whole duration of the program. +RTC_EXPORT absl::string_view ToString(RTCErrorType error); +RTC_EXPORT absl::string_view ToString(RTCErrorDetailType error); + // Roughly corresponds to RTCError in the web api. Holds an error type, a // message, and possibly additional information specific to that error. // @@ -146,6 +156,16 @@ class RTC_EXPORT RTCError { // error occurred. bool ok() const { return type_ == RTCErrorType::NONE; } + template + friend void AbslStringify(Sink& sink, const RTCError& error) { + sink.Append(ToString(error.type_)); + if (!error.message_.empty()) { + sink.Append(" with message: \""); + sink.Append(error.message_); + sink.Append("\""); + } + } + private: RTCErrorType type_ = RTCErrorType::NONE; std::string message_; @@ -153,14 +173,6 @@ class RTC_EXPORT RTCError { std::optional sctp_cause_code_; }; -// Outputs the error as a friendly string. Update this method when adding a new -// error type. -// -// Only intended to be used for logging/diagnostics. The returned char* points -// to literal string that lives for the whole duration of the program. -RTC_EXPORT absl::string_view ToString(RTCErrorType error); -RTC_EXPORT absl::string_view ToString(RTCErrorDetailType error); - // Helper macro that can be used by implementations to create an error with a // message and log it. `message` should be a string literal or movable // std::string. @@ -307,6 +319,19 @@ class RTCErrorOr { return std::move(*value_); } + template + friend void AbslStringify(Sink& sink, const RTCErrorOr& error_or) { + if (error_or.ok()) { + sink.Append("OK"); + if constexpr (std::is_convertible_v) { + sink.Append(" with value: "); + sink.Append(absl::StrCat(error_or.value())); + } + } else { + sink.Append(absl::StrCat(error_or.error())); + } + } + private: RTCError error_; std::optional value_; diff --git a/api/rtc_error_unittest.cc b/api/rtc_error_unittest.cc index 68ca2f5a51..860f3ead05 100644 --- a/api/rtc_error_unittest.cc +++ b/api/rtc_error_unittest.cc @@ -13,6 +13,7 @@ #include #include +#include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" #include "rtc_base/checks.h" #include "test/gtest.h" @@ -147,6 +148,11 @@ TEST(RTCErrorTest, SetMessage) { EXPECT_STREQ(e.message(), "string"); } +TEST(RTCErrorTest, Stringify) { + RTCError e(RTCErrorType::INVALID_PARAMETER, "foo"); + EXPECT_EQ(absl::StrCat(e), "INVALID_PARAMETER with message: \"foo\""); +} + // Test that the default constructor creates an "INTERNAL_ERROR". TEST(RTCErrorOrTest, DefaultConstructor) { RTCErrorOr e; @@ -212,6 +218,26 @@ TEST(RTCErrorOrTest, MoveValue) { EXPECT_EQ(value.value, 88); } +TEST(RTCErrorOrTest, StringifyWithUnprintableValue) { + RTCErrorOr e(MoveOnlyInt(1337)); + EXPECT_EQ(absl::StrCat(e), "OK"); +} + +TEST(RTCErrorOrTest, StringifyWithStringValue) { + RTCErrorOr e("foo"); + EXPECT_EQ(absl::StrCat(e), "OK with value: foo"); +} + +TEST(RTCErrorOrTest, StringifyWithPrintableValue) { + RTCErrorOr e(1337); + EXPECT_EQ(absl::StrCat(e), "OK with value: 1337"); +} + +TEST(RTCErrorOrTest, StringifyWithError) { + RTCErrorOr e({RTCErrorType::SYNTAX_ERROR, "message"}); + EXPECT_EQ(absl::StrCat(e), "SYNTAX_ERROR with message: \"message\""); +} + // Death tests. // Disabled on Android because death tests misbehave on Android, see // base/test/gtest_util.h. diff --git a/api/test/DEPS b/api/test/DEPS index 5506d6c76a..4a51584c86 100644 --- a/api/test/DEPS +++ b/api/test/DEPS @@ -56,4 +56,7 @@ specific_include_rules = { "videocodec_test_fixture\.h": [ "+modules/video_coding/codecs/h264/include/h264_globals.h", ], + "rtc_error_matchers\.h": [ + "+test/gmock.h", + ], } diff --git a/api/test/rtc_error_matchers.h b/api/test/rtc_error_matchers.h new file mode 100644 index 0000000000..cf923b28fa --- /dev/null +++ b/api/test/rtc_error_matchers.h @@ -0,0 +1,60 @@ +/* + * Copyright 2024 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#ifndef API_TEST_RTC_ERROR_MATCHERS_H_ +#define API_TEST_RTC_ERROR_MATCHERS_H_ + +#include + +#include "absl/strings/str_cat.h" +#include "api/rtc_error.h" +#include "test/gmock.h" + +namespace webrtc { + +MATCHER(IsRtcOk, "") { + if (!arg.ok()) { + *result_listener << "Expected OK, got " << absl::StrCat(arg); + return false; + } + return true; +} + +MATCHER_P(IsRtcOkAndHolds, + matcher, + "RtcErrorOr that is holding an OK status and ") { + if (!arg.ok()) { + *result_listener << "Expected OK, got " << absl::StrCat(arg); + return false; + } + return testing::ExplainMatchResult(matcher, arg.value(), result_listener); +} + +MATCHER_P2(IsRtcErrorWithMessage, + error_matcher, + message_matcher, + "RtcErrorOr that is holding an error that " + + testing::DescribeMatcher(error_matcher, negation) + + (negation ? " or " : " and ") + " with a message that " + + testing::DescribeMatcher(message_matcher, + negation)) { + if (arg.ok()) { + *result_listener << "Expected error, got " << absl::StrCat(arg); + return false; + } + return testing::ExplainMatchResult(error_matcher, arg.error(), + result_listener) && + testing::ExplainMatchResult(message_matcher, arg.error().message(), + result_listener); +} + +} // namespace webrtc + +#endif // API_TEST_RTC_ERROR_MATCHERS_H_ diff --git a/pc/BUILD.gn b/pc/BUILD.gn index 3dbc6b6583..719334951a 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -2337,6 +2337,7 @@ if (rtc_include_tests && !build_with_chromium) { "../api:packet_socket_factory", "../api:priority", "../api:rtc_error", + "../api:rtc_error_matchers", "../api:rtp_sender_interface", "../api:rtp_transceiver_direction", "../api:scoped_refptr", @@ -2432,6 +2433,7 @@ if (rtc_include_tests && !build_with_chromium) { "../test:rtc_expect_death", "../test:run_loop", "../test:scoped_key_value_config", + "../test:wait_until", "../test/pc/sctp:fake_sctp_transport", "//testing/gtest", "//third_party/abseil-cpp/absl/algorithm:container", diff --git a/pc/peer_connection_encodings_integrationtest.cc b/pc/peer_connection_encodings_integrationtest.cc index e8c05fc1c8..b6a8b08d79 100644 --- a/pc/peer_connection_encodings_integrationtest.cc +++ b/pc/peer_connection_encodings_integrationtest.cc @@ -38,6 +38,7 @@ #include "api/scoped_refptr.h" #include "api/stats/rtc_stats_report.h" #include "api/stats/rtcstats_objects.h" +#include "api/test/rtc_error_matchers.h" #include "api/units/data_rate.h" #include "api/units/time_delta.h" #include "api/video/resolution.h" @@ -55,6 +56,7 @@ #include "test/gmock.h" #include "test/gtest.h" #include "test/scoped_key_value_config.h" +#include "test/wait_until.h" using ::testing::AllOf; using ::testing::Each; @@ -72,36 +74,6 @@ namespace { constexpr TimeDelta kDefaultTimeout = TimeDelta::Seconds(5); -MATCHER(IsRtcOk, "") { - if (!arg.ok()) { - *result_listener << "Expected OK, got " << arg.error().message(); - return false; - } - return true; -} - -template -auto WaitForCondition(const Fn& fn, - Matcher matcher, - TimeDelta timeout = kDefaultTimeout) - -> RTCErrorOr { - testing::StringMatchResultListener listener; - int64_t start = rtc::SystemTimeMillis(); - auto result = fn(); - if (testing::ExplainMatchResult(matcher, result, &listener)) { - return result; - } - do { - rtc::Thread::Current()->ProcessMessages(0); - rtc::Thread::Current()->SleepMs(1); - result = fn(); - if (testing::ExplainMatchResult(matcher, result, &listener)) { - return result; - } - } while (rtc::SystemTimeMillis() < start + timeout.ms()); - return RTCError(RTCErrorType::INTERNAL_ERROR, listener.str()); -} - // Most tests pass in 20-30 seconds, but some tests take longer such as AV1 // requiring additional ramp-up time (https://crbug.com/webrtc/15006) or SVC // (LxTx_KEY) being slower than simulcast to send top spatial layer. @@ -993,7 +965,7 @@ TEST_F(PeerConnectionEncodingsIntegrationTest, // Since the standard API is configuring simulcast we get three outbound-rtps, // and two are active. ASSERT_THAT( - WaitForCondition( + WaitUntil( [&] { std::vector outbound_rtps = GetStats(local_pc_wrapper) @@ -1005,7 +977,7 @@ TEST_F(PeerConnectionEncodingsIntegrationTest, } return bytes_sent; }, - AllOf(SizeIs(3), Each(Gt(0))), kLongTimeoutForRampingUp), + AllOf(SizeIs(3), Each(Gt(0))), {.timeout = kLongTimeoutForRampingUp}), IsRtcOk()); rtc::scoped_refptr report = GetStats(local_pc_wrapper); ASSERT_TRUE(report); @@ -1031,7 +1003,7 @@ TEST_F(PeerConnectionEncodingsIntegrationTest, // Deactivate the active layer. parameters.encodings[2].active = false; EXPECT_TRUE(sender->SetParameters(parameters).ok()); - ASSERT_THAT(WaitForCondition( + ASSERT_THAT(WaitUntil( [&]() { return GetStats(local_pc_wrapper) ->GetStatsOfType(); diff --git a/test/BUILD.gn b/test/BUILD.gn index d5a5314f0e..09bfbf8785 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -709,6 +709,7 @@ if (rtc_include_tests) { ":video_frame_writer", ":video_test_common", ":video_test_support", + ":wait_until", ":y4m_frame_generator", "../api:array_view", "../api:create_frame_generator", @@ -717,6 +718,8 @@ if (rtc_include_tests) { "../api:mock_video_codec_factory", "../api:mock_video_decoder", "../api:mock_video_encoder", + "../api:rtc_error", + "../api:rtc_error_matchers", "../api:scoped_refptr", "../api:simulcast_test_fixture_api", "../api/environment", @@ -729,6 +732,7 @@ if (rtc_include_tests) { "../api/units:data_size", "../api/units:frequency", "../api/units:time_delta", + "../api/units:timestamp", "../api/video:encoded_image", "../api/video:video_frame", "../api/video_codecs:builtin_video_decoder_factory", @@ -749,8 +753,10 @@ if (rtc_include_tests) { "../modules/video_coding/svc:scalability_mode_util", "../rtc_base:criticalsection", "../rtc_base:rtc_event", + "../rtc_base:threading", "../rtc_base/synchronization:mutex", "../rtc_base/system:file_wrapper", + "../system_wrappers", "jitter:jitter_unittests", "pc/e2e:e2e_unittests", "pc/e2e/analyzer/video:video_analyzer_unittests", @@ -782,6 +788,7 @@ if (rtc_include_tests) { "testsupport/yuv_frame_reader_unittest.cc", "testsupport/yuv_frame_writer_unittest.cc", "video_codec_tester_unittest.cc", + "wait_until_unittest.cc", ] if (rtc_enable_protobuf) { @@ -1385,3 +1392,22 @@ rtc_library("video_codec_tester") { "//third_party/libyuv", ] } + +rtc_source_set("wait_until") { + testonly = true + sources = [ + "wait_until.h", + "wait_until_internal.h", + ] + deps = [ + ":test_support", + "../api:rtc_error", + "../api/units:time_delta", + "../api/units:timestamp", + "../rtc_base:checks", + "../rtc_base:threading", + "../system_wrappers", + "//third_party/abseil-cpp/absl/base:nullability", + "//third_party/abseil-cpp/absl/strings:string_view", + ] +} diff --git a/test/wait_until.h b/test/wait_until.h new file mode 100644 index 0000000000..a8f7ac2a9c --- /dev/null +++ b/test/wait_until.h @@ -0,0 +1,92 @@ +/* + * Copyright 2024 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#ifndef TEST_WAIT_UNTIL_H_ +#define TEST_WAIT_UNTIL_H_ + +#include + +#include "absl/base/nullability.h" +#include "api/rtc_error.h" +#include "api/units/time_delta.h" +#include "api/units/timestamp.h" +#include "rtc_base/checks.h" +#include "rtc_base/thread.h" +#include "system_wrappers/include/clock.h" +#include "test/gmock.h" +#include "test/wait_until_internal.h" // IWYU pragma: private + +namespace webrtc { + +struct WaitUntilSettings { + // The maximum time to wait for the condition to be met. + TimeDelta timeout = TimeDelta::Seconds(5); + // The interval between polling the condition. + TimeDelta polling_interval = TimeDelta::Millis(1); + // The clock to use for timing. + absl::Nullable clock = nullptr; + + // Name of the result to be used in the error message. + std::string result_name = "result"; +}; + +// Runs a function `fn`, which returns a result, until `matcher` matches the +// result. +// +// The function is called repeatedly until the result matches the matcher or the +// timeout is reached. If the matcher matches the result, the result is +// returned. Otherwise, an error is returned. +// +// Example: +// +// int counter = 0; +// RTCErrorOr result = Waituntil([&] { return ++counter; }, Eq(3)) +// EXPECT_THAT(result, IsOkAndHolds(3)); +template +auto WaitUntil(const Fn& fn, Matcher matcher, WaitUntilSettings settings = {}) + -> RTCErrorOr { + if (!settings.clock) { + RTC_CHECK(rtc::Thread::Current()) << "A current thread is required. An " + "rtc::AutoThread can work for tests."; + } + + absl::Nonnull clock = + settings.clock ? settings.clock : Clock::GetRealTimeClock(); + + Timestamp start = clock->CurrentTime(); + + do { + auto result = fn(); + if (testing::Value(result, matcher)) { + return result; + } + if (settings.clock) { + settings.clock->AdvanceTime(settings.polling_interval); + } else { + rtc::Thread::Current()->ProcessMessages(0); + rtc::Thread::Current()->SleepMs(settings.polling_interval.ms()); + } + } while (clock->CurrentTime() < start + settings.timeout); + + // One more try after the last sleep. This failure will contain the error + // message. + auto result = fn(); + testing::StringMatchResultListener listener; + if (wait_until_internal::ExplainMatchResult(matcher, result, &listener, + settings.result_name)) { + return result; + } + + return RTCError(RTCErrorType::INTERNAL_ERROR, listener.str()); +} + +} // namespace webrtc + +#endif // TEST_WAIT_UNTIL_H_ diff --git a/test/wait_until_internal.h b/test/wait_until_internal.h new file mode 100644 index 0000000000..5065e965b3 --- /dev/null +++ b/test/wait_until_internal.h @@ -0,0 +1,59 @@ +/* + * Copyright 2024 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#ifndef TEST_WAIT_UNTIL_INTERNAL_H_ +#define TEST_WAIT_UNTIL_INTERNAL_H_ + +#include + +#include "absl/base/nullability.h" +#include "absl/strings/string_view.h" +#include "test/gmock.h" +#include "test/gtest.h" + +namespace webrtc { +namespace wait_until_internal { + +// Explains the match result of `matcher` against `value` to `listener`. +// `value_name` is the name of the value to be used in the error message. +// This is inspired by testing::ExplainMatchResult and +// testing::internal::MatchPrintAndExplain. +template +bool ExplainMatchResult( + const M& matcher, + const T& value, + absl::Nonnull listener, + absl::string_view value_name) { + // SafeMatcherCast is required for matchers whose type does not match the + // argument type. + testing::Matcher safe_matcher = + testing::SafeMatcherCast(matcher); + + auto* ss = listener->stream(); + *ss << "Value of: " << value_name << "\n"; + *ss << "Expected: "; + safe_matcher.DescribeTo(ss); + *ss << "\nActual: "; + testing::StringMatchResultListener inner_listener; + if (testing::ExplainMatchResult(safe_matcher, value, &inner_listener)) { + return true; + } + *ss << testing::PrintToString(value); + if (const std::string& inner_message = inner_listener.str(); + !inner_message.empty()) { + *ss << ", " << inner_message; + } + return false; +} + +} // namespace wait_until_internal +} // namespace webrtc + +#endif // TEST_WAIT_UNTIL_INTERNAL_H_ diff --git a/test/wait_until_unittest.cc b/test/wait_until_unittest.cc new file mode 100644 index 0000000000..96fec94a15 --- /dev/null +++ b/test/wait_until_unittest.cc @@ -0,0 +1,87 @@ +/* + * Copyright 2024 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include "test/wait_until.h" + +#include "api/rtc_error.h" +#include "api/test/rtc_error_matchers.h" +#include "api/units/time_delta.h" +#include "api/units/timestamp.h" +#include "rtc_base/thread.h" +#include "system_wrappers/include/clock.h" +#include "test/gmock.h" +#include "test/gtest.h" + +namespace webrtc { +namespace { + +using testing::_; +using testing::AllOf; +using testing::Eq; +using testing::Ge; +using testing::Gt; +using testing::Lt; +using testing::MatchesRegex; + +TEST(WaitUntilTest, ReturnsWhenConditionIsMet) { + rtc::AutoThread thread; + + int counter = 0; + RTCErrorOr result = WaitUntil([&] { return ++counter; }, Eq(3)); + EXPECT_THAT(result, IsRtcOkAndHolds(3)); +} + +TEST(WaitUntilTest, ReturnsErrorWhenTimeoutIsReached) { + rtc::AutoThread thread; + int counter = 0; + RTCErrorOr result = + WaitUntil([&] { return --counter; }, Eq(1), + {.timeout = TimeDelta::Millis(10), .result_name = "counter"}); + // Only returns the last error. Note we only are checking that the error + // message ends with a negative number rather than a specific number to avoid + // flakiness. + EXPECT_THAT( + result, + IsRtcErrorWithMessage( + _, MatchesRegex( + "Value of: counter\nExpected: is equal to 1\nActual: -\\d+"))); +} + +TEST(WaitUntilTest, ErrorContainsMatcherExplanation) { + rtc::AutoThread thread; + int counter = 0; + auto matcher = AllOf(Gt(0), Lt(10)); + RTCErrorOr result = + WaitUntil([&] { return --counter; }, matcher, + {.timeout = TimeDelta::Millis(10), .result_name = "counter"}); + // Only returns the last error. Note we only are checking that the error + // message ends with a negative number rather than a specific number to avoid + // flakiness. + EXPECT_THAT( + result, + IsRtcErrorWithMessage( + _, MatchesRegex("Value of: counter\nExpected: \\(is > 0\\) and " + "\\(is < 10\\)\nActual: -\\d+, which doesn't match " + "\\(is > 0\\)"))); +} + +TEST(WaitUntilTest, ReturnsWhenConditionIsMetWithSimulatedClock) { + SimulatedClock fake_clock = SimulatedClock(Timestamp::Millis(1337)); + + int counter = 0; + RTCErrorOr result = + WaitUntil([&] { return ++counter; }, Eq(3), {.clock = &fake_clock}); + EXPECT_THAT(result, IsRtcOkAndHolds(3)); + // The fake clock should have advanced at least 2ms. + EXPECT_THAT(fake_clock.CurrentTime(), Ge(Timestamp::Millis(1339))); +} + +} // namespace +} // namespace webrtc