Skip to content

Configure CI presubmits with Github Actions. #3856

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

Jump to bottom
Merged
merged 7 commits into from
Jun 30, 2022
Merged

Configure CI presubmits with Github Actions. #3856

merged 7 commits into from
Jun 30, 2022

Conversation

vkryachko
Copy link
Member

No description provided.

@vkryachko vkryachko closed this Jun 29, 2022
@vkryachko vkryachko reopened this Jun 29, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jun 29, 2022 •
edited
Loading

Unit Test Results

   392 files     392 suites   16m 45s ⏱️
4 699 tests 4 676 ✔️ 22 💤 1 ❌
4 715 runs  4 692 ✔️ 22 💤 1 ❌

For more details on these failures, see this check.

Results for commit a8763a4.

♻️ This comment has been updated with latest results.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jun 29, 2022 •
edited
Loading

Coverage Report 1

Affected Products

  • firebase-database

    Overall coverage changed from 50.20% (63f112f) to 50.23% (029de58) by +0.02%.

    FilenameBase (63f112f)Merge (029de58)Diff
    ChildChangeAccumulator.java96.67%83.33%-13.33%
    DoubleNode.java88.24%100.00%+11.76%
    WebsocketConnection.java32.77%35.03%+2.26%

  • firebase-firestore

    Overall coverage changed from 45.96% (63f112f) to 45.97% (029de58) by +0.01%.

    FilenameBase (63f112f)Merge (029de58)Diff
    GrpcCallProvider.java55.29%58.82%+3.53%

  • firebase-messaging

    FilenameBase (63f112f)Merge (029de58)Diff
    FirebaseMessaging.java75.33%75.77%+0.44%
    MessagingAnalytics.java82.19%81.78%-0.40%

Test Logs

Notes

  • Commit (029de58) is created by Prow via merging PR base commit (63f112f) and head commit (a8763a4).
  • Run gradle <product>:checkCoverage to produce HTML coverage reports locally. After gradle commands finished, report files can be found under <product-build-dir>/reports/jacoco/.

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/6wTn6JMgJh.html

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jun 29, 2022 •
edited
Loading

Size Report 1

Affected Products

No changes between base commit (63f112f) and merge commit (029de58).

Test Logs

Notes

  • Commit (029de58) is created by Prow via merging PR base commit (63f112f) and head commit (a8763a4).

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/SUUJO9RL91.html

@vkryachko vkryachko requested review from rlazo and yifanyang June 29, 2022 19:10
@vkryachko
Copy link
Member Author

While some tests are failing, they are not related to any missing configuration in CI, I will ping individual teams for help on those tests.

yifanyang
yifanyang approved these changes Jun 30, 2022
View reviewed changes
Copy link
Contributor

@yifanyang yifanyang left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Nits

  • Complaints of no new line at EOF in several files
  • Mix usage of single and double quotes in a few files

FYIs

  • Other tests (smoke, metrics) will also need steps for setting up jdk and ndk. I can create a composite action to simplify them a bit.
  • Other platforms does similar things like detecting changed SDKs before testing. I'll try to see if it can be abstracted as a reusable action.

.github/workflows/ci_tests_experimental.yml Outdated
- '*'
push:
branches:
- 'master'
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it intended to support only the master branch?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, as we don't want to run post submits on all branches

.github/workflows/ci_tests_experimental.yml
integ_tests:
name: "Instrumentation Tests"
# only run on post submit or PRs not originating from forks.
if: (github.repository == 'Firebase/firebase-android-sdk' && github.event_name == 'push') || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need github.repository == 'Firebase/firebase-android-sdk'?

Copy link
Member Author

Choose a reason for hiding this comment

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

This check ensures that secrets are available, which is not the case when PRs originate from forks.
So effectively integ tests will only run on "trusted" PRs as well as on post submit on master.

Copy link
Contributor

Choose a reason for hiding this comment

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

My question was not clear enough. Is there a case where for push events that may trigger this workflow, github.repository is not Firebase/firebase-android-sdk? I thought only for pull request event do we need to detect the repo.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 30, 2022 •
edited
Loading

buildSrc Test Results

18 tests   18 ✔️  2m 1s ⏱️
  4 suites    0 💤
  4 files      0 ❌

Results for commit a8763a4.

♻️ This comment has been updated with latest results.

@vkryachko vkryachko merged commit 7d64cde into master Jun 30, 2022
@vkryachko vkryachko deleted the vk.gha_test branch June 30, 2022 17:31
@google-oss-bot
Copy link
Contributor

@vkryachko: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
fireci a8763a4 link /test fireci
smoke-tests a8763a4 link /test smoke-tests
device-check-changed a8763a4 link /test device-check-changed

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@firebase firebase locked and limited conversation to collaborators Jul 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@yifanyang yifanyang yifanyang approved these changes

@rlazo rlazo rlazo approved these changes

Assignees
No one assigned
Labels
size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vkryachko @google-oss-bot @rlazo @yifanyang