Skip to content

Fix issues with Auth integration tests#1851

Open
a-maurice wants to merge 3 commits intomainfrom
am-auth_tests
Open

Fix issues with Auth integration tests#1851
a-maurice wants to merge 3 commits intomainfrom
am-auth_tests

Conversation

@a-maurice
Copy link
Copy Markdown
Contributor

Description

Provide details of the change, and generalize the change in the PR title above.

There are two noticeable issues in the nightly tests:
SignInWithBadEmail would regularly fail because the backend thought it was suspicious, which resulted in the wrong error. Just deleting that test, since that behavior doesn't really need to be tested anyway.
LinkAnonymousUserWithEmailCredential was not properly cleaning up the users it created, resulting in leftover users on the backend that needed to be cleaned up. Refactored the test to address it.

Also adjusted the generated emails, so that they can be more easily tracked back if they are not cleaned up properly.


Testing

Describe how you've tested these changes. Link any manually triggered Integration tests or CPP binary SDK Packaging Github Action workflows, if applicable.

https://github.com/firebase/firebase-cpp-sdk/actions/runs/25352154658


Type of Change

Place an x the applicable box:

  • Bug fix. Add the issue # below if applicable.
  • New feature. A non-breaking change which adds functionality.
  • Other, such as a build process or documentation change.

Notes

  • Bug fixes and feature changes require an update to the Release Notes section of release_build_files/readme.md.
  • Read the contribution guidelines CONTRIBUTING.md.
  • Changes to the public API require an internal API review. If you'd like to help us make Firebase APIs better, please propose your change in a feature request so that we can discuss it together.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request enhances the Firebase Auth integration tests by updating the GenerateEmailAddress utility to incorporate platform-specific labels and test names, which improves the traceability of generated test accounts. It also refactors several test cases for better robustness, including more explicit error checking for provider linking and additional debug logging. The review feedback suggests using std::to_string for more idiomatic and portable integer-to-string conversion and simplifying the email string construction logic.

Comment on lines 448 to 450
char time_string[22];
snprintf(time_string, sizeof(time_string), "%lld",
app_framework::GetCurrentTimeInMicroseconds());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Using snprintf with %lld for an int64_t value can lead to portability issues or compiler warnings if int64_t is not exactly long long on all target platforms. Since this is C++, using std::to_string is a safer and more idiomatic way to convert the timestamp to a string.

Suggested change
char time_string[22];
snprintf(time_string, sizeof(time_string), "%lld",
app_framework::GetCurrentTimeInMicroseconds());
std::string time_string = std::to_string(app_framework::GetCurrentTimeInMicroseconds());

Comment on lines +463 to 469
std::string email = "user_";
email.append(INTEGRATION_TEST_PLATFORM_NAME);
email.append("_");
email.append(sanitized_test_name);
email.append("_");
email.append(time_string);
email.append("@gmail.com");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The email construction logic can be simplified by using string concatenation or a single append call, which improves readability. Additionally, since time_string is now a std::string (per the previous suggestion), the append calls are straightforward.

Suggested change
std::string email = "user_";
email.append(INTEGRATION_TEST_PLATFORM_NAME);
email.append("_");
email.append(sanitized_test_name);
email.append("_");
email.append(time_string);
email.append("@gmail.com");
std::string email = "user_" + std::string(INTEGRATION_TEST_PLATFORM_NAME) + "_" +
sanitized_test_name + "_" + time_string + "@gmail.com";

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant