Skip to content

Conversation

@dconeybe
Copy link
Contributor

@dconeybe dconeybe commented Mar 30, 2021

When Firestore's integration tests were moved to the integration_test_internal folder, there were a few loose ends that needed to be tied. This PR ties them up.

Changes in this PR include:

…hat the cached Java instance will be cleared.

This changelist was previously ported in #329 but the integration testing component was lost in a merge.
…lled from Run()

This changelist was previously ported in #330 but the integration testing component was lost in a merge.
This changelist was previously ported in #331 but the integration testing component was lost in a merge.

As a benefit, this also fixes the broken promise_android_test.cc tests.
@dconeybe dconeybe self-assigned this Mar 30, 2021
@google-cla google-cla bot added the cla: yes label Mar 30, 2021
@dconeybe dconeybe requested a review from jonsimantov March 31, 2021 06:35
@dconeybe dconeybe assigned jonsimantov and unassigned dconeybe Mar 31, 2021
@dconeybe dconeybe changed the title Clean up the tiny mess left behind when the Firestore internal integration tests were set up Tie up the loose ends from moving Firestore'e integration tests into integration_test_internal Mar 31, 2021
@jonsimantov jonsimantov added the tests-requested: quick Trigger a quick set of integration tests. label Mar 31, 2021
@github-actions github-actions bot added tests: in-progress This PR's integration tests are in progress. tests: failed This PR's integration tests failed. and removed tests-requested: quick Trigger a quick set of integration tests. labels Mar 31, 2021
@dconeybe
Copy link
Contributor Author

It looks like the integration tests are failing to build due to firebase_firestore_settings_android_test.cc, which was added in this PR. I'll push up a fix.

  /home/runner/work/firebase-cpp-sdk/firebase-cpp-sdk/ta/firestore/iti/src/android/firebase_firestore_settings_android_test.cc:1:10: fatal error: 'firestore/src/android/firebase_firestore_settings_android.h' file not found
  #include "firestore/src/android/firebase_firestore_settings_android.h"
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  1 error generated.

@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Mar 31, 2021
@dconeybe
Copy link
Contributor Author

Ahh, firebase_firestore_settings_android_test.cc is obsolete. It was deleted in cl/329734038 in Sept 2020 but I guess that deletion didn't get propagated into this GitHub repository. I'll delete it from this PR.

It was deleted in cl/329734038 in Sept 2020 but that deletion appears to not have gotten propagated to this GitHub repository).
@dconeybe dconeybe added tests-requested: quick Trigger a quick set of integration tests. and removed tests: failed This PR's integration tests failed. labels Mar 31, 2021
@github-actions github-actions bot added tests: in-progress This PR's integration tests are in progress. tests: failed This PR's integration tests failed. and removed tests-requested: quick Trigger a quick set of integration tests. labels Mar 31, 2021
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Mar 31, 2021
Copy link
Contributor

@jonsimantov jonsimantov left a comment

Choose a reason for hiding this comment

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

Looks like two tests are still failing on Android:
[ FAILED ] TypeTest.TestCanReadAndWriteNullFields
[ FAILED ] TypeTest.TestCanReadAndWriteArrayFields

Could you (ideally) fix them or (alternately) skip them with the TODO?

…dcoded document name to prevent tests from interfering with each other
@dconeybe
Copy link
Contributor Author

dconeybe commented Apr 1, 2021

@jonsimantov: Those two failing tests (TestCanReadAndWriteNullFields and TestCanReadAndWriteArrayFields) appear to be transient failures. Looking at the code, each test case was using a hardcoded document, leaving room for tests to interfere with each other or even other unrelated parallel test runs interfering with each other. I suspect that is what happened here. To fix, I've change the test code in firestore/integration_test_internal/src/type_test.cc to use a randomly-generated document name, to prevent interference.

@dconeybe dconeybe added the tests-requested: quick Trigger a quick set of integration tests. label Apr 1, 2021
@github-actions github-actions bot added tests: in-progress This PR's integration tests are in progress. and removed tests-requested: quick Trigger a quick set of integration tests. tests: failed This PR's integration tests failed. labels Apr 1, 2021
@dconeybe dconeybe added tests: in-progress This PR's integration tests are in progress. and removed tests: in-progress This PR's integration tests are in progress. labels Apr 1, 2021
@dconeybe dconeybe self-assigned this Apr 1, 2021
@github-actions github-actions bot added the tests: failed This PR's integration tests failed. label Apr 1, 2021
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Apr 1, 2021
@dconeybe dconeybe changed the title Tie up the loose ends from moving Firestore'e integration tests into integration_test_internal Tie up the loose ends from moving Firestore's integration tests into integration_test_internal Apr 1, 2021
@dconeybe dconeybe added the tests-requested: quick Trigger a quick set of integration tests. label Apr 1, 2021
@github-actions github-actions bot added tests: in-progress This PR's integration tests are in progress. tests: failed This PR's integration tests failed. and removed tests-requested: quick Trigger a quick set of integration tests. tests: failed This PR's integration tests failed. labels Apr 1, 2021
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Apr 1, 2021
@dconeybe dconeybe added tests-requested: quick Trigger a quick set of integration tests. and removed tests: failed This PR's integration tests failed. labels Apr 7, 2021
@github-actions github-actions bot added tests: in-progress This PR's integration tests are in progress. and removed tests-requested: quick Trigger a quick set of integration tests. labels Apr 7, 2021
@github-actions
Copy link

github-actions bot commented Apr 7, 2021

✅  Integration test succeeded!

Requested by @dconeybe on commit 0684efc
Last updated: Wed Apr 7 10:18:41 PDT 2021
View integration test results

@dconeybe dconeybe requested a review from jonsimantov April 7, 2021 17:04
@github-actions github-actions bot added the tests: succeeded This PR's integration tests succeeded. label Apr 7, 2021
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Apr 7, 2021
@dconeybe dconeybe enabled auto-merge (squash) April 7, 2021 17:22
@dconeybe dconeybe merged commit fae8b6e into main Apr 8, 2021
@jonsimantov jonsimantov deleted the dconeybe/FirestoreOldAndroidCleanUp branch April 8, 2021 19:36
@firebase firebase locked and limited conversation to collaborators May 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: firestore cla: yes tests: succeeded This PR's integration tests succeeded.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants