Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrap the realtime RC test in flaky-block to automatically retry. #1406

Merged
merged 5 commits into from
Jul 31, 2023
Merged

Wrap the realtime RC test in flaky-block to automatically retry. #1406

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3d179fe
Wrap the realtime RC test in flaky block to see if retry after a dela…
AlmostMatt Jul 27, 2023
305f149
whitespace
AlmostMatt Jul 27, 2023
fe3a90a
Only flaky_test on android
AlmostMatt Jul 28, 2023
64a826b
fix typo
AlmostMatt Jul 28, 2023
7686e65
Merge branch 'main' into amatt-rc-retry
AlmostMatt Jul 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 16 additions & 8 deletions remote_config/integration_test/src/integration_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,10 @@ TEST_F(FirebaseRemoteConfigTest, TestSetDefault) {
TEST_F(FirebaseRemoteConfigTest, TestAddOnConfigUpdateListener) {
ASSERT_NE(rc_, nullptr);

// This test sometimes times out on Android with the config not updated.
#if defined(__ANDROID__)
FLAKY_TEST_SECTION_BEGIN();
AlmostMatt marked this conversation as resolved.
Show resolved Hide resolved
#endif // defined(__ANDROID__)
// Check if the config has default values. If not, we have cached data
// from a previous test run, and auto-fetch will not happen.
EXPECT_TRUE(WaitForCompletion(SetDefaults(rc_), "SetDefaults"));
Expand Down Expand Up @@ -302,6 +306,7 @@ TEST_F(FirebaseRemoteConfigTest, TestAddOnConfigUpdateListener) {
firebase::remote_config::RemoteConfigError) {});
#else
auto config_update_promise = std::make_shared<std::promise<void> >();
auto config_update_future = config_update_promise->get_future();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is all this promise stuff just used for the delay?

Copy link
Contributor Author

@AlmostMatt AlmostMatt Jul 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea the promise+future is used just to verify that the callback is called.
The promise is used to set a value, and the corresponding future is used to read the value once it has been set. This change to the order of the lines is not functional, just defining the two variables together.


firebase::remote_config::ConfigUpdateListenerRegistration registration =
rc_->AddOnConfigUpdateListener(
Expand All @@ -312,9 +317,8 @@ TEST_F(FirebaseRemoteConfigTest, TestAddOnConfigUpdateListener) {
config_update_promise->set_value();
});
if (!has_cached_data) {
auto config_update_future = config_update_promise->get_future();
ASSERT_EQ(std::future_status::ready,
config_update_future.wait_for(std::chrono::milliseconds(30000)));
config_update_future.wait_for(std::chrono::milliseconds(20000)));

// On Android WaitForCompletion must be called from the main thread,
// so Activate is called here outside of the listener.
Expand All @@ -324,15 +328,19 @@ TEST_F(FirebaseRemoteConfigTest, TestAddOnConfigUpdateListener) {
std::map<std::string, firebase::Variant> key_values = rc_->GetAll();
EXPECT_EQ(key_values.size(), 6);

for (auto key_valur_pair : kServerValue) {
firebase::Variant k_value = key_valur_pair.value;
firebase::Variant fetched_value = key_values[key_valur_pair.key];
for (auto key_value_pair : kServerValue) {
firebase::Variant k_value = key_value_pair.value;
firebase::Variant fetched_value = key_values[key_value_pair.key];
EXPECT_EQ(k_value.type(), fetched_value.type());
EXPECT_EQ(k_value, fetched_value);
}
registration.Remove();
AlmostMatt marked this conversation as resolved.
Show resolved Hide resolved
}
#endif // !FIREBASE_PLATFORM_DESKTOP
// This test sometimes times out on Android with the config not updated.
#if defined(__ANDROID__)
FLAKY_TEST_SECTION_END();
#endif // defined(__ANDROID__)
}

TEST_F(FirebaseRemoteConfigTest, TestRemoveConfigUpdateListener) {
Expand Down Expand Up @@ -377,9 +385,9 @@ TEST_F(FirebaseRemoteConfigTest, TestGetAll) {
std::map<std::string, firebase::Variant> key_values = rc_->GetAll();
EXPECT_EQ(key_values.size(), 6);

for (auto key_valur_pair : kServerValue) {
firebase::Variant k_value = key_valur_pair.value;
firebase::Variant fetched_value = key_values[key_valur_pair.key];
for (auto key_value_pair : kServerValue) {
firebase::Variant k_value = key_value_pair.value;
firebase::Variant fetched_value = key_values[key_value_pair.key];
EXPECT_EQ(k_value.type(), fetched_value.type());
EXPECT_EQ(k_value, fetched_value);
}
Expand Down