SERVER-79407 Enable bugprone-parent-virtual-call clang-tidy check

This commit is contained in:
Zack Winter 2023-08-14 15:27:41 +00:00 committed by Evergreen Agent
parent c9c79c4ec0
commit 57c2635822
14 changed files with 130 additions and 177 deletions

View File

@ -18,6 +18,7 @@ Checks: '-*,
bugprone-misplaced-operator-in-strlen-in-alloc,
bugprone-misplaced-widening-cast,
bugprone-multiple-statement-macro,
bugprone-parent-virtual-call,
bugprone-string-integer-assignment,
bugprone-suspicious-enum-usage,
bugprone-suspicious-memset-usage,
@ -67,7 +68,6 @@ Checks: '-*,
-bugprone-macro-parentheses,
-bugprone-move-forwarding-reference,
-bugprone-narrowing-conversions,
-bugprone-parent-virtual-call,
-bugprone-sizeof-container,
-bugprone-sizeof-expression,
-bugprone-string-constructor,

View File

@ -1226,12 +1226,6 @@ DEATH_TEST_REGEX_F(OpObserverTest,
class OpObserverTxnParticipantTest : public OpObserverTest {
public:
void setUp() override {
_opCtx = cc().makeOperationContext();
_opObserver.emplace(std::make_unique<OplogWriterImpl>());
_times.emplace(opCtx());
}
void tearDown() override {
_sessionCheckout.reset();
_times.reset();
@ -1240,6 +1234,12 @@ public:
OpObserverTest::tearDown();
}
void setUpObserverContext() {
_opCtx = cc().makeOperationContext();
_opObserver.emplace(std::make_unique<OplogWriterImpl>());
_times.emplace(opCtx());
}
void setUpRetryableWrite() {
beginRetryableWriteWithTxnNumber(opCtx(), txnNum(), _sessionCheckout);
_txnParticipant.emplace(TransactionParticipant::get(opCtx()));
@ -1333,8 +1333,8 @@ private:
class OpObserverTransactionTest : public OpObserverTxnParticipantTest {
protected:
void setUp() override {
OpObserverTest::setUp();
OpObserverTxnParticipantTest::setUp();
setUpObserverContext();
OpObserverTxnParticipantTest::setUpNonRetryableTransaction();
}
@ -2375,8 +2375,8 @@ class OpObserverRetryableFindAndModifyOutsideTransactionTest
public:
void setUp() override {
OpObserverRetryableFindAndModifyTest::setUp();
OpObserverTxnParticipantTest::setUp();
OpObserverTxnParticipantTest::setUpRetryableWrite();
setUpObserverContext();
setUpRetryableWrite();
}
protected:
@ -2410,8 +2410,8 @@ class OpObserverRetryableFindAndModifyInsideUnpreparedRetryableInternalTransacti
public:
void setUp() override {
OpObserverRetryableFindAndModifyTest::setUp();
OpObserverTxnParticipantTest::setUp();
OpObserverTxnParticipantTest::setUpRetryableInternalTransaction();
setUpObserverContext();
setUpRetryableInternalTransaction();
}
protected:
@ -2447,8 +2447,8 @@ class OpObserverRetryableFindAndModifyInsidePreparedRetryableInternalTransaction
public:
void setUp() override {
OpObserverRetryableFindAndModifyTest::setUp();
OpObserverTxnParticipantTest::setUp();
OpObserverTxnParticipantTest::setUpRetryableInternalTransaction();
setUpObserverContext();
setUpRetryableInternalTransaction();
}
protected:

View File

@ -361,20 +361,13 @@ public:
_testExecutor = makeTestExecutor();
}
void tearDown() override {
// Ensure that even on test failures all failpoint state gets reset.
globalFailPointRegistry().disableAllFailpoints();
WaitForMajorityService::get(getServiceContext()).shutDown();
void shutdownHook() override {
_testExecutor->shutdown();
_testExecutor->join();
_testExecutor.reset();
_registry->onShutdown();
_service = nullptr;
ServiceContextMongoDTest::tearDown();
}
void stepUp() {

View File

@ -90,17 +90,6 @@ void PrimaryOnlyServiceMongoDTest::setUp() {
}
}
void PrimaryOnlyServiceMongoDTest::tearDown() {
// Ensure that even on test failures all failpoint state gets reset.
globalFailPointRegistry().disableAllFailpoints();
WaitForMajorityService::get(getServiceContext()).shutDown();
shutdown();
ServiceContextMongoDTest::tearDown();
}
void PrimaryOnlyServiceMongoDTest::startup(OperationContext* opCtx) {
_registry->onStartup(opCtx);
}
@ -125,6 +114,21 @@ PrimaryOnlyServiceMongoDTest::makeReplicationCoordinator() {
return std::make_unique<repl::ReplicationCoordinatorMock>(getServiceContext());
}
void PrimaryOnlyServiceMongoDTest::tearDown() {
// Ensure that even on test failures all failpoint state gets reset.
globalFailPointRegistry().disableAllFailpoints();
WaitForMajorityService::get(getServiceContext()).shutDown();
shutdownHook();
ServiceContextMongoDTest::tearDown();
}
void PrimaryOnlyServiceMongoDTest::shutdownHook() {
shutdown();
}
void stepUp(OperationContext* opCtx,
ServiceContext* serviceCtx,
repl::PrimaryOnlyServiceRegistry* registry,

View File

@ -88,6 +88,8 @@ protected:
*/
virtual void setUpPersistence(OperationContext* opCtx){};
virtual void shutdownHook();
OpObserverRegistry* _opObserverRegistry = nullptr;
repl::PrimaryOnlyServiceRegistry* _registry = nullptr;
repl::PrimaryOnlyService* _service = nullptr;

View File

@ -185,16 +185,9 @@ public:
ASSERT(_service);
}
void tearDown() override {
// Ensure that even on test failures all failpoint state gets reset.
globalFailPointRegistry().disableAllFailpoints();
WaitForMajorityService::get(getServiceContext()).shutDown();
void shutdownHook() override {
_registry->onShutdown();
_service = nullptr;
ServiceContextMongoDTest::tearDown();
}
void stepUp() {

View File

@ -1577,6 +1577,21 @@ protected:
std::shared_ptr<OperationContext> opCtx;
};
virtual void initAndStart() {
init("mySet/test1:1234,test2:1234,test3:1234");
assertStartSuccess(BSON("_id"
<< "mySet"
<< "version" << 1 << "members"
<< BSON_ARRAY(BSON("_id" << 0 << "host"
<< "test1:1234")
<< BSON("_id" << 1 << "host"
<< "test2:1234")
<< BSON("_id" << 2 << "host"
<< "test3:1234"))),
HostAndPort("test1", 1234));
ASSERT_OK(getReplCoord()->setFollowerMode(MemberState::RS_SECONDARY));
}
std::pair<SharedClientAndOperation, stdx::future<boost::optional<Status>>> stepDown_nonBlocking(
bool force, Milliseconds waitTime, Milliseconds stepDownTime) {
using PromisedClientAndOperation = stdx::promise<SharedClientAndOperation>;
@ -1647,18 +1662,7 @@ protected:
private:
virtual void setUp() {
ReplCoordTest::setUp();
init("mySet/test1:1234,test2:1234,test3:1234");
assertStartSuccess(BSON("_id"
<< "mySet"
<< "version" << 1 << "members"
<< BSON_ARRAY(BSON("_id" << 0 << "host"
<< "test1:1234")
<< BSON("_id" << 1 << "host"
<< "test2:1234")
<< BSON("_id" << 2 << "host"
<< "test3:1234"))),
HostAndPort("test1", 1234));
ASSERT_OK(getReplCoord()->setFollowerMode(MemberState::RS_SECONDARY));
initAndStart();
}
};
@ -2077,9 +2081,8 @@ TEST_F(StepDownTest, StepDownFailureRestoresDrainState) {
}
class StepDownTestWithUnelectableNode : public StepDownTest {
private:
void setUp() override {
ReplCoordTest::setUp();
protected:
void initAndStart() override {
init("mySet/test1:1234,test2:1234,test3:1234");
assertStartSuccess(BSON("_id"
<< "mySet"
@ -2291,11 +2294,8 @@ protected:
}
}
private:
virtual void setUp() {
ReplCoordTest::setUp();
void initAndStart() override {
init("mySet/test1:1234,test2:1234,test3:1234,test4:1234,test5:1234");
assertStartSuccess(BSON("_id"
<< "mySet"
<< "version" << 1 << "members"

View File

@ -303,34 +303,17 @@ TEST_F(CollectionShardingRuntimeTest, ReturnUnshardedMetadataInServerlessMode) {
setGlobalReplSettings(originalRs);
}
class CollectionShardingRuntimeTestWithMockedLoader : public ShardServerTestFixture {
class CollectionShardingRuntimeTestWithMockedLoader
: public ShardServerTestFixtureWithCatalogCacheLoaderMock {
public:
const NamespaceString kNss = NamespaceString::createNamespaceString_forTest("test.foo");
const UUID kCollUUID = UUID::gen();
const std::string kShardKey = "x";
const HostAndPort kConfigHostAndPort{"DummyConfig", 12345};
const std::vector<ShardType> kShardList = {ShardType("shard0", "Host0:12345")};
const std::vector<ShardType> kShardList = {ShardType(_myShardName.toString(), "Host0:12345")};
void setUp() override {
// Don't call ShardServerTestFixture::setUp so we can install a mock catalog cache
// loader.
ShardingMongodTestFixture::setUp();
replicationCoordinator()->alwaysAllowWrites(true);
serverGlobalParams.clusterRole = ClusterRole::ShardServer;
_clusterId = OID::gen();
ShardingState::get(getServiceContext())
->setInitialized(kShardList[0].getName(), _clusterId);
auto mockLoader = std::make_unique<CatalogCacheLoaderMock>();
_mockCatalogCacheLoader = mockLoader.get();
CatalogCacheLoader::set(getServiceContext(), std::move(mockLoader));
uassertStatusOK(
initializeGlobalShardingStateForMongodForTest(ConnectionString(kConfigHostAndPort)));
configTargeterMock()->setFindHostReturnValue(kConfigHostAndPort);
ShardServerTestFixtureWithCatalogCacheLoaderMock::setUp();
WaitForMajorityService::get(getServiceContext()).startup(getServiceContext());
@ -347,7 +330,7 @@ public:
void tearDown() override {
WaitForMajorityService::get(getServiceContext()).shutDown();
ShardServerTestFixture::tearDown();
ShardServerTestFixtureWithCatalogCacheLoaderMock::tearDown();
}
class StaticCatalogClient final : public ShardingCatalogClientMock {
@ -398,9 +381,6 @@ public:
return {chunk1, chunk2};
}
protected:
CatalogCacheLoaderMock* _mockCatalogCacheLoader;
};
/**

View File

@ -73,33 +73,16 @@
namespace mongo {
namespace {
class DatabaseShardingStateTestWithMockedLoader : public ShardServerTestFixture {
class DatabaseShardingStateTestWithMockedLoader
: public ShardServerTestFixtureWithCatalogCacheLoaderMock {
public:
const StringData kDbName{"test"};
const HostAndPort kConfigHostAndPort{"DummyConfig", 12345};
const std::vector<ShardType> kShardList = {ShardType("shard0", "Host0:12345")};
const std::vector<ShardType> kShardList = {ShardType(_myShardName.toString(), "Host0:12345")};
void setUp() override {
// Don't call ShardServerTestFixture::setUp so we can install a mock catalog cache
// loader.
ShardingMongodTestFixture::setUp();
replicationCoordinator()->alwaysAllowWrites(true);
serverGlobalParams.clusterRole = ClusterRole::ShardServer;
_clusterId = OID::gen();
ShardingState::get(getServiceContext())
->setInitialized(kShardList[0].getName(), _clusterId);
auto mockLoader = std::make_unique<CatalogCacheLoaderMock>();
_mockCatalogCacheLoader = mockLoader.get();
CatalogCacheLoader::set(getServiceContext(), std::move(mockLoader));
uassertStatusOK(
initializeGlobalShardingStateForMongodForTest(ConnectionString(kConfigHostAndPort)));
configTargeterMock()->setFindHostReturnValue(kConfigHostAndPort);
ShardServerTestFixtureWithCatalogCacheLoaderMock::setUp();
WaitForMajorityService::get(getServiceContext()).startup(getServiceContext());
@ -116,7 +99,7 @@ public:
void tearDown() override {
WaitForMajorityService::get(getServiceContext()).shutDown();
ShardServerTestFixture::tearDown();
ShardServerTestFixtureWithCatalogCacheLoaderMock::tearDown();
}
class StaticCatalogClient final : public ShardingCatalogClientMock {
@ -152,9 +135,6 @@ public:
return DatabaseType(
kDbName.toString(), kShardList[0].getName(), DatabaseVersion(uuid, timestamp));
}
protected:
CatalogCacheLoaderMock* _mockCatalogCacheLoader;
};
TEST_F(DatabaseShardingStateTestWithMockedLoader, OnDbVersionMismatch) {
@ -173,7 +153,7 @@ TEST_F(DatabaseShardingStateTestWithMockedLoader, OnDbVersionMismatch) {
return scopedDss->getDbVersion(opCtx);
};
_mockCatalogCacheLoader->setDatabaseRefreshReturnValue(newDb);
getCatalogCacheLoaderMock()->setDatabaseRefreshReturnValue(newDb);
ASSERT_OK(onDbVersionMismatchNoExcept(opCtx, kDbName, newDbVersion));
auto activeDbVersion = getActiveDbVersion();
@ -198,7 +178,7 @@ TEST_F(DatabaseShardingStateTestWithMockedLoader, ForceDatabaseRefresh) {
const auto newDbVersion = newDb.getVersion();
auto opCtx = operationContext();
_mockCatalogCacheLoader->setDatabaseRefreshReturnValue(newDb);
getCatalogCacheLoaderMock()->setDatabaseRefreshReturnValue(newDb);
ASSERT_OK(onDbVersionMismatchNoExcept(opCtx, kDbName, boost::none));
boost::optional<DatabaseVersion> activeDbVersion = [&] {

View File

@ -141,33 +141,16 @@ void runInTransaction(OperationContext* opCtx, Callable&& func) {
txnParticipant.stashTransactionResources(opCtx);
}
class DestinedRecipientTest : public ShardServerTestFixture {
class DestinedRecipientTest : public ShardServerTestFixtureWithCatalogCacheLoaderMock {
public:
const NamespaceString kNss = NamespaceString::createNamespaceString_forTest("test.foo");
const std::string kShardKey = "x";
const HostAndPort kConfigHostAndPort{"DummyConfig", 12345};
const std::vector<ShardType> kShardList = {ShardType("shard0", "Host0:12345"),
const std::vector<ShardType> kShardList = {ShardType(_myShardName.toString(), "Host0:12345"),
ShardType("shard1", "Host1:12345")};
void setUp() override {
// Don't call ShardServerTestFixture::setUp so we can install a mock catalog cache loader.
ShardingMongodTestFixture::setUp();
replicationCoordinator()->alwaysAllowWrites(true);
serverGlobalParams.clusterRole = ClusterRole::ShardServer;
_clusterId = OID::gen();
ShardingState::get(getServiceContext())
->setInitialized(kShardList[0].getName(), _clusterId);
auto mockLoader = std::make_unique<CatalogCacheLoaderMock>();
_mockCatalogCacheLoader = mockLoader.get();
CatalogCacheLoader::set(getServiceContext(), std::move(mockLoader));
uassertStatusOK(
initializeGlobalShardingStateForMongodForTest(ConnectionString(kConfigHostAndPort)));
configTargeterMock()->setFindHostReturnValue(kConfigHostAndPort);
ShardServerTestFixtureWithCatalogCacheLoaderMock::setUp();
WaitForMajorityService::get(getServiceContext()).startup(getServiceContext());
@ -184,7 +167,7 @@ public:
void tearDown() override {
WaitForMajorityService::get(getServiceContext()).shutDown();
ShardServerTestFixture::tearDown();
ShardServerTestFixtureWithCatalogCacheLoaderMock::tearDown();
}
class StaticCatalogClient final : public ShardingCatalogClientMock {
@ -294,9 +277,9 @@ protected:
BSON(kShardKey << 1));
coll.setAllowMigrations(false);
_mockCatalogCacheLoader->setDatabaseRefreshReturnValue(
getCatalogCacheLoaderMock()->setDatabaseRefreshReturnValue(
DatabaseType(kNss.db_forTest().toString(), kShardList[0].getName(), env.dbVersion));
_mockCatalogCacheLoader->setCollectionRefreshValues(
getCatalogCacheLoaderMock()->setCollectionRefreshValues(
kNss,
coll,
createChunks(env.version.placementVersion().epoch(),
@ -304,7 +287,7 @@ protected:
env.version.placementVersion().getTimestamp(),
kShardKey),
reshardingFields);
_mockCatalogCacheLoader->setCollectionRefreshValues(
getCatalogCacheLoaderMock()->setCollectionRefreshValues(
env.tempNss,
coll,
createChunks(env.version.placementVersion().epoch(),
@ -366,9 +349,6 @@ protected:
const auto& doc = unittest::assertGet(oplogIter->next()).first;
return unittest::assertGet(repl::OplogEntry::parse(doc));
}
protected:
CatalogCacheLoaderMock* _mockCatalogCacheLoader;
};
TEST_F(DestinedRecipientTest, TestGetDestinedRecipient) {

View File

@ -140,35 +140,20 @@
namespace mongo {
namespace {
class ReshardingTxnClonerTest : public ShardServerTestFixture {
class ReshardingTxnClonerTest : public ShardServerTestFixtureWithCatalogCacheLoaderMock {
void setUp() {
// Don't call ShardServerTestFixture::setUp so we can install a mock catalog cache loader.
ShardingMongodTestFixture::setUp();
replicationCoordinator()->alwaysAllowWrites(true);
serverGlobalParams.clusterRole = ClusterRole::ShardServer;
_clusterId = OID::gen();
ShardingState::get(getServiceContext())->setInitialized(kTwoShardIdList[0], _clusterId);
auto mockLoader = std::make_unique<CatalogCacheLoaderMock>();
ShardServerTestFixtureWithCatalogCacheLoaderMock::setUp();
// The config database's primary shard is always config, and it is always sharded.
mockLoader->setDatabaseRefreshReturnValue(DatabaseType{DatabaseName::kConfig.toString(),
ShardId::kConfigServerId,
DatabaseVersion::makeFixed()});
getCatalogCacheLoaderMock()->setDatabaseRefreshReturnValue(
DatabaseType{DatabaseName::kConfig.toString(),
ShardId::kConfigServerId,
DatabaseVersion::makeFixed()});
// The config.transactions collection is always unsharded.
mockLoader->setCollectionRefreshReturnValue(
getCatalogCacheLoaderMock()->setCollectionRefreshReturnValue(
{ErrorCodes::NamespaceNotFound, "collection not found"});
CatalogCacheLoader::set(getServiceContext(), std::move(mockLoader));
uassertStatusOK(
initializeGlobalShardingStateForMongodForTest(ConnectionString(kConfigHostAndPort)));
configTargeterMock()->setFindHostReturnValue(kConfigHostAndPort);
for (const auto& shardId : kTwoShardIdList) {
auto shardTargeter = RemoteCommandTargeterMock::get(
uassertStatusOK(shardRegistry()->getShard(operationContext(), shardId))
@ -188,7 +173,7 @@ class ReshardingTxnClonerTest : public ShardServerTestFixture {
void tearDown() {
WaitForMajorityService::get(getServiceContext()).shutDown();
ShardServerTestFixture::tearDown();
ShardServerTestFixtureWithCatalogCacheLoaderMock::tearDown();
}
/**

View File

@ -117,4 +117,24 @@ CatalogCacheMock* ShardServerTestFixtureWithCatalogCacheMock::getCatalogCacheMoc
return static_cast<CatalogCacheMock*>(catalogCache());
}
CatalogCacheLoaderMock* ShardServerTestFixtureWithCatalogCacheMock::getCatalogCacheLoaderMock() {
return _cacheLoaderMock;
}
void ShardServerTestFixtureWithCatalogCacheLoaderMock::setUp() {
auto loader = std::make_unique<CatalogCacheLoaderMock>();
_cacheLoaderMock = loader.get();
setCatalogCacheLoader(std::move(loader));
ShardServerTestFixture::setUp();
}
CatalogCacheMock* ShardServerTestFixtureWithCatalogCacheLoaderMock::getCatalogCacheMock() {
return static_cast<CatalogCacheMock*>(catalogCache());
}
CatalogCacheLoaderMock*
ShardServerTestFixtureWithCatalogCacheLoaderMock::getCatalogCacheLoaderMock() {
return _cacheLoaderMock;
}
} // namespace mongo

View File

@ -85,6 +85,17 @@ protected:
void setUp() override;
virtual std::unique_ptr<CatalogCache> makeCatalogCache() override;
CatalogCacheMock* getCatalogCacheMock();
CatalogCacheLoaderMock* getCatalogCacheLoaderMock();
private:
CatalogCacheLoaderMock* _cacheLoaderMock;
};
class ShardServerTestFixtureWithCatalogCacheLoaderMock : public ShardServerTestFixture {
protected:
void setUp() override;
CatalogCacheMock* getCatalogCacheMock();
CatalogCacheLoaderMock* getCatalogCacheLoaderMock();
private:
CatalogCacheLoaderMock* _cacheLoaderMock;

View File

@ -34,6 +34,7 @@
#include <ostream>
#include <string>
#include <utility>
#include <vector>
#include <boost/move/utility_core.hpp>
#include <boost/optional/optional.hpp>
@ -92,6 +93,8 @@ protected:
};
void setUp() override;
virtual void preSetUp();
virtual std::vector<NamespaceString> getNamespaceStrings();
std::pair<ServiceContext::UniqueClient, ServiceContext::UniqueOperationContext>
_makeOperationContext();
@ -137,10 +140,11 @@ protected:
class BucketCatalogInMultitenancyEnv : public BucketCatalogTest {
protected:
void setUp() override;
void preSetUp() override;
std::vector<NamespaceString> getNamespaceStrings() override;
private:
boost::optional<RAIIServerParameterControllerForTest> __multitenancyController;
boost::optional<RAIIServerParameterControllerForTest> _multitenancyController;
protected:
NamespaceString _tenant1Ns1 =
@ -149,13 +153,21 @@ protected:
NamespaceString::createNamespaceString_forTest({TenantId(OID::gen())}, "db1", "coll1");
};
void BucketCatalogTest::preSetUp() {}
std::vector<NamespaceString> BucketCatalogTest::getNamespaceStrings() {
return {_ns1, _ns2, _ns3};
}
void BucketCatalogTest::setUp() {
preSetUp();
CatalogTestFixture::setUp();
_opCtx = operationContext();
_bucketCatalog = &BucketCatalog::get(_opCtx);
for (const auto& ns : {_ns1, _ns2, _ns3}) {
const auto namespaceStrings = getNamespaceStrings();
for (const auto& ns : namespaceStrings) {
ASSERT_OK(createCollection(
_opCtx,
ns.dbName(),
@ -163,19 +175,12 @@ void BucketCatalogTest::setUp() {
}
}
void BucketCatalogInMultitenancyEnv::setUp() {
__multitenancyController.emplace("multitenancySupport", true);
CatalogTestFixture::setUp();
void BucketCatalogInMultitenancyEnv::preSetUp() {
_multitenancyController.emplace("multitenancySupport", true);
}
_opCtx = operationContext();
_bucketCatalog = &BucketCatalog::get(_opCtx);
for (const auto& ns : {_tenant1Ns1, _tenant2Ns1}) {
ASSERT_OK(createCollection(
_opCtx,
ns.dbName(),
BSON("create" << ns.coll() << "timeseries" << _makeTimeseriesOptionsForCreate())));
}
std::vector<NamespaceString> BucketCatalogInMultitenancyEnv::getNamespaceStrings() {
return {_tenant1Ns1, _tenant2Ns1};
}
BucketCatalogTest::RunBackgroundTaskAndWaitForFailpoint::RunBackgroundTaskAndWaitForFailpoint(