From c5c4a7afc71b7fded0fe3f18db037ddaa3754857 Mon Sep 17 00:00:00 2001 From: NeaguGeorgiana23 Date: Tue, 12 May 2026 09:18:13 +0000 Subject: [PATCH 1/5] Fix old provider not shutting down when the new provider fails to initialize. Signed-off-by: NeaguGeorgiana23 --- openfeature/provider_repository.cpp | 4 +--- test/provider_repository_test.cpp | 6 +++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/openfeature/provider_repository.cpp b/openfeature/provider_repository.cpp index e15bb88..6ff5a7b 100644 --- a/openfeature/provider_repository.cpp +++ b/openfeature/provider_repository.cpp @@ -185,9 +185,7 @@ void ProviderRepository::InitializeProvider( new_status_manager->Init(ctx); } - if (new_status_manager->GetStatus() == ProviderStatus::kReady) { - ShutdownOldProvider(old_status_manager); - } + ShutdownOldProvider(old_status_manager); } void ProviderRepository::ShutdownOldProvider( diff --git a/test/provider_repository_test.cpp b/test/provider_repository_test.cpp index d7ef4c8..0fefa62 100644 --- a/test/provider_repository_test.cpp +++ b/test/provider_repository_test.cpp @@ -226,9 +226,9 @@ TEST_F(ProviderRepositoryTest, OldProviderIsShutdownAfterNewOneIsReady) { EXPECT_EQ(repo.GetProviderStatus(), ProviderStatus::kReady); } -// Test to verify the old provider is not shut down if the new one fails to +// Test to verify the old provider is shut down if the new one fails to // init. -TEST_F(ProviderRepositoryTest, OldProviderIsNotShutdownIfNewOneFailsToInit) { +TEST_F(ProviderRepositoryTest, OldProviderIsShutdownIfNewOneFailsToInit) { std::shared_ptr mock_provider1 = std::make_shared(); std::shared_ptr mock_provider2 = @@ -239,7 +239,7 @@ TEST_F(ProviderRepositoryTest, OldProviderIsNotShutdownIfNewOneFailsToInit) { EXPECT_CALL(*mock_provider2, Init(_)) .WillOnce(Return(absl::InternalError("Init failed"))); - EXPECT_CALL(*mock_provider1, Shutdown()).Times(0); + EXPECT_CALL(*mock_provider1, Shutdown()).WillOnce(Return(absl::OkStatus())); repo.SetProvider(mock_provider2, ctx, true); From e09a8020385a1b039197e694d39a6d1cb3c87f5a Mon Sep 17 00:00:00 2001 From: NeaguGeorgiana23 Date: Tue, 12 May 2026 09:41:04 +0000 Subject: [PATCH 2/5] Fix linter errors. Signed-off-by: NeaguGeorgiana23 --- openfeature/provider_repository.cpp | 32 ++++++++++++++--------------- openfeature/provider_repository.h | 8 ++++---- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/openfeature/provider_repository.cpp b/openfeature/provider_repository.cpp index 6ff5a7b..b7d08fc 100644 --- a/openfeature/provider_repository.cpp +++ b/openfeature/provider_repository.cpp @@ -30,9 +30,9 @@ ProviderRepository::GetFeatureProviderStatusManager( return default_manager_; } - auto it = provider_manager_.find(std::string(domain)); - if (it != provider_manager_.end()) { - return it->second; + auto provider_manager_it = provider_manager_.find(std::string(domain)); + if (provider_manager_it != provider_manager_.end()) { + return provider_manager_it->second; } return default_manager_; @@ -53,7 +53,7 @@ void ProviderRepository::SetProvider(std::shared_ptr provider, const EvaluationContext& ctx, bool wait_for_init) { if (!provider) { - std::cerr << "Provider cannot be null" << std::endl; + std::cerr << "Provider cannot be null\n"; return; } PrepareAndInitializeProvider(std::nullopt, std::move(provider), ctx, @@ -65,7 +65,7 @@ void ProviderRepository::SetProvider(std::string_view domain, const EvaluationContext& ctx, bool wait_for_init) { if (!provider) { - std::cerr << "Provider cannot be null" << std::endl; + std::cerr << "Provider cannot be null\n"; return; } @@ -90,7 +90,7 @@ ProviderStatus ProviderRepository::GetProviderStatus( void ProviderRepository::Shutdown() { { - std::lock_guard lock(threads_mutex_); + std::scoped_lock lock(threads_mutex_); for (std::thread& thread : initialization_threads_) { if (thread.joinable()) { thread.join(); @@ -128,9 +128,9 @@ void ProviderRepository::Shutdown() { } void ProviderRepository::PrepareAndInitializeProvider( - const std::optional domain, - std::shared_ptr new_provider, const EvaluationContext& ctx, - bool wait_for_init) { + const std::optional& domain, + const std::shared_ptr& new_provider, + const EvaluationContext& ctx, bool wait_for_init) { std::shared_ptr new_status_manager; std::shared_ptr old_status_manager; @@ -144,7 +144,7 @@ void ProviderRepository::PrepareAndInitializeProvider( FeatureProviderStatusManager::Create(new_provider); if (!manager.ok()) { std::cerr << "Failed to create FeatureProviderStatusManager: " - << manager.status() << std::endl; + << manager.status() << "\n"; return; } new_status_manager = std::move(manager.value()); @@ -154,9 +154,9 @@ void ProviderRepository::PrepareAndInitializeProvider( if (domain) { // Setting a named provider. - auto it = provider_manager_.find(domain.value()); - if (it != provider_manager_.end()) { - old_status_manager = it->second; + auto provider_manager_it = provider_manager_.find(domain.value()); + if (provider_manager_it != provider_manager_.end()) { + old_status_manager = provider_manager_it->second; } provider_manager_[domain.value()] = new_status_manager; } else { @@ -169,7 +169,7 @@ void ProviderRepository::PrepareAndInitializeProvider( if (wait_for_init) { InitializeProvider(new_status_manager, old_status_manager, ctx); } else { - std::lock_guard lock(threads_mutex_); + std::scoped_lock lock(threads_mutex_); initialization_threads_.emplace_back( [this, new_status_manager, old_status_manager, ctx] { InitializeProvider(new_status_manager, old_status_manager, ctx); @@ -178,8 +178,8 @@ void ProviderRepository::PrepareAndInitializeProvider( } void ProviderRepository::InitializeProvider( - std::shared_ptr new_status_manager, - std::shared_ptr old_status_manager, + const std::shared_ptr& new_status_manager, + const std::shared_ptr& old_status_manager, const EvaluationContext& ctx) { if (new_status_manager->GetStatus() == ProviderStatus::kNotReady) { new_status_manager->Init(ctx); diff --git a/openfeature/provider_repository.h b/openfeature/provider_repository.h index 6db23d4..4888022 100644 --- a/openfeature/provider_repository.h +++ b/openfeature/provider_repository.h @@ -63,13 +63,13 @@ class ProviderRepository { private: void PrepareAndInitializeProvider( - const std::optional domain, - std::shared_ptr new_provider, + const std::optional& domain, + const std::shared_ptr& new_provider, const EvaluationContext& ctx, bool waitForInit); void InitializeProvider( - std::shared_ptr new_status_manager, - std::shared_ptr old_status_manager, + const std::shared_ptr& new_status_manager, + const std::shared_ptr& old_status_manager, const EvaluationContext& ctx); void ShutdownOldProvider( From 74db796f2c0e7b818d35f005e406970ef5dd58af Mon Sep 17 00:00:00 2001 From: NeaguGeorgiana23 Date: Tue, 12 May 2026 09:57:24 +0000 Subject: [PATCH 3/5] Fix linter errors. Signed-off-by: NeaguGeorgiana23 --- openfeature/provider_repository.cpp | 32 +++++++++++++---------------- openfeature/provider_repository.h | 12 +++++------ test/provider_repository_test.cpp | 7 ++++++- 3 files changed, 26 insertions(+), 25 deletions(-) diff --git a/openfeature/provider_repository.cpp b/openfeature/provider_repository.cpp index b7d08fc..695d4c7 100644 --- a/openfeature/provider_repository.cpp +++ b/openfeature/provider_repository.cpp @@ -1,5 +1,6 @@ #include "openfeature/provider_repository.h" +#include #include #include "absl/status/statusor.h" @@ -49,32 +50,30 @@ std::shared_ptr ProviderRepository::GetProvider( return nullptr; } -void ProviderRepository::SetProvider(std::shared_ptr provider, - const EvaluationContext& ctx, - bool wait_for_init) { +void ProviderRepository::SetProvider( + const std::shared_ptr& provider, + const EvaluationContext& ctx, bool wait_for_init) { if (!provider) { std::cerr << "Provider cannot be null\n"; return; } - PrepareAndInitializeProvider(std::nullopt, std::move(provider), ctx, - wait_for_init); + PrepareAndInitializeProvider(std::nullopt, provider, ctx, wait_for_init); } -void ProviderRepository::SetProvider(std::string_view domain, - std::shared_ptr provider, - const EvaluationContext& ctx, - bool wait_for_init) { +void ProviderRepository::SetProvider( + std::string_view domain, const std::shared_ptr& provider, + const EvaluationContext& ctx, bool wait_for_init) { if (!provider) { std::cerr << "Provider cannot be null\n"; return; } if (domain.empty()) { - SetProvider(std::move(provider), ctx, wait_for_init); + SetProvider(provider, ctx, wait_for_init); return; } - PrepareAndInitializeProvider(std::string(domain), std::move(provider), ctx, + PrepareAndInitializeProvider(std::string(domain), provider, ctx, wait_for_init); } @@ -189,7 +188,7 @@ void ProviderRepository::InitializeProvider( } void ProviderRepository::ShutdownOldProvider( - std::shared_ptr old_status_manager) { + const std::shared_ptr& old_status_manager) { if (old_status_manager) { std::shared_lock lock(repo_mutex_); @@ -219,12 +218,9 @@ bool ProviderRepository::IsStatusManagerRegistered( if (default_manager_ == manager) { return true; } - for (const auto& pair : provider_manager_) { - if (pair.second == manager) { - return true; - } - } - return false; + return std::any_of( + provider_manager_.begin(), provider_manager_.end(), + [&manager](const auto& pair) { return pair.second == manager; }); } } // namespace openfeature diff --git a/openfeature/provider_repository.h b/openfeature/provider_repository.h index 4888022..ad82a6e 100644 --- a/openfeature/provider_repository.h +++ b/openfeature/provider_repository.h @@ -43,13 +43,13 @@ class ProviderRepository { std::string_view domain = "") const; // Set the default provider. - void SetProvider(std::shared_ptr provider, - const EvaluationContext& ctx, bool waitForInit); + void SetProvider(const std::shared_ptr& provider, + const EvaluationContext& ctx, bool wait_for_init); // Add a provider for a domain. void SetProvider(std::string_view domain, - std::shared_ptr provider, - const EvaluationContext& ctx, bool waitForInit); + const std::shared_ptr& provider, + const EvaluationContext& ctx, bool wait_for_init); // Fetch the status of a provider for a domain. // If the domain is not set, return the default provider status. @@ -65,7 +65,7 @@ class ProviderRepository { void PrepareAndInitializeProvider( const std::optional& domain, const std::shared_ptr& new_provider, - const EvaluationContext& ctx, bool waitForInit); + const EvaluationContext& ctx, bool wait_for_init); void InitializeProvider( const std::shared_ptr& new_status_manager, @@ -73,7 +73,7 @@ class ProviderRepository { const EvaluationContext& ctx); void ShutdownOldProvider( - std::shared_ptr old_status_manager); + const std::shared_ptr& old_status_manager); std::shared_ptr GetExistingStatusManagerForProvider( diff --git a/test/provider_repository_test.cpp b/test/provider_repository_test.cpp index 0fefa62..8f4303f 100644 --- a/test/provider_repository_test.cpp +++ b/test/provider_repository_test.cpp @@ -9,7 +9,12 @@ #include "mocks/mock_feature_provider.h" #include "openfeature/noop_provider.h" -using namespace openfeature; +using ::openfeature::EvaluationContext; +using ::openfeature::FeatureProvider; +using ::openfeature::FeatureProviderStatusManager; +using ::openfeature::NoopProvider; +using ::openfeature::ProviderRepository; +using ::openfeature::ProviderStatus; using ::testing::_; using ::testing::InSequence; using ::testing::Return; From af3cd35e2d42fad6c89e91ed5996f64bc6511515 Mon Sep 17 00:00:00 2001 From: NeaguGeorgiana23 Date: Tue, 12 May 2026 09:59:04 +0000 Subject: [PATCH 4/5] Fix linter errors. Signed-off-by: NeaguGeorgiana23 --- test/provider_repository_test.cpp | 140 +++++++++++++++--------------- 1 file changed, 71 insertions(+), 69 deletions(-) diff --git a/test/provider_repository_test.cpp b/test/provider_repository_test.cpp index 8f4303f..9e799e4 100644 --- a/test/provider_repository_test.cpp +++ b/test/provider_repository_test.cpp @@ -12,6 +12,7 @@ using ::openfeature::EvaluationContext; using ::openfeature::FeatureProvider; using ::openfeature::FeatureProviderStatusManager; +using ::openfeature::MockFeatureProvider; using ::openfeature::NoopProvider; using ::openfeature::ProviderRepository; using ::openfeature::ProviderStatus; @@ -21,17 +22,17 @@ using ::testing::Return; class ProviderRepositoryTest : public ::testing::Test { protected: - ProviderRepository repo; - EvaluationContext ctx = EvaluationContext::Builder().build(); + ProviderRepository repo_; + EvaluationContext ctx_ = EvaluationContext::Builder().build(); }; // Test to verify the constructor initializes with a NoopProvider. TEST_F(ProviderRepositoryTest, ConstructorInitializesWithNoopProvider) { - std::shared_ptr provider = repo.GetProvider(); + std::shared_ptr provider = repo_.GetProvider(); ASSERT_NE(provider, nullptr); EXPECT_NE(dynamic_cast(provider.get()), nullptr); - EXPECT_EQ(repo.GetProvider()->GetMetadata().name, "Noop Provider"); - EXPECT_EQ(repo.GetProviderStatus(), ProviderStatus::kReady); + EXPECT_EQ(repo_.GetProvider()->GetMetadata().name, "Noop Provider"); + EXPECT_EQ(repo_.GetProviderStatus(), ProviderStatus::kReady); } // Test to verify that GetFeatureProviderStatusManager returns the correct @@ -40,12 +41,12 @@ TEST_F(ProviderRepositoryTest, GetFeatureProviderStatusManagerReturnsCorrectManager) { // On initialization, it should return the default manager. std::shared_ptr initial_default_manager = - repo.GetFeatureProviderStatusManager(); + repo_.GetFeatureProviderStatusManager(); ASSERT_NE(initial_default_manager, nullptr); // Asking for a non-existent domain should return the default manager. std::shared_ptr non_existent_manager = - repo.GetFeatureProviderStatusManager("non-existent"); + repo_.GetFeatureProviderStatusManager("non-existent"); EXPECT_EQ(non_existent_manager, initial_default_manager); // After setting a named provider, it should return the new manager. @@ -53,32 +54,33 @@ TEST_F(ProviderRepositoryTest, std::make_shared(); std::string domain = "my-domain"; EXPECT_CALL(*mock_provider, Init(_)).WillOnce(Return(absl::OkStatus())); - repo.SetProvider(domain, mock_provider, ctx, true); + repo_.SetProvider(domain, mock_provider, ctx_, true); std::shared_ptr named_manager = - repo.GetFeatureProviderStatusManager(domain); + repo_.GetFeatureProviderStatusManager(domain); ASSERT_NE(named_manager, nullptr); EXPECT_NE(named_manager, initial_default_manager); EXPECT_EQ(named_manager->GetProvider(), mock_provider); // Getting the default manager should still return the original one. std::shared_ptr current_default_manager = - repo.GetFeatureProviderStatusManager(); + repo_.GetFeatureProviderStatusManager(); EXPECT_EQ(current_default_manager, initial_default_manager); } // Test to verify GetProvider returns the default when the domain is empty. TEST_F(ProviderRepositoryTest, GetProviderReturnsDefaultWhenDomainIsEmpty) { - std::shared_ptr default_provider = repo.GetProvider(); - std::shared_ptr empty_domain_provider = repo.GetProvider(""); + std::shared_ptr default_provider = repo_.GetProvider(); + std::shared_ptr empty_domain_provider = + repo_.GetProvider(""); EXPECT_EQ(default_provider, empty_domain_provider); } // Test to verify GetProvider returns the default when the domain is not found. TEST_F(ProviderRepositoryTest, GetProviderReturnsDefaultWhenDomainNotFound) { - std::shared_ptr default_provider = repo.GetProvider(); + std::shared_ptr default_provider = repo_.GetProvider(); std::shared_ptr not_found_provider = - repo.GetProvider("non-existent-domain"); + repo_.GetProvider("non-existent-domain"); EXPECT_EQ(default_provider, not_found_provider); } @@ -90,10 +92,10 @@ TEST_F(ProviderRepositoryTest, SetAndGetNamedProvider) { EXPECT_CALL(*mock_provider, Init(_)).WillOnce(Return(absl::OkStatus())); - repo.SetProvider(domain, mock_provider, ctx, true); + repo_.SetProvider(domain, mock_provider, ctx_, true); - EXPECT_EQ(repo.GetProvider(domain), mock_provider); - EXPECT_NE(dynamic_cast(repo.GetProvider().get()), nullptr); + EXPECT_EQ(repo_.GetProvider(domain), mock_provider); + EXPECT_NE(dynamic_cast(repo_.GetProvider().get()), nullptr); } // Test to verify that setting the default provider replaces the NoopProvider. @@ -103,23 +105,23 @@ TEST_F(ProviderRepositoryTest, SetDefaultProviderReplacesNoop) { EXPECT_CALL(*mock_provider, Init(_)).WillOnce(Return(absl::OkStatus())); - repo.SetProvider(mock_provider, ctx, true); + repo_.SetProvider(mock_provider, ctx_, true); - EXPECT_EQ(repo.GetProvider(), mock_provider); + EXPECT_EQ(repo_.GetProvider(), mock_provider); } // Test that setting a null provider for the default does not change the state. TEST_F(ProviderRepositoryTest, SetProviderWithNullProviderDoesNothing) { - std::shared_ptr initial_provider = repo.GetProvider(); - ProviderStatus initial_status = repo.GetProviderStatus(); + std::shared_ptr initial_provider = repo_.GetProvider(); + ProviderStatus initial_status = repo_.GetProviderStatus(); testing::internal::CaptureStderr(); - repo.SetProvider(nullptr, ctx, true); + repo_.SetProvider(nullptr, ctx_, true); std::string output = testing::internal::GetCapturedStderr(); EXPECT_THAT(output, testing::HasSubstr("Provider cannot be null")); - EXPECT_EQ(repo.GetProvider(), initial_provider); - EXPECT_EQ(repo.GetProviderStatus(), initial_status); + EXPECT_EQ(repo_.GetProvider(), initial_provider); + EXPECT_EQ(repo_.GetProviderStatus(), initial_status); } // Test that setting a null provider for a named domain does not change the @@ -127,17 +129,17 @@ TEST_F(ProviderRepositoryTest, SetProviderWithNullProviderDoesNothing) { TEST_F(ProviderRepositoryTest, SetNamedProviderWithNullProviderDoesNothing) { const std::string domain = "my-domain"; std::shared_ptr initial_provider_for_domain = - repo.GetProvider(domain); - ProviderStatus initial_status = repo.GetProviderStatus(domain); + repo_.GetProvider(domain); + ProviderStatus initial_status = repo_.GetProviderStatus(domain); testing::internal::CaptureStderr(); - repo.SetProvider(domain, nullptr, ctx, true); + repo_.SetProvider(domain, nullptr, ctx_, true); std::string output = testing::internal::GetCapturedStderr(); EXPECT_THAT(output, testing::HasSubstr("Provider cannot be null")); - EXPECT_EQ(repo.GetProvider(domain), initial_provider_for_domain); - EXPECT_EQ(repo.GetProviderStatus(domain), initial_status); + EXPECT_EQ(repo_.GetProvider(domain), initial_provider_for_domain); + EXPECT_EQ(repo_.GetProviderStatus(domain), initial_status); } // Test to verify that SetProvider waits for initialization to complete. @@ -147,9 +149,9 @@ TEST_F(ProviderRepositoryTest, SetProviderWaitsForInitialization) { EXPECT_CALL(*mock_provider, Init(_)).WillOnce(Return(absl::OkStatus())); - repo.SetProvider(mock_provider, ctx, true); + repo_.SetProvider(mock_provider, ctx_, true); - EXPECT_EQ(repo.GetProviderStatus(), ProviderStatus::kReady); + EXPECT_EQ(repo_.GetProviderStatus(), ProviderStatus::kReady); } // Test to verify that SetProvider initializes asynchronously. @@ -165,14 +167,14 @@ TEST_F(ProviderRepositoryTest, SetProviderDoesNotWaitForInitialization) { return absl::OkStatus(); }); - repo.SetProvider(mock_provider, ctx, false); + repo_.SetProvider(mock_provider, ctx_, false); - EXPECT_EQ(repo.GetProviderStatus(), ProviderStatus::kNotReady); + EXPECT_EQ(repo_.GetProviderStatus(), ProviderStatus::kNotReady); init_can_complete.set_value(); std::this_thread::sleep_for(std::chrono::milliseconds(100)); - EXPECT_EQ(repo.GetProviderStatus(), ProviderStatus::kReady); + EXPECT_EQ(repo_.GetProviderStatus(), ProviderStatus::kReady); } // Test to verify that a failed initialization sets the status to kError. @@ -183,9 +185,9 @@ TEST_F(ProviderRepositoryTest, SetProviderWithFailedInitSetsErrorStatus) { EXPECT_CALL(*mock_provider, Init(_)) .WillOnce(Return(absl::InternalError("Init failed"))); - repo.SetProvider(mock_provider, ctx, true); + repo_.SetProvider(mock_provider, ctx_, true); - EXPECT_EQ(repo.GetProviderStatus(), ProviderStatus::kError); + EXPECT_EQ(repo_.GetProviderStatus(), ProviderStatus::kError); } // Test that Shutdown resets the repository to its initial @@ -194,18 +196,18 @@ TEST_F(ProviderRepositoryTest, ShutdownResetsToReadyNoopProvider) { // Set a mock provider to ensure we're not in the initial state. auto mock_provider = std::make_shared(); EXPECT_CALL(*mock_provider, Init(_)).WillOnce(Return(absl::OkStatus())); - repo.SetProvider(mock_provider, ctx, true); - ASSERT_EQ(repo.GetProvider(), mock_provider); + repo_.SetProvider(mock_provider, ctx_, true); + ASSERT_EQ(repo_.GetProvider(), mock_provider); // Expect the mock provider to be shut down. EXPECT_CALL(*mock_provider, Shutdown()).WillOnce(Return(absl::OkStatus())); - repo.Shutdown(); + repo_.Shutdown(); // After shutdown, the repository should be reset to the default NoopProvider. - auto provider = repo.GetProvider(); + auto provider = repo_.GetProvider(); ASSERT_NE(provider, nullptr); EXPECT_NE(dynamic_cast(provider.get()), nullptr); - EXPECT_EQ(repo.GetProviderStatus(), ProviderStatus::kReady); + EXPECT_EQ(repo_.GetProviderStatus(), ProviderStatus::kReady); } // Test to verify the old provider is shutdown after a new one is ready. @@ -216,9 +218,9 @@ TEST_F(ProviderRepositoryTest, OldProviderIsShutdownAfterNewOneIsReady) { std::make_shared(); EXPECT_CALL(*mock_provider1, Init(_)).WillOnce(Return(absl::OkStatus())); - repo.SetProvider(mock_provider1, ctx, true); - EXPECT_EQ(repo.GetProvider(), mock_provider1); - ASSERT_EQ(repo.GetProviderStatus(), ProviderStatus::kReady); + repo_.SetProvider(mock_provider1, ctx_, true); + EXPECT_EQ(repo_.GetProvider(), mock_provider1); + ASSERT_EQ(repo_.GetProviderStatus(), ProviderStatus::kReady); { InSequence seq; @@ -226,9 +228,9 @@ TEST_F(ProviderRepositoryTest, OldProviderIsShutdownAfterNewOneIsReady) { EXPECT_CALL(*mock_provider1, Shutdown()).WillOnce(Return(absl::OkStatus())); } - repo.SetProvider(mock_provider2, ctx, true); - EXPECT_EQ(repo.GetProvider(), mock_provider2); - EXPECT_EQ(repo.GetProviderStatus(), ProviderStatus::kReady); + repo_.SetProvider(mock_provider2, ctx_, true); + EXPECT_EQ(repo_.GetProvider(), mock_provider2); + EXPECT_EQ(repo_.GetProviderStatus(), ProviderStatus::kReady); } // Test to verify the old provider is shut down if the new one fails to @@ -240,17 +242,17 @@ TEST_F(ProviderRepositoryTest, OldProviderIsShutdownIfNewOneFailsToInit) { std::make_shared(); EXPECT_CALL(*mock_provider1, Init(_)).WillOnce(Return(absl::OkStatus())); - repo.SetProvider(mock_provider1, ctx, true); + repo_.SetProvider(mock_provider1, ctx_, true); EXPECT_CALL(*mock_provider2, Init(_)) .WillOnce(Return(absl::InternalError("Init failed"))); EXPECT_CALL(*mock_provider1, Shutdown()).WillOnce(Return(absl::OkStatus())); - repo.SetProvider(mock_provider2, ctx, true); + repo_.SetProvider(mock_provider2, ctx_, true); // The new provider should be set, but in an error state. - EXPECT_EQ(repo.GetProvider(), mock_provider2); - EXPECT_EQ(repo.GetProviderStatus(), ProviderStatus::kError); + EXPECT_EQ(repo_.GetProvider(), mock_provider2); + EXPECT_EQ(repo_.GetProviderStatus(), ProviderStatus::kError); } // Test to verify Shutdown calls Shutdown on all registered providers. @@ -269,10 +271,10 @@ TEST_F(ProviderRepositoryTest, ShutdownAllProviders) { EXPECT_CALL(*mock_named_1, Init(_)).WillOnce(Return(absl::OkStatus())); EXPECT_CALL(*mock_named_2, Init(_)).WillOnce(Return(absl::OkStatus())); - repo.SetProvider(mock_default, ctx, true); - repo.SetProvider(domain_1, mock_named_1, ctx, true); - repo.SetProvider(domain_2, mock_named_2, ctx, true); - repo.SetProvider(domain_3, mock_named_2, ctx, true); + repo_.SetProvider(mock_default, ctx_, true); + repo_.SetProvider(domain_1, mock_named_1, ctx_, true); + repo_.SetProvider(domain_2, mock_named_2, ctx_, true); + repo_.SetProvider(domain_3, mock_named_2, ctx_, true); EXPECT_CALL(*mock_default, Shutdown()).WillOnce(Return(absl::OkStatus())); EXPECT_CALL(*mock_named_1, Shutdown()).WillOnce(Return(absl::OkStatus())); @@ -280,14 +282,14 @@ TEST_F(ProviderRepositoryTest, ShutdownAllProviders) { // Keep a reference to the old manager. std::shared_ptr old_manager = - repo.GetFeatureProviderStatusManager(); + repo_.GetFeatureProviderStatusManager(); - repo.Shutdown(); + repo_.Shutdown(); // Assert that a new default manager has been created and it's not the old // one. std::shared_ptr new_manager = - repo.GetFeatureProviderStatusManager(); + repo_.GetFeatureProviderStatusManager(); ASSERT_NE(new_manager, nullptr); ASSERT_NE(new_manager, old_manager); @@ -319,7 +321,7 @@ TEST_F(ProviderRepositoryTest, ShutdownWaitsForAsyncInitializationToComplete) { EXPECT_CALL(*mock_provider, Shutdown()).WillOnce(Return(absl::OkStatus())); - repo.SetProvider(mock_provider, ctx, false); + repo_.SetProvider(mock_provider, ctx_, false); // Wait until the background thread is confirmed to be inside the Init() // method. @@ -327,7 +329,7 @@ TEST_F(ProviderRepositoryTest, ShutdownWaitsForAsyncInitializationToComplete) { // Run Shutdown() to verify that it blocks until init is allowed to complete. auto shutdown_future = - std::async(std::launch::async, [&]() { repo.Shutdown(); }); + std::async(std::launch::async, [&]() { repo_.Shutdown(); }); // We expect it to time out because the Init() is still blocked. auto status = shutdown_future.wait_for(std::chrono::milliseconds(100)); @@ -351,13 +353,13 @@ TEST_F(ProviderRepositoryTest, SetExistingProviderReusesManager) { EXPECT_CALL(*mock_provider, Init(_)).WillOnce(Return(absl::OkStatus())); - repo.SetProvider(mock_provider, ctx, true); + repo_.SetProvider(mock_provider, ctx_, true); std::shared_ptr default_manager = - repo.GetFeatureProviderStatusManager(); + repo_.GetFeatureProviderStatusManager(); - repo.SetProvider(domain, mock_provider, ctx, true); + repo_.SetProvider(domain, mock_provider, ctx_, true); std::shared_ptr named_manager = - repo.GetFeatureProviderStatusManager(domain); + repo_.GetFeatureProviderStatusManager(domain); EXPECT_EQ(default_manager, named_manager); } @@ -373,20 +375,20 @@ TEST_F(ProviderRepositoryTest, ReplacingProviderDoesNotShutdownIfStillInUse) { EXPECT_CALL(*mock_provider1, Init(_)).WillOnce(Return(absl::OkStatus())); EXPECT_CALL(*mock_provider2, Init(_)).WillOnce(Return(absl::OkStatus())); - repo.SetProvider(mock_provider1, ctx, true); - repo.SetProvider(domain_a, mock_provider1, ctx, true); + repo_.SetProvider(mock_provider1, ctx_, true); + repo_.SetProvider(domain_a, mock_provider1, ctx_, true); std::shared_ptr manager_for_provider1 = - repo.GetFeatureProviderStatusManager(); + repo_.GetFeatureProviderStatusManager(); ASSERT_EQ(manager_for_provider1->GetProvider(), mock_provider1); EXPECT_CALL(*mock_provider1, Shutdown()).Times(0); - repo.SetProvider(mock_provider2, ctx, true); + repo_.SetProvider(mock_provider2, ctx_, true); EXPECT_EQ(manager_for_provider1->GetStatus(), ProviderStatus::kReady); EXPECT_CALL(*mock_provider1, Shutdown()).WillOnce(Return(absl::OkStatus())); - repo.SetProvider(domain_a, mock_provider2, ctx, true); + repo_.SetProvider(domain_a, mock_provider2, ctx_, true); EXPECT_EQ(manager_for_provider1->GetStatus(), ProviderStatus::kNotReady); } From 775754f270049c42b2bd47d673610ac3532b2803 Mon Sep 17 00:00:00 2001 From: NeaguGeorgiana23 Date: Tue, 12 May 2026 10:06:04 +0000 Subject: [PATCH 5/5] Fix linter errors. Signed-off-by: NeaguGeorgiana23 --- test/provider_repository_test.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/provider_repository_test.cpp b/test/provider_repository_test.cpp index 9e799e4..dc1e883 100644 --- a/test/provider_repository_test.cpp +++ b/test/provider_repository_test.cpp @@ -22,6 +22,7 @@ using ::testing::Return; class ProviderRepositoryTest : public ::testing::Test { protected: + static constexpr std::chrono::milliseconds kWaitDuration{100}; ProviderRepository repo_; EvaluationContext ctx_ = EvaluationContext::Builder().build(); }; @@ -172,7 +173,7 @@ TEST_F(ProviderRepositoryTest, SetProviderDoesNotWaitForInitialization) { EXPECT_EQ(repo_.GetProviderStatus(), ProviderStatus::kNotReady); init_can_complete.set_value(); - std::this_thread::sleep_for(std::chrono::milliseconds(100)); + std::this_thread::sleep_for(kWaitDuration); EXPECT_EQ(repo_.GetProviderStatus(), ProviderStatus::kReady); } @@ -332,7 +333,7 @@ TEST_F(ProviderRepositoryTest, ShutdownWaitsForAsyncInitializationToComplete) { std::async(std::launch::async, [&]() { repo_.Shutdown(); }); // We expect it to time out because the Init() is still blocked. - auto status = shutdown_future.wait_for(std::chrono::milliseconds(100)); + auto status = shutdown_future.wait_for(kWaitDuration); ASSERT_EQ(status, std::future_status::timeout) << "Shutdown() did not wait for initialization to complete.";