From e96ef39857b624be5e5da29b7f88abd6500b893e Mon Sep 17 00:00:00 2001 From: Alex Kremer Date: Tue, 5 May 2026 15:21:54 +0100 Subject: [PATCH] chore: Fix clang-tidy issues after introducing new checks (#3061) --- .../integration/data/BackendFactoryTests.cpp | 3 + .../CassandraMigrationManagerTests.cpp | 9 +- tests/unit/etl/LedgerPublisherTests.cpp | 40 +++++--- tests/unit/etl/NFTHelpersTests.cpp | 12 ++- .../unit/migration/MigratorRegisterTests.cpp | 16 ++- tests/unit/rpc/common/CheckersTests.cpp | 12 +-- tests/unit/rpc/common/SpecsTests.cpp | 4 +- .../util/config/ClioConfigDefinitionTests.cpp | 1 + tests/unit/util/config/ConfigValueTests.cpp | 97 +++++++++++-------- tests/unit/util/prometheus/HttpTests.cpp | 1 + 10 files changed, 122 insertions(+), 73 deletions(-) diff --git a/tests/integration/data/BackendFactoryTests.cpp b/tests/integration/data/BackendFactoryTests.cpp index 0adbc301e..709f95b7d 100644 --- a/tests/integration/data/BackendFactoryTests.cpp +++ b/tests/integration/data/BackendFactoryTests.cpp @@ -138,7 +138,10 @@ TEST_F(BackendCassandraFactoryTestWithDB, CreateCassandraBackend) EXPECT_TRUE(backend); auto const range = backend->fetchLedgerRange(); + ASSERT_TRUE(range.has_value()); + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) EXPECT_EQ(range->minSequence, 100); + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) EXPECT_EQ(range->maxSequence, 500); } } diff --git a/tests/integration/migration/cassandra/CassandraMigrationManagerTests.cpp b/tests/integration/migration/cassandra/CassandraMigrationManagerTests.cpp index 55d5990bc..ecf46fb73 100644 --- a/tests/integration/migration/cassandra/CassandraMigrationManagerTests.cpp +++ b/tests/integration/migration/cassandra/CassandraMigrationManagerTests.cpp @@ -241,19 +241,22 @@ TEST_F(MigrationCassandraManagerTxTableTest, MigrateExampleTransactionsMigrator) auto txType = getTxType( ripple::uint256("CEECF7E516F8A53C5D32A357B737ED54D3186FDD510B1973D908AD8D93AD8E00") ); - EXPECT_TRUE(txType.has_value()); + ASSERT_TRUE(txType.has_value()); + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) EXPECT_EQ(txType.value(), "OracleSet"); txType = getTxType( ripple::uint256("35DBFB1A88DE17EBD2BCE37F6E1FD6D3B9887C92B7933ED2FCF2A84E9138B7CA") ); - EXPECT_TRUE(txType.has_value()); + ASSERT_TRUE(txType.has_value()); + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) EXPECT_EQ(txType.value(), "Payment"); txType = getTxType( ripple::uint256("FCACE9D00625FA3BCC5316078324EA153EC8551243AC1701D496CC1CA2B8A474") ); - EXPECT_TRUE(txType.has_value()); + ASSERT_TRUE(txType.has_value()); + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) EXPECT_EQ(txType.value(), "AMMCreate"); EXPECT_EQ( diff --git a/tests/unit/etl/LedgerPublisherTests.cpp b/tests/unit/etl/LedgerPublisherTests.cpp index 98999bda7..90b7ba14a 100644 --- a/tests/unit/etl/LedgerPublisherTests.cpp +++ b/tests/unit/etl/LedgerPublisherTests.cpp @@ -62,9 +62,10 @@ TEST_F(ETLLedgerPublisherTest, PublishLedgerHeaderSkipDueToAge) publisher.publish(dummyLedgerHeader); // Verify last published sequence is set immediately - EXPECT_TRUE(publisher.getLastPublishedSequence()); + auto const seq = publisher.getLastPublishedSequence(); + ASSERT_TRUE(seq.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) - EXPECT_EQ(publisher.getLastPublishedSequence().value(), kSEQ); + EXPECT_EQ(seq.value(), kSEQ); // Since age > MAX_LEDGER_AGE_SECONDS, these should not be called EXPECT_CALL(*backend_, doFetchLedgerObject).Times(0); @@ -97,9 +98,10 @@ TEST_F(ETLLedgerPublisherTest, PublishLedgerHeaderWithinAgeLimit) publisher.publish(dummyLedgerHeader); // Verify last published sequence is set immediately - EXPECT_TRUE(publisher.getLastPublishedSequence()); + auto const seq = publisher.getLastPublishedSequence(); + ASSERT_TRUE(seq.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) - EXPECT_EQ(publisher.getLastPublishedSequence().value(), kSEQ); + EXPECT_EQ(seq.value(), kSEQ); ctx.join(); EXPECT_TRUE(publisher.lastPublishAgeSeconds() <= 1); @@ -113,9 +115,10 @@ TEST_F(ETLLedgerPublisherTest, PublishLedgerHeaderIsWritingTrue) auto publisher = impl::LedgerPublisher(ctx, backend_, mockSubscriptionManagerPtr, dummyState); publisher.publish(dummyLedgerHeader); - EXPECT_TRUE(publisher.getLastPublishedSequence()); + auto const seq = publisher.getLastPublishedSequence(); + ASSERT_TRUE(seq.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) - EXPECT_EQ(publisher.getLastPublishedSequence().value(), kSEQ); + EXPECT_EQ(seq.value(), kSEQ); ctx.join(); @@ -155,9 +158,10 @@ TEST_F(ETLLedgerPublisherTest, PublishLedgerHeaderInRange) EXPECT_CALL(*mockSubscriptionManagerPtr, pubTransaction); publisher.publish(dummyLedgerHeader); - EXPECT_TRUE(publisher.getLastPublishedSequence()); + auto const seq = publisher.getLastPublishedSequence(); + ASSERT_TRUE(seq.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) - EXPECT_EQ(publisher.getLastPublishedSequence().value(), kSEQ); + EXPECT_EQ(seq.value(), kSEQ); ctx.join(); @@ -202,9 +206,10 @@ TEST_F(ETLLedgerPublisherTest, PublishLedgerHeaderCloseTimeGreaterThanNow) EXPECT_CALL(*mockSubscriptionManagerPtr, pubTransaction); publisher.publish(dummyLedgerHeader); - EXPECT_TRUE(publisher.getLastPublishedSequence()); + auto const seq = publisher.getLastPublishedSequence(); + ASSERT_TRUE(seq.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) - EXPECT_EQ(publisher.getLastPublishedSequence().value(), kSEQ); + EXPECT_EQ(seq.value(), kSEQ); ctx.join(); @@ -294,9 +299,10 @@ TEST_F(ETLLedgerPublisherTest, PublishMultipleTxInOrder) EXPECT_CALL(*mockSubscriptionManagerPtr, pubTransaction(t1, _)).InSequence(s); publisher.publish(dummyLedgerHeader); - EXPECT_TRUE(publisher.getLastPublishedSequence()); + auto const seq = publisher.getLastPublishedSequence(); + ASSERT_TRUE(seq.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) - EXPECT_EQ(publisher.getLastPublishedSequence().value(), kSEQ); + EXPECT_EQ(seq.value(), kSEQ); ctx.join(); @@ -318,9 +324,10 @@ TEST_F(ETLLedgerPublisherTest, PublishVeryOldLedgerShouldSkip) EXPECT_CALL(*mockSubscriptionManagerPtr, pubTransaction).Times(0); publisher.publish(dummyLedgerHeader); - EXPECT_TRUE(publisher.getLastPublishedSequence()); + auto const seq = publisher.getLastPublishedSequence(); + ASSERT_TRUE(seq.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) - EXPECT_EQ(publisher.getLastPublishedSequence().value(), kSEQ); + EXPECT_EQ(seq.value(), kSEQ); ctx.join(); } @@ -367,9 +374,10 @@ TEST_F(ETLLedgerPublisherTest, PublishMultipleLedgersInQuickSuccession) publisher.publish(dummyLedgerHeader1); publisher.publish(dummyLedgerHeader2); - EXPECT_TRUE(publisher.getLastPublishedSequence()); + auto const seq = publisher.getLastPublishedSequence(); + ASSERT_TRUE(seq.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) - EXPECT_EQ(publisher.getLastPublishedSequence().value(), kSEQ + 1); + EXPECT_EQ(seq.value(), kSEQ + 1); ctx.join(); } diff --git a/tests/unit/etl/NFTHelpersTests.cpp b/tests/unit/etl/NFTHelpersTests.cpp index 3e373c19c..938cba80e 100644 --- a/tests/unit/etl/NFTHelpersTests.cpp +++ b/tests/unit/etl/NFTHelpersTests.cpp @@ -69,7 +69,7 @@ protected: if (sttx.getTxnType() == ripple::ttNFTOKEN_MINT || sttx.getTxnType() == ripple::ttNFTOKEN_MODIFY) { - EXPECT_TRUE(data.uri.has_value()); + ASSERT_TRUE(data.uri.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) EXPECT_EQ(*data.uri, sttx.getFieldVL(ripple::sfURI)); } else { @@ -150,6 +150,7 @@ TEST_F(NFTHelpersTest, NFTModifyWithURI) EXPECT_EQ(nftTxs.size(), 1); verifyNFTTransactionsData(nftTxs[0], sttx, txMeta, kNFT_ID); + ASSERT_TRUE(nftDatas.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) verifyNFTsData(*nftDatas, sttx, txMeta, kNFT_ID, std::nullopt); } @@ -164,6 +165,7 @@ TEST_F(NFTHelpersTest, NFTModifyWithoutURI) EXPECT_EQ(nftTxs.size(), 1); verifyNFTTransactionsData(nftTxs[0], sttx, txMeta, kNFT_ID); + ASSERT_TRUE(nftDatas.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) verifyNFTsData(*nftDatas, sttx, txMeta, kNFT_ID, std::nullopt); } @@ -179,6 +181,7 @@ TEST_F(NFTHelpersTest, NFTMintFromModifiedNode) EXPECT_EQ(nftTxs.size(), 1); verifyNFTTransactionsData(nftTxs[0], sttx, txMeta, kNFT_ID); + ASSERT_TRUE(nftDatas.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) verifyNFTsData(*nftDatas, sttx, txMeta, kNFT_ID, kACCOUNT); } @@ -213,6 +216,7 @@ TEST_F(NFTHelpersTest, NFTMintFromCreatedNode) EXPECT_EQ(nftTxs.size(), 1); verifyNFTTransactionsData(nftTxs[0], sttx, txMeta, kNFT_ID); + ASSERT_TRUE(nftDatas.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) verifyNFTsData(*nftDatas, sttx, txMeta, kNFT_ID, kACCOUNT); } @@ -230,6 +234,7 @@ TEST_F(NFTHelpersTest, NFTMintWithoutUriField) EXPECT_EQ(nftTxs.size(), 1); verifyNFTTransactionsData(nftTxs[0], sttx, txMeta, kNFT_ID); + ASSERT_TRUE(nftDatas.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) verifyNFTsData(*nftDatas, sttx, txMeta, kNFT_ID, kACCOUNT); } @@ -261,6 +266,7 @@ TEST_F(NFTHelpersTest, NFTBurnFromDeletedNode) EXPECT_EQ(nftTxs.size(), 1); verifyNFTTransactionsData(nftTxs[0], sttx, txMeta, kNFT_ID); + ASSERT_TRUE(nftDatas.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) verifyNFTsData(*nftDatas, sttx, txMeta, kNFT_ID, kACCOUNT); } @@ -291,6 +297,7 @@ TEST_F(NFTHelpersTest, NFTBurnFromModifiedNode) EXPECT_EQ(nftTxs.size(), 1); verifyNFTTransactionsData(nftTxs[0], sttx, txMeta, kNFT_ID); + ASSERT_TRUE(nftDatas.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) verifyNFTsData(*nftDatas, sttx, txMeta, kNFT_ID, kACCOUNT); } @@ -373,6 +380,7 @@ TEST_F(NFTHelpersTest, NFTAcceptBuyerOffer) EXPECT_EQ(nftTxs.size(), 1); EXPECT_TRUE(nftDatas); verifyNFTTransactionsData(nftTxs[0], sttx, txMeta, kNFT_ID); + ASSERT_TRUE(nftDatas.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) verifyNFTsData(*nftDatas, sttx, txMeta, kNFT_ID, kACCOUNT); } @@ -406,6 +414,7 @@ TEST_F(NFTHelpersTest, NFTAcceptSellerOfferFromCreatedNode) EXPECT_EQ(nftTxs.size(), 1); EXPECT_TRUE(nftDatas); verifyNFTTransactionsData(nftTxs[0], sttx, txMeta, kNFT_ID); + ASSERT_TRUE(nftDatas.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) verifyNFTsData(*nftDatas, sttx, txMeta, kNFT_ID, kACCOUNT); } @@ -423,6 +432,7 @@ TEST_F(NFTHelpersTest, NFTAcceptSellerOfferFromModifiedNode) EXPECT_EQ(nftTxs.size(), 1); EXPECT_TRUE(nftDatas); verifyNFTTransactionsData(nftTxs[0], sttx, txMeta, kNFT_ID); + ASSERT_TRUE(nftDatas.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) verifyNFTsData(*nftDatas, sttx, txMeta, kNFT_ID, kACCOUNT); } diff --git a/tests/unit/migration/MigratorRegisterTests.cpp b/tests/unit/migration/MigratorRegisterTests.cpp index bde0f755e..b52097550 100644 --- a/tests/unit/migration/MigratorRegisterTests.cpp +++ b/tests/unit/migration/MigratorRegisterTests.cpp @@ -73,6 +73,7 @@ TEST_F(MultipleMigratorRegisterTests, GetMigratorsStatusWhenError) .Times(3) .WillRepeatedly(testing::Return(std::nullopt)); + ASSERT_TRUE(migratorRegister.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) auto const status = migratorRegister->getMigratorsStatus(); EXPECT_EQ(status.size(), 3); @@ -102,6 +103,7 @@ TEST_F(MultipleMigratorRegisterTests, GetMigratorsStatusWhenReturnInvalidStatus) .Times(3) .WillRepeatedly(testing::Return("Invalid")); + ASSERT_TRUE(migratorRegister.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) auto const status = migratorRegister->getMigratorsStatus(); EXPECT_EQ(status.size(), 3); @@ -134,6 +136,7 @@ TEST_F(MultipleMigratorRegisterTests, GetMigratorsStatusWhenOneMigrated) EXPECT_CALL(*backend_, fetchMigratorStatus("SimpleTestMigrator3", testing::_)) .WillOnce(testing::Return("NotMigrated")); + ASSERT_TRUE(migratorRegister.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) auto const status = migratorRegister->getMigratorsStatus(); EXPECT_EQ(status.size(), 3); @@ -164,6 +167,7 @@ TEST_F(MultipleMigratorRegisterTests, GetMigratorStatus) EXPECT_CALL(*backend_, fetchMigratorStatus("SimpleTestMigrator2", testing::_)) .WillOnce(testing::Return("NotMigrated")); + ASSERT_TRUE(migratorRegister.has_value()); EXPECT_EQ( // NOLINTNEXTLINE(bugprone-unchecked-optional-access) migratorRegister->getMigratorStatus("unknown"), @@ -187,6 +191,7 @@ TEST_F(MultipleMigratorRegisterTests, GetMigratorStatusWhenError) .Times(2) .WillRepeatedly(testing::Return(std::nullopt)); + ASSERT_TRUE(migratorRegister.has_value()); EXPECT_EQ( // NOLINTNEXTLINE(bugprone-unchecked-optional-access) migratorRegister->getMigratorStatus("unknown"), @@ -206,6 +211,7 @@ TEST_F(MultipleMigratorRegisterTests, GetMigratorStatusWhenError) TEST_F(MultipleMigratorRegisterTests, Names) { + ASSERT_TRUE(migratorRegister.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) auto names = migratorRegister->getMigratorNames(); EXPECT_EQ(names.size(), 3); @@ -216,6 +222,7 @@ TEST_F(MultipleMigratorRegisterTests, Names) TEST_F(MultipleMigratorRegisterTests, Description) { + ASSERT_TRUE(migratorRegister.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) EXPECT_EQ(migratorRegister->getMigratorDescription("unknown"), "No Description"); EXPECT_EQ( @@ -250,22 +257,23 @@ TEST_F(MultipleMigratorRegisterTests, MigrateNormalMigrator) TEST_F(MultipleMigratorRegisterTests, canBlock) { + ASSERT_TRUE(migratorRegister.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) auto canBlock = migratorRegister->canMigratorBlockClio("SimpleTestMigrator"); - EXPECT_TRUE(canBlock); + ASSERT_TRUE(canBlock.has_value()); EXPECT_TRUE(*canBlock); // NOLINT(bugprone-unchecked-optional-access) // NOLINTNEXTLINE(bugprone-unchecked-optional-access) canBlock = migratorRegister->canMigratorBlockClio("SimpleTestMigrator2"); - EXPECT_TRUE(canBlock); + ASSERT_TRUE(canBlock.has_value()); EXPECT_FALSE(*canBlock); // NOLINT(bugprone-unchecked-optional-access) // NOLINTNEXTLINE(bugprone-unchecked-optional-access) canBlock = migratorRegister->canMigratorBlockClio("SimpleTestMigrator3"); - EXPECT_TRUE(canBlock); + ASSERT_TRUE(canBlock.has_value()); EXPECT_FALSE(*canBlock); // NOLINT(bugprone-unchecked-optional-access) // NOLINTNEXTLINE(bugprone-unchecked-optional-access) canBlock = migratorRegister->canMigratorBlockClio("NotAMigrator"); - EXPECT_FALSE(canBlock); + EXPECT_FALSE(canBlock.has_value()); } diff --git a/tests/unit/rpc/common/CheckersTests.cpp b/tests/unit/rpc/common/CheckersTests.cpp index d12950d1d..2c34322cb 100644 --- a/tests/unit/rpc/common/CheckersTests.cpp +++ b/tests/unit/rpc/common/CheckersTests.cpp @@ -21,19 +21,19 @@ struct DeprecatedTests : ::testing::Test { TEST_F(DeprecatedTests, Field) { auto warning = Deprecated<>::check(json, "some_string"); - ASSERT_TRUE(warning); + ASSERT_TRUE(warning.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) EXPECT_EQ(warning->warningCode, WarningCode::WarnRpcDeprecated); warning = Deprecated<>::check(json, "other"); - EXPECT_FALSE(warning); + EXPECT_FALSE(warning.has_value()); } TEST_F(DeprecatedTests, FieldWithStringValue) { Deprecated const checker{"some_value"}; auto warning = checker.check(json, "some_string"); - ASSERT_TRUE(warning); + ASSERT_TRUE(warning.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) EXPECT_EQ(warning->warningCode, WarningCode::WarnRpcDeprecated); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) @@ -45,7 +45,7 @@ TEST_F(DeprecatedTests, FieldWithIntValue) { Deprecated const checker{42}; auto warning = checker.check(json, "some_number"); - ASSERT_TRUE(warning); + ASSERT_TRUE(warning.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) EXPECT_EQ(warning->warningCode, WarningCode::WarnRpcDeprecated); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) @@ -57,7 +57,7 @@ TEST_F(DeprecatedTests, FieldWithBoolValue) { Deprecated const checker{false}; auto warning = checker.check(json, "some_bool"); - ASSERT_TRUE(warning); + ASSERT_TRUE(warning.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) EXPECT_EQ(warning->warningCode, WarningCode::WarnRpcDeprecated); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) @@ -69,7 +69,7 @@ TEST_F(DeprecatedTests, FieldWithFloatValue) { Deprecated const checker{3.14}; auto warning = checker.check(json, "some_float"); - ASSERT_TRUE(warning); + ASSERT_TRUE(warning.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) EXPECT_EQ(warning->warningCode, WarningCode::WarnRpcDeprecated); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) diff --git a/tests/unit/rpc/common/SpecsTests.cpp b/tests/unit/rpc/common/SpecsTests.cpp index ab69c6522..8e48db857 100644 --- a/tests/unit/rpc/common/SpecsTests.cpp +++ b/tests/unit/rpc/common/SpecsTests.cpp @@ -100,7 +100,7 @@ INSTANTIATE_TEST_SUITE_P( TEST_P(FieldProcessorTests, FieldSpecWithRequirementProcess) { EXPECT_CALL(requirementMock, verify).WillOnce(testing::Return(GetParam().requirementResult)); - if (GetParam().otherRequirementResult) { + if (GetParam().otherRequirementResult.has_value()) { EXPECT_CALL(anotherRequirementMock, verify) // NOLINTNEXTLINE(bugprone-unchecked-optional-access) .WillOnce(testing::Return(GetParam().otherRequirementResult.value())); @@ -205,7 +205,7 @@ INSTANTIATE_TEST_SUITE_P( TEST_P(RpcSpecProcessTests, Process) { EXPECT_CALL(requirementMock, verify).WillOnce(testing::Return(GetParam().requirementResult)); - if (GetParam().otherRequirementResult) { + if (GetParam().otherRequirementResult.has_value()) { EXPECT_CALL(anotherRequirementMock, verify) // NOLINTNEXTLINE(bugprone-unchecked-optional-access) .WillOnce(testing::Return(GetParam().otherRequirementResult.value())); diff --git a/tests/unit/util/config/ClioConfigDefinitionTests.cpp b/tests/unit/util/config/ClioConfigDefinitionTests.cpp index 31bbdb8bf..f57814820 100644 --- a/tests/unit/util/config/ClioConfigDefinitionTests.cpp +++ b/tests/unit/util/config/ClioConfigDefinitionTests.cpp @@ -482,6 +482,7 @@ TEST(ClioConfigDefinitionParse, unexpectedFields) auto const configFile = ConfigFileJson{configJson}; auto result = config.parse(configFile); + ASSERT_TRUE(result.has_value()); std::ranges::sort( // NOLINTNEXTLINE(bugprone-unchecked-optional-access) *result, diff --git a/tests/unit/util/config/ConfigValueTests.cpp b/tests/unit/util/config/ConfigValueTests.cpp index b6fa0d43b..797097501 100644 --- a/tests/unit/util/config/ConfigValueTests.cpp +++ b/tests/unit/util/config/ConfigValueTests.cpp @@ -135,7 +135,7 @@ TEST_F(ConstraintTest, SetValuesOnPortConstraint) { auto cvPort = ConfigValue{ConfigType::Integer}.defaultValue(4444).withConstraint(gValidatePort); auto const err = cvPort.setValue(99999); - EXPECT_TRUE(err.has_value()); + ASSERT_TRUE(err.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) EXPECT_EQ(err->error, "Unknown_key Port does not satisfy the constraint bounds"); EXPECT_TRUE(cvPort.setValue(33.33).has_value()); @@ -149,7 +149,7 @@ TEST_F(ConstraintTest, SetValuesOnPortConstraint) auto cvPort2 = ConfigValue{ConfigType::String}.defaultValue("4444").withConstraint(gValidatePort); auto const strPortError = cvPort2.setValue("100000"); - EXPECT_TRUE(strPortError.has_value()); + ASSERT_TRUE(strPortError.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) EXPECT_EQ(strPortError->error, "Unknown_key Port does not satisfy the constraint bounds"); } @@ -160,19 +160,22 @@ TEST_F(ConstraintTest, OneOfConstraintOneValue) auto const databaseConstraint{OneOf{"database.type", arr}}; EXPECT_FALSE(databaseConstraint.checkConstraint("tracer").has_value()); - EXPECT_TRUE(databaseConstraint.checkConstraint(345).has_value()); - EXPECT_EQ( + { + auto const res = databaseConstraint.checkConstraint(345); + ASSERT_TRUE(res.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) - databaseConstraint.checkConstraint(345)->error, - R"(Key "database.type"'s value must be a string)" - ); + EXPECT_EQ(res->error, R"(Key "database.type"'s value must be a string)"); + } - EXPECT_TRUE(databaseConstraint.checkConstraint("123.44").has_value()); - EXPECT_EQ( - // NOLINTNEXTLINE(bugprone-unchecked-optional-access) - databaseConstraint.checkConstraint("123.44")->error, - R"(You provided value "123.44". Key "database.type"'s value must be one of the following: tracer)" - ); + { + auto const res = databaseConstraint.checkConstraint("123.44"); + ASSERT_TRUE(res.has_value()); + EXPECT_EQ( + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) + res->error, + R"(You provided value "123.44". Key "database.type"'s value must be one of the following: tracer)" + ); + } } TEST_F(ConstraintTest, OneOfConstraint) @@ -182,16 +185,22 @@ TEST_F(ConstraintTest, OneOfConstraint) EXPECT_FALSE(oneOfCons.checkConstraint("trace").has_value()); - EXPECT_TRUE(oneOfCons.checkConstraint(345).has_value()); - // NOLINTNEXTLINE(bugprone-unchecked-optional-access) - EXPECT_EQ(oneOfCons.checkConstraint(345)->error, R"(Key "log.level"'s value must be a string)"); - - EXPECT_TRUE(oneOfCons.checkConstraint("PETER_WAS_HERE").has_value()); - EXPECT_EQ( + { + auto const res = oneOfCons.checkConstraint(345); + ASSERT_TRUE(res.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) - oneOfCons.checkConstraint("PETER_WAS_HERE")->error, - R"(You provided value "PETER_WAS_HERE". Key "log.level"'s value must be one of the following: 123, trace, haha)" - ); + EXPECT_EQ(res->error, R"(Key "log.level"'s value must be a string)"); + } + + { + auto const res = oneOfCons.checkConstraint("PETER_WAS_HERE"); + ASSERT_TRUE(res.has_value()); + EXPECT_EQ( + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) + res->error, + R"(You provided value "PETER_WAS_HERE". Key "log.level"'s value must be one of the following: 123, trace, haha)" + ); + } } TEST_F(ConstraintTest, IpConstraint) @@ -223,16 +232,19 @@ TEST_F(ConstraintTest, positiveNumConstraint) EXPECT_FALSE(numCons.checkConstraint(0)); EXPECT_FALSE(numCons.checkConstraint(5)); - EXPECT_TRUE(numCons.checkConstraint(true)); - // NOLINTNEXTLINE(bugprone-unchecked-optional-access) - EXPECT_EQ(numCons.checkConstraint(true)->error, fmt::format("Number must be of type integer")); - - EXPECT_TRUE(numCons.checkConstraint(8)); - EXPECT_EQ( + { + auto const res = numCons.checkConstraint(true); + ASSERT_TRUE(res.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) - numCons.checkConstraint(8)->error, - fmt::format("Number must be between {} and {}", 0, 5) - ); + EXPECT_EQ(res->error, fmt::format("Number must be of type integer")); + } + + { + auto const res = numCons.checkConstraint(8); + ASSERT_TRUE(res.has_value()); + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) + EXPECT_EQ(res->error, fmt::format("Number must be between {} and {}", 0, 5)); + } } TEST_F(ConstraintTest, SetValuesOnNumberConstraint) @@ -240,7 +252,7 @@ TEST_F(ConstraintTest, SetValuesOnNumberConstraint) auto positiveNum = ConfigValue{ConfigType::Integer}.defaultValue(20u).withConstraint(gValidateUint16); auto const err = positiveNum.setValue(-22, "key"); - EXPECT_TRUE(err.has_value()); + ASSERT_TRUE(err.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) EXPECT_EQ(err->error, fmt::format("key Number must be between {} and {}", 1, 65535)); EXPECT_FALSE(positiveNum.setValue(99, "key")); @@ -252,17 +264,20 @@ TEST_F(ConstraintTest, PositiveDoubleConstraint) EXPECT_FALSE(doubleCons.checkConstraint(0.2)); EXPECT_FALSE(doubleCons.checkConstraint(5.54)); EXPECT_FALSE(doubleCons.checkConstraint(3)); - EXPECT_TRUE(doubleCons.checkConstraint("-5")); - EXPECT_EQ( + { + auto const res = doubleCons.checkConstraint("-5"); + ASSERT_TRUE(res.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) - doubleCons.checkConstraint("-5")->error, - "Double number must be of type int or double" - ); - EXPECT_EQ( + EXPECT_EQ(res->error, "Double number must be of type int or double"); + } + + { + auto const res = doubleCons.checkConstraint(-5.6); + ASSERT_TRUE(res.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) - doubleCons.checkConstraint(-5.6)->error, - "Double number must be greater than or equal to 0" - ); + EXPECT_EQ(res->error, "Double number must be greater than or equal to 0"); + } + EXPECT_FALSE(doubleCons.checkConstraint(12.1)); } diff --git a/tests/unit/util/prometheus/HttpTests.cpp b/tests/unit/util/prometheus/HttpTests.cpp index 6dc000568..811c32714 100644 --- a/tests/unit/util/prometheus/HttpTests.cpp +++ b/tests/unit/util/prometheus/HttpTests.cpp @@ -196,6 +196,7 @@ TEST_F(PrometheusHandleRequestTests, responseWithCounterAndGauge) auto response = handlePrometheusRequest(req, true); + ASSERT_TRUE(response.has_value()); EXPECT_EQ(response->result(), http::status::ok); // NOLINT(bugprone-unchecked-optional-access) // NOLINTNEXTLINE(bugprone-unchecked-optional-access) EXPECT_EQ(response->operator[](http::field::content_type), "text/plain; version=0.0.4");