Skip to content

Quick check uses dummy google-services file, and skip certain tests #705

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.

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

Merged
merged 19 commits into from
Oct 25, 2021

Conversation

sunmou99
Copy link
Contributor

@sunmou99 sunmou99 commented Oct 18, 2021

Description

External contributors failed the quick check and cannot merge their PR, because they don't have access to the SECRET.

Now, remove the SECRET logic in all pre-submit check.
Add dummy google-services file as a placeholder to make sure the testapp still build.
Also skip tests that requires SECRET.


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.

remove restore_secret.py
@google-cla google-cla bot added the cla: yes label Oct 18, 2021
@sunmou99 sunmou99 changed the title Update desktop.yml Quick check usesdummy google-services file, and skip certain tests Oct 19, 2021
@sunmou99 sunmou99 marked this pull request as ready for review October 19, 2021 23:58
@sunmou99 sunmou99 changed the title Quick check usesdummy google-services file, and skip certain tests Quick check uses dummy google-services file, and skip certain tests Oct 20, 2021
@sunmou99 sunmou99 added the skip-release-notes Skip release notes check label Oct 21, 2021
@@ -603,6 +610,7 @@ TEST_F(FirebaseFirestoreBasicTest, TestRunTransaction) {
// TODO: Add test for failing transaction.

Choose a reason for hiding this comment

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

⚠️ Lint warning: Missing username in TODO; it should look like "// TODO(my_username): Stuff."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will firestore team resolve the TODO?

Copy link
Contributor

Choose a reason for hiding this comment

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

That is a pre-existing TODO, unrelated to this PR. And, yes, it would be Firestore's to fix.

@@ -665,6 +673,7 @@ TEST_F(FirebaseFirestoreBasicTest,
}

TEST_F(FirebaseFirestoreBasicTest, TestInvalidatingReferencesWhenDeletingApp) {
SKIP_TEST_ON_QUICK_CHECK;
delete shared_app_;
shared_app_ = nullptr;
// TODO: Ensure existing Firestore objects are invalidated.

Choose a reason for hiding this comment

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

⚠️ Lint warning: Missing username in TODO; it should look like "// TODO(my_username): Stuff."

@sunmou99 sunmou99 merged commit 3990d4c into main Oct 25, 2021
@github-actions github-actions bot added the tests: in-progress This PR's integration tests are in progress. label Oct 25, 2021
@github-actions
Copy link

github-actions bot commented Oct 25, 2021

❌  Integration test FAILED

Requested by @sunmou99 on commit 3990d4c
Last updated: Mon Oct 25 16:16 PDT 2021
View integration test log & download artifacts

Failures Configs
firestore [TEST] [ERROR] [Android] [windows] [android_target]

Add flaky tests to go/fpl-cpp-flake-tracker

@github-actions github-actions bot added the tests: failed This PR's integration tests failed. label Oct 25, 2021
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Oct 25, 2021
@firebase firebase locked and limited conversation to collaborators Nov 25, 2021
@sunmou99 sunmou99 deleted the bugfix/desktop_restore_sercet branch December 7, 2021 00:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes skip-release-notes Skip release notes check tests: failed This PR's integration tests failed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants