Skip to content

Commit 5989e57

Browse files
committed
Merge branch 'main' at 94df73a into FixFirestoreCanPageThroughItemsTestCrash to pick up #430, which will fix some of the integration test failures by setting the minSdkVersion of the messaging test to 16 (was 26).
2 parents 391acc5 + 94df73a commit 5989e57

File tree

5 files changed

+94
-22
lines changed

5 files changed

+94
-22
lines changed

.github/workflows/cpp-packaging.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -677,7 +677,7 @@ jobs:
677677
with:
678678
python-version: '3.7'
679679
- name: Use expanded matrix
680-
if: github.event_name == 'schedule' || github.event.inputs.use_expanded_matrix == '1'
680+
if: github.event.inputs.use_expanded_matrix == '1'
681681
run: |
682682
echo "USE_EXPANDED_MATRIX=1" >> $GITHUB_ENV
683683
- name: Generate token for GitHub API

.github/workflows/integration_tests.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ jobs:
191191
fetch-depth: 0
192192
submodules: false
193193
- name: Use expanded matrix
194-
if: github.event_name == 'schedule' || github.event.inputs.use_expanded_matrix == '1' || needs.check_trigger.outputs.requested_tests == 'full'
194+
if: github.event.inputs.use_expanded_matrix == '1' || needs.check_trigger.outputs.requested_tests == 'full'
195195
run: |
196196
echo "EXPANDED_MATRIX_PARAM=-e=1" >> $GITHUB_ENV
197197
- name: Set auto-diff option if specified.

firestore/integration_test_internal/src/android/snapshot_metadata_android_test.cc

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include "android/firestore_integration_test_android.h"
44
#include "firebase_test_framework.h"
55
#include "firestore/src/include/firebase/firestore/snapshot_metadata.h"
6+
#include "firestore/src/jni/declaration.h"
67
#include "firestore/src/jni/env.h"
78
#include "firestore_integration_test.h"
89
#include "gmock/gmock.h"
@@ -14,30 +15,33 @@ namespace firestore {
1415
using SnapshotMetadataAndroidTest = FirestoreAndroidIntegrationTest;
1516

1617
using jni::Class;
18+
using jni::Constructor;
1719
using jni::Env;
1820
using jni::Local;
1921

2022
TEST_F(SnapshotMetadataAndroidTest, Converts) {
21-
SKIP_TEST_ON_ANDROID; // TODO(b/183294303): Fix this test on Android.
22-
2323
Env env;
24-
25-
Local<Class> clazz =
26-
env.FindClass("com/google/firebase/firestore/SnapshotMetadata");
27-
jmethodID ctor = env.GetMethodId(clazz, "<init>", "(ZZ)V");
28-
29-
auto java_metadata = env.New<SnapshotMetadataInternal>(
30-
clazz, ctor, /*has_pending_writes=*/true, /*is_from_cache=*/false);
31-
SnapshotMetadata result = java_metadata.ToPublic(env);
32-
EXPECT_TRUE(result.has_pending_writes());
33-
EXPECT_FALSE(result.is_from_cache());
34-
35-
java_metadata = env.New<SnapshotMetadataInternal>(
36-
clazz, ctor, /*has_pending_writes=*/false,
37-
/*is_from_cache=*/true);
38-
result = java_metadata.ToPublic(env);
39-
EXPECT_FALSE(result.has_pending_writes());
40-
EXPECT_TRUE(result.is_from_cache());
24+
Constructor<SnapshotMetadataInternal> ctor("(ZZ)V");
25+
loader().LoadClass("com/google/firebase/firestore/SnapshotMetadata", ctor);
26+
ASSERT_TRUE(loader().ok());
27+
28+
{
29+
auto java_metadata =
30+
env.New(ctor, /*has_pending_writes=*/true, /*is_from_cache=*/false);
31+
SnapshotMetadata result = java_metadata.ToPublic(env);
32+
EXPECT_TRUE(env.ok());
33+
EXPECT_TRUE(result.has_pending_writes());
34+
EXPECT_FALSE(result.is_from_cache());
35+
}
36+
37+
{
38+
auto java_metadata =
39+
env.New(ctor, /*has_pending_writes=*/false, /*is_from_cache=*/true);
40+
SnapshotMetadata result = java_metadata.ToPublic(env);
41+
EXPECT_TRUE(env.ok());
42+
EXPECT_FALSE(result.has_pending_writes());
43+
EXPECT_TRUE(result.is_from_cache());
44+
}
4145
}
4246

4347
} // namespace firestore

messaging/integration_test/build.gradle

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ android {
5050

5151
defaultConfig {
5252
applicationId 'com.google.firebase.cpp.messaging.testapp'
53-
minSdkVersion 26
53+
minSdkVersion 16
5454
targetSdkVersion 28
5555
versionCode 1
5656
versionName '1.0'

testing/test_framework/src/firebase_test_framework.cc

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515
#include "firebase_test_framework.h" // NOLINT
1616

1717
#include <cstdio>
18+
#include <cstring>
19+
#include <string>
20+
#include <vector>
1821

1922
#include "firebase/future.h"
2023

@@ -294,7 +297,72 @@ std::ostream& operator<<(std::ostream& os, const Variant& v) {
294297
}
295298
} // namespace firebase
296299

300+
namespace {
301+
302+
std::vector<std::string> ArgcArgvToVector(int argc, char* argv[]) {
303+
std::vector<std::string> args_vector;
304+
for (int i = 0; i < argc; ++i) {
305+
args_vector.push_back(argv[i]);
306+
}
307+
return args_vector;
308+
}
309+
310+
char** VectorToArgcArgv(const std::vector<std::string>& args_vector,
311+
int* argc) {
312+
char** argv = new char*[args_vector.size()];
313+
for (int i = 0; i < args_vector.size(); ++i) {
314+
const char* arg = args_vector[i].c_str();
315+
char* arg_copy = new char[std::strlen(arg) + 1];
316+
std::strcpy(arg_copy, arg);
317+
argv[i] = arg_copy;
318+
}
319+
*argc = static_cast<int>(args_vector.size());
320+
return argv;
321+
}
322+
323+
/**
324+
* Makes changes to argc and argv before passing them to `InitGoogleTest`.
325+
*
326+
* This function is a convenience function for developers to edit during
327+
* development/debugging to customize the the arguments specified to googletest
328+
* when directly specifying command-line arguments is not available, such as on
329+
* Android and iOS. For example, to debug a specific test, add the
330+
* --gtest_filter argument, and to list all tests add the --gtest_list_tests
331+
* argument.
332+
*
333+
* @param argc A pointer to the `argc` that will be specified to
334+
* `InitGoogleTest`; the integer to which this pointer points will be updated
335+
* with the new length of `argv`.
336+
* @param argv The `argv` that contains the arguments that would have otherwise
337+
* been specified to `InitGoogleTest()`; they will not be modified.
338+
*
339+
* @return The new `argv` to be specified to `InitGoogleTest()`.
340+
*/
341+
char** EditMainArgsForGoogleTest(int* argc, char* argv[]) {
342+
// Put the args into a vector of strings because modifying string objects in
343+
// a vector is far easier than modifying a char** array.
344+
const std::vector<std::string> original_args = ArgcArgvToVector(*argc, argv);
345+
std::vector<std::string> modified_args(original_args);
346+
347+
// Add elements to the `modified_args` vector to specify to googletest.
348+
// e.g. modified_args.push_back("--gtest_list_tests");
349+
// e.g. modified_args.push_back("--gtest_filter=MyTestFixture.MyTest");
350+
351+
// Avoid the memory leaks documented below if there were no arg changes.
352+
if (modified_args == original_args) {
353+
return argv;
354+
}
355+
356+
// Create a new `argv` with the elements from the `modified_args` vector and
357+
// write the new count back to `argc`. The memory leaks produced by
358+
// `VectorToArgcArgv` acceptable because they last for the entire application.
359+
return VectorToArgcArgv(modified_args, argc);
360+
}
361+
362+
} // namespace
363+
297364
extern "C" int common_main(int argc, char* argv[]) {
365+
argv = EditMainArgsForGoogleTest(&argc, argv);
298366
::testing::InitGoogleTest(&argc, argv);
299367
firebase_test_framework::FirebaseTest::SetArgs(argc, argv);
300368
app_framework::SetLogLevel(app_framework::kDebug);

0 commit comments

Comments
 (0)