From 954e72b6542273a3b90fea7245114af577a992a2 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Wed, 10 Jul 2024 19:22:07 +0200 Subject: [PATCH] Update MockAudioEncoderFactory to override Create instead of MakeAudioEncoder MakeAudioEncoder planned to be removed, and Create planned to become pure virtual While at it, cleanup nearby mock usage: Remove ON_CALL that by default return default constructed result Remove EXPECT_CALL().Times(AnyNumber()) for a NiceMock Remove parameters in EXPECT_CALL when all are wildcard Remove redundant get to deference a smart pointer Bug: webrtc:343086059 Change-Id: Ica90a4980350cb82bcebd11df6c63a01b828bb9d Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/356884 Commit-Queue: Danil Chapovalov Reviewed-by: Artem Titov Reviewed-by: Gustaf Ullberg Cr-Commit-Position: refs/heads/main@{#42622} --- audio/audio_send_stream_unittest.cc | 71 ++++++++++++----------------- test/BUILD.gn | 1 + test/mock_audio_encoder_factory.h | 60 ++++-------------------- 3 files changed, 40 insertions(+), 92 deletions(-) diff --git a/audio/audio_send_stream_unittest.cc b/audio/audio_send_stream_unittest.cc index 7c74a115d4..45f5391373 100644 --- a/audio/audio_send_stream_unittest.cc +++ b/audio/audio_send_stream_unittest.cc @@ -51,6 +51,7 @@ using ::testing::Ne; using ::testing::NiceMock; using ::testing::Return; using ::testing::StrEq; +using ::testing::WithArg; static const float kTolerance = 0.0001f; @@ -100,21 +101,19 @@ class MockLimitObserver : public BitrateAllocator::LimitObserver { }; std::unique_ptr SetupAudioEncoderMock( - int payload_type, const SdpAudioFormat& format) { for (const auto& spec : kCodecSpecs) { if (format == spec.format) { - std::unique_ptr encoder( - new ::testing::NiceMock()); - ON_CALL(*encoder.get(), SampleRateHz()) + auto encoder = std::make_unique>(); + ON_CALL(*encoder, SampleRateHz) .WillByDefault(Return(spec.info.sample_rate_hz)); - ON_CALL(*encoder.get(), NumChannels()) + ON_CALL(*encoder, NumChannels) .WillByDefault(Return(spec.info.num_channels)); - ON_CALL(*encoder.get(), RtpTimestampRateHz()) + ON_CALL(*encoder, RtpTimestampRateHz) .WillByDefault(Return(spec.format.clockrate_hz)); - ON_CALL(*encoder.get(), GetFrameLengthRange()) - .WillByDefault(Return(absl::optional>{ - {TimeDelta::Millis(20), TimeDelta::Millis(120)}})); + ON_CALL(*encoder, GetFrameLengthRange) + .WillByDefault(Return( + std::make_pair(TimeDelta::Millis(20), TimeDelta::Millis(120)))); return encoder; } } @@ -124,11 +123,11 @@ std::unique_ptr SetupAudioEncoderMock( rtc::scoped_refptr SetupEncoderFactoryMock() { rtc::scoped_refptr factory = rtc::make_ref_counted(); - ON_CALL(*factory.get(), GetSupportedEncoders()) + ON_CALL(*factory, GetSupportedEncoders) .WillByDefault(Return(std::vector( std::begin(kCodecSpecs), std::end(kCodecSpecs)))); - ON_CALL(*factory.get(), QueryAudioEncoder(_)) - .WillByDefault(Invoke( + ON_CALL(*factory, QueryAudioEncoder) + .WillByDefault( [](const SdpAudioFormat& format) -> absl::optional { for (const auto& spec : kCodecSpecs) { if (format == spec.format) { @@ -136,13 +135,8 @@ rtc::scoped_refptr SetupEncoderFactoryMock() { } } return absl::nullopt; - })); - ON_CALL(*factory.get(), MakeAudioEncoderMock(_, _, _, _)) - .WillByDefault(Invoke([](int payload_type, const SdpAudioFormat& format, - absl::optional codec_pair_id, - std::unique_ptr* return_value) { - *return_value = SetupAudioEncoderMock(payload_type, format); - })); + }); + ON_CALL(*factory, Create).WillByDefault(WithArg<1>(&SetupAudioEncoderMock)); return factory; } @@ -518,19 +512,17 @@ TEST(AudioSendStreamTest, SendCodecAppliesAudioNetworkAdaptor) { helper.config().audio_network_adaptor_config = kAnaConfigString; - EXPECT_CALL(helper.mock_encoder_factory(), MakeAudioEncoderMock(_, _, _, _)) - .WillOnce(Invoke([&kAnaConfigString, &kAnaReconfigString]( - int payload_type, const SdpAudioFormat& format, - absl::optional codec_pair_id, - std::unique_ptr* return_value) { - auto mock_encoder = SetupAudioEncoderMock(payload_type, format); + EXPECT_CALL(helper.mock_encoder_factory(), Create) + .WillOnce(WithArg<1>([&kAnaConfigString, &kAnaReconfigString]( + const SdpAudioFormat& format) { + auto mock_encoder = SetupAudioEncoderMock(format); EXPECT_CALL(*mock_encoder, EnableAudioNetworkAdaptor(StrEq(kAnaConfigString), _)) .WillOnce(Return(true)); EXPECT_CALL(*mock_encoder, EnableAudioNetworkAdaptor(StrEq(kAnaReconfigString), _)) .WillOnce(Return(true)); - *return_value = std::move(mock_encoder); + return mock_encoder; })); auto send_stream = helper.CreateAudioSendStream(); @@ -549,12 +541,10 @@ TEST(AudioSendStreamTest, AudioNetworkAdaptorReceivesOverhead) { AudioSendStream::Config::SendCodecSpec(0, kOpusFormat); const std::string kAnaConfigString = "abcde"; - EXPECT_CALL(helper.mock_encoder_factory(), MakeAudioEncoderMock(_, _, _, _)) - .WillOnce(Invoke( - [&kAnaConfigString](int payload_type, const SdpAudioFormat& format, - absl::optional codec_pair_id, - std::unique_ptr* return_value) { - auto mock_encoder = SetupAudioEncoderMock(payload_type, format); + EXPECT_CALL(helper.mock_encoder_factory(), Create) + .WillOnce( + WithArg<1>([&kAnaConfigString](const SdpAudioFormat& format) { + auto mock_encoder = SetupAudioEncoderMock(format); InSequence s; EXPECT_CALL( *mock_encoder, @@ -565,9 +555,8 @@ TEST(AudioSendStreamTest, AudioNetworkAdaptorReceivesOverhead) { // Note: Overhead is received AFTER ANA has been enabled. EXPECT_CALL( *mock_encoder, - OnReceivedOverhead(Eq(kOverheadPerPacket.bytes()))) - .WillOnce(Return()); - *return_value = std::move(mock_encoder); + OnReceivedOverhead(Eq(kOverheadPerPacket.bytes()))); + return mock_encoder; })); EXPECT_CALL(*helper.rtp_rtcp(), ExpectedPerPacketOverhead) .WillRepeatedly(Return(kOverheadPerPacket.bytes())); @@ -960,14 +949,12 @@ TEST(AudioSendStreamTest, UseEncoderBitrateRange) { ConfigHelper helper(true, true, true); std::pair bitrate_range{DataRate::BitsPerSec(5000), DataRate::BitsPerSec(10000)}; - EXPECT_CALL(helper.mock_encoder_factory(), MakeAudioEncoderMock(_, _, _, _)) - .WillOnce(Invoke([&](int payload_type, const SdpAudioFormat& format, - absl::optional codec_pair_id, - std::unique_ptr* return_value) { - auto mock_encoder = SetupAudioEncoderMock(payload_type, format); - EXPECT_CALL(*mock_encoder, GetBitrateRange()) + EXPECT_CALL(helper.mock_encoder_factory(), Create) + .WillOnce(WithArg<1>([&](const SdpAudioFormat& format) { + auto mock_encoder = SetupAudioEncoderMock(format); + EXPECT_CALL(*mock_encoder, GetBitrateRange) .WillRepeatedly(Return(bitrate_range)); - *return_value = std::move(mock_encoder); + return mock_encoder; })); auto send_stream = helper.CreateAudioSendStream(); EXPECT_CALL(*helper.bitrate_allocator(), AddObserver(send_stream.get(), _)) diff --git a/test/BUILD.gn b/test/BUILD.gn index 3055c616ed..a3e396601e 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -1184,6 +1184,7 @@ rtc_library("audio_codec_mocks") { "../api:scoped_refptr", "../api/audio_codecs:audio_codecs_api", "../api/audio_codecs:builtin_audio_decoder_factory", + "../api/environment", ] } diff --git a/test/mock_audio_encoder_factory.h b/test/mock_audio_encoder_factory.h index eaa5b8f17d..9f1b0ce4f9 100644 --- a/test/mock_audio_encoder_factory.h +++ b/test/mock_audio_encoder_factory.h @@ -15,6 +15,7 @@ #include #include "api/audio_codecs/audio_encoder_factory.h" +#include "api/environment/environment.h" #include "api/make_ref_counted.h" #include "api/scoped_refptr.h" #include "test/gmock.h" @@ -32,66 +33,25 @@ class MockAudioEncoderFactory QueryAudioEncoder, (const SdpAudioFormat& format), (override)); - - std::unique_ptr MakeAudioEncoder( - int payload_type, - const SdpAudioFormat& format, - absl::optional codec_pair_id) override { - std::unique_ptr return_value; - MakeAudioEncoderMock(payload_type, format, codec_pair_id, &return_value); - return return_value; - } - MOCK_METHOD(void, - MakeAudioEncoderMock, - (int payload_type, - const SdpAudioFormat& format, - absl::optional codec_pair_id, - std::unique_ptr*)); + MOCK_METHOD(std::unique_ptr, + Create, + (const Environment&, const SdpAudioFormat&, Options), + (override)); // Creates a MockAudioEncoderFactory with no formats and that may not be // invoked to create a codec - useful for initializing a voice engine, for // example. - static rtc::scoped_refptr - CreateUnusedFactory() { - using ::testing::_; - using ::testing::AnyNumber; - using ::testing::Return; - - auto factory = rtc::make_ref_counted(); - ON_CALL(*factory.get(), GetSupportedEncoders()) - .WillByDefault(Return(std::vector())); - ON_CALL(*factory.get(), QueryAudioEncoder(_)) - .WillByDefault(Return(absl::nullopt)); - - EXPECT_CALL(*factory.get(), GetSupportedEncoders()).Times(AnyNumber()); - EXPECT_CALL(*factory.get(), QueryAudioEncoder(_)).Times(AnyNumber()); - EXPECT_CALL(*factory.get(), MakeAudioEncoderMock(_, _, _, _)).Times(0); + static scoped_refptr CreateUnusedFactory() { + auto factory = make_ref_counted(); + EXPECT_CALL(*factory, Create).Times(0); return factory; } // Creates a MockAudioEncoderFactory with no formats that may be invoked to // create a codec any number of times. It will, though, return nullptr on each // call, since it supports no codecs. - static rtc::scoped_refptr - CreateEmptyFactory() { - using ::testing::_; - using ::testing::AnyNumber; - using ::testing::Return; - using ::testing::SetArgPointee; - - auto factory = rtc::make_ref_counted(); - ON_CALL(*factory.get(), GetSupportedEncoders()) - .WillByDefault(Return(std::vector())); - ON_CALL(*factory.get(), QueryAudioEncoder(_)) - .WillByDefault(Return(absl::nullopt)); - ON_CALL(*factory.get(), MakeAudioEncoderMock(_, _, _, _)) - .WillByDefault(SetArgPointee<3>(nullptr)); - - EXPECT_CALL(*factory.get(), GetSupportedEncoders()).Times(AnyNumber()); - EXPECT_CALL(*factory.get(), QueryAudioEncoder(_)).Times(AnyNumber()); - EXPECT_CALL(*factory.get(), MakeAudioEncoderMock(_, _, _, _)) - .Times(AnyNumber()); - return factory; + static scoped_refptr CreateEmptyFactory() { + return make_ref_counted(); } };