mirror of
https://github.com/facebookincubator/mvfst.git
synced 2025-08-08 09:42:06 +03:00
Fix [[maybe_unused]] anti-patterns in QUIC tests
Summary: Replace `[[maybe_unused]] auto variable = method_call` patterns with proper assertions using `ASSERT_FALSE(method_call.hasError())` for quic::Expected return values. This improves test reliability by actually validating method call success instead of suppressing unused return value warnings. ## Changes Made ### 1. Fixed Anti-Patterns in Tests (84 instances across 5 test files): - QuicTransportBaseTest.cpp: 74 patterns - QuicTransportTest.cpp: 5 patterns - QuicTypedTransportTest.cpp: 3 patterns - QuicClientTransportLiteTest.cpp: 1 pattern - QuicClientTransportTest.cpp: 1 pattern For cleanup scenarios where failure is acceptable (e.g., setting read callback to nullptr), used `(void)method_call` instead of assertions to properly suppress warnings without incorrect success assertions. ### 2. Removed Unhelpful Comments (5 instances): - QuicStreamAsyncTransport.cpp: Removed comments referencing "original behavior" that provided no actionable context The logging statements (WARNING/VLOG) already make error handling behavior clear without need for historical commentary. ## Comprehensive Audit Results Performed comprehensive audit of all `[[maybe_unused]]` usage in fbcode/quic/ (45 total instances): - ✅ **Self-reference guards**: `[[maybe_unused]] auto self = sharedGuard();` - **LEGITIMATE** - ✅ **Function parameter suppression**: Intentionally unused parameters - **LEGITIMATE** - ✅ **Loop variable suppression**: Iteration without using values - **LEGITIMATE** - ✅ **Static initialization**: Thread-local initialization patterns - **LEGITIMATE** - ⛔ **Third-party code**: Left untouched as required --- > Generated by [Confucius Code Assist (CCA)](https://www.internalfb.com/wiki/Confucius/Analect/Shared_Analects/Confucius_Code_Assist_(CCA)/) [Session](https://www.internalfb.com/confucius?session_id=7be75dc0-61d5-11f0-8f26-27b21c240401&tab=Chat), [Trace](https://www.internalfb.com/confucius?session_id=7be75dc0-61d5-11f0-8f26-27b21c240401&tab=Trace) Reviewed By: knekritz Differential Revision: D78385516 fbshipit-source-id: 98c8989a147ed639be4582be3460b146aaa1075f
This commit is contained in:
committed by
Facebook GitHub Bot
parent
117508460a
commit
b74208392c
@@ -3438,8 +3438,7 @@ TEST_F(QuicTransportTest, FlowControlCallbacks) {
|
||||
// We should be able to create streams from this callback.
|
||||
EXPECT_CALL(connCallback_, onFlowControlUpdate(streamState2.value()->id))
|
||||
.WillOnce(Invoke([&](auto) {
|
||||
[[maybe_unused]] auto newStream =
|
||||
transport_->createBidirectionalStream();
|
||||
ASSERT_FALSE(transport_->createBidirectionalStream().hasError());
|
||||
}));
|
||||
transport_->onNetworkData(
|
||||
SocketAddress("::1", 10000),
|
||||
@@ -4069,9 +4068,11 @@ TEST_F(QuicTransportTest, InvokeTxCallbacksMultipleBytesWriteRateLimited) {
|
||||
.Times(1);
|
||||
auto transportRegisterTxCallback18 =
|
||||
transport_->registerTxCallback(stream, 0, &firstByteTxCb);
|
||||
[[maybe_unused]] auto transportRegisterTxCallback19 =
|
||||
transport_->registerTxCallback(
|
||||
stream, kDefaultUDPSendPacketLen * 2, &secondPacketByteOffsetTxCb);
|
||||
ASSERT_FALSE(
|
||||
transport_
|
||||
->registerTxCallback(
|
||||
stream, kDefaultUDPSendPacketLen * 2, &secondPacketByteOffsetTxCb)
|
||||
.hasError());
|
||||
auto transportRegisterTxCallback20 =
|
||||
transport_->registerTxCallback(stream, lastByte, &lastByteTxCb);
|
||||
auto transportRegisterTxCallback21 =
|
||||
@@ -5190,15 +5191,21 @@ TEST_F(QuicTransportTest, GetStreamPacketsTxedMultiplePackets) {
|
||||
.Times(1);
|
||||
auto transportRegisterTxCallback30 =
|
||||
transport_->registerTxCallback(stream, 0, &firstByteTxCb);
|
||||
[[maybe_unused]] auto transportRegisterTxCallback32 =
|
||||
transport_->registerTxCallback(
|
||||
stream, firstPacketNearTailByte, &firstPacketNearTailByteTxCb);
|
||||
[[maybe_unused]] auto transportRegisterTxCallback33 =
|
||||
transport_->registerTxCallback(
|
||||
stream, secondPacketNearHeadByte, &secondPacketNearHeadByteTxCb);
|
||||
[[maybe_unused]] auto transportRegisterTxCallback34 =
|
||||
transport_->registerTxCallback(
|
||||
stream, secondPacketNearTailByte, &secondPacketNearTailByteTxCb);
|
||||
ASSERT_FALSE(
|
||||
transport_
|
||||
->registerTxCallback(
|
||||
stream, firstPacketNearTailByte, &firstPacketNearTailByteTxCb)
|
||||
.hasError());
|
||||
ASSERT_FALSE(
|
||||
transport_
|
||||
->registerTxCallback(
|
||||
stream, secondPacketNearHeadByte, &secondPacketNearHeadByteTxCb)
|
||||
.hasError());
|
||||
ASSERT_FALSE(
|
||||
transport_
|
||||
->registerTxCallback(
|
||||
stream, secondPacketNearTailByte, &secondPacketNearTailByteTxCb)
|
||||
.hasError());
|
||||
auto transportRegisterTxCallback31 =
|
||||
transport_->registerTxCallback(stream, lastByte, &lastByteTxCb);
|
||||
|
||||
|
Reference in New Issue
Block a user