Setup separate GA jobs for firestore #7523

wu-hui · 2023-08-03T18:23:41Z

This PR utilizes setup different github actions jobs for different flavors of Firestore integration tests, as opposed to running them from one single job. This brings benefits:

Better parallelism
Better test isolation, and less flakiness
By-pass the log clobbering issue with current utility scripts

Please see the new CI checks from the last two commits:

without firestore changes (46dfa4e)
with firestore changes (df87cac)

changeset-bot · 2023-08-03T18:23:45Z

⚠️ No Changeset found

Latest commit: 1e17b96

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

google-oss-bot · 2023-08-03T18:49:20Z

Size Report ¹

Affected Products

No changes between base commit (5dac8b3) and merge commit (4aec516).

Test Logs

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

google-oss-bot · 2023-08-03T19:05:47Z

Size Analysis Report ¹

Affected Products

No changes between base commit (5dac8b3) and merge commit (4aec516).

Test Logs

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

wu-hui

I applied some of your suggestions. Some of them seem to lead to other issues, which I do not want to chase down, as all the suggestions are maybe only saving us 30 seconds each run.

.github/workflows/test-changed-firestore.yml

wu-hui · 2023-08-15T23:05:54Z

.github/workflows/test-changed-firestore.yml

+      - name: Archive build
+        if: ${{ !cancelled() }}
+        run: |
+          tar -cf build.tar .


This leads to tar complaining the directory changed while archiving, and it only happens with CI.

.github/workflows/test-changed-firestore.yml

wu-hui · 2023-08-15T23:44:46Z

@DellaBitta

The old Test Firestore on Chrome and Node If Changed check is frozen, and the newly added jobs are not required. Is it because they are not in master yet? Or is there some other place I need to change to make it the way I wanted?

DellaBitta · 2023-08-16T14:37:11Z

.github/workflows/test-changed-firestore.yml

+
+  compat-test-chrome:
+    name: Test Firestore Compatible
+


remove empty lines please.

DellaBitta · 2023-08-16T14:37:26Z

.github/workflows/test-changed-firestore.yml

+    name: Test Firestore Compatible
+
+    runs-on: ubuntu-latest
+


remove empty line.

DellaBitta · 2023-08-16T14:44:40Z

.github/workflows/test-changed-firestore.yml

-      - name: Run tests if firestore or its dependencies has changed
-        run: yarn test:changed firestore
+        id: build
+        run: yarn build:changed firestore | egrep "Skipping all"


What happens if there's a build failure that doesn't produce the output Skipping All. Does it still generate an error in steps.build.outcome (which is tested below)?

Yes, the tests will run and they will fail for missing dependencies because build failed.

So if there's a build failure then steps.build.outcome will be success?

Is there a way that we can make this job fail instead, so that the other jobs don't even start? Seems like it would be more clear as to where the problem is when an error occurs.

Also, by piping the output from yarn build:changed firestore through egrep the entire output is lost. So if the build fails there will be no logs indicating why the build failed. Please find a way to do this while preserving the command's output in the logs.

Yeah, we could make yarn build:changed > SOME_OUTPUT a separate step, and do the grepping in the next step.

For now, can I get by with a TODO?

You'd probably want to redirect stderr as well, so that it shows up inline. Using the tee command seems like a good candidate here:

yarn build:changed 2>&1 | tee ${{ runner.temp }}/yarn.log.txt

This would print the output to stdout and also save it into a temp file. The next line in the step's bash script could run egrep.

.github/workflows/test-changed-firestore.yml

dconeybe · 2023-08-16T16:34:28Z

.github/workflows/test-changed-firestore.yml

-      - name: Run tests if firestore or its dependencies has changed
-        run: yarn test:changed firestore
+        id: build
+        run: yarn build:changed firestore | egrep "Skipping all"


Also, by piping the output from yarn build:changed firestore through egrep the entire output is lost. So if the build fails there will be no logs indicating why the build failed. Please find a way to do this while preserving the command's output in the logs.

.github/workflows/test-changed-firestore.yml

dconeybe · 2023-08-16T16:37:10Z

.github/workflows/test-changed-firestore.yml

+      - name: Archive build
+        if: ${{ !cancelled() && steps.build.outcome != 'success' }}
+        run: |
+          tar -cf build.tar --exclude="\.git" .


nit: Change --exclude="\.git" to --exclude=.git because you don't have to "escape" the . character. The pattern given to --exclude is a "glob" pattern (see man 7 glob) which does not treat . specially.

Thanks, will address in the next PR.

dconeybe · 2023-08-16T16:40:06Z

.github/workflows/test-changed-firestore.yml

+      - name: Bump Node memory limit
+        run: echo "NODE_OPTIONS=--max_old_space_size=4096" >> $GITHUB_ENV
+      - name: Test setup and yarn install
+        run: |


nit: reduce this single-line bash script to one line (i.e. get rid of the pipe | and bring the line below up to this line). Also on lines 113 and 148.

Thanks, will address in the next PR.

dconeybe

Giving approval to facilitate testing of the changes.

dconeybe · 2023-08-16T17:33:18Z

.github/workflows/test-changed-firestore.yml

+          path: build.tar.gz
+          retention-days: ${{ env.artifactRetentionDays }}
+
+  compat-test-chrome:


Should we also add a compat-test-firefox? It looks like the old code ran the compat tests on firefox as well.

Thanks, will address in the next PR.

Just a reminder to make sure to add compat-test-firefox to this repo's "required" checks when you add it.

wu-hui

Thanks, I am going to merge this PR and address the comments in a followup one, such that people can run these checks now.

.github/workflows/test-changed-firestore.yml

wu-hui · 2023-08-16T18:19:27Z

.github/workflows/test-changed-firestore.yml

+      - name: Archive build
+        if: ${{ !cancelled() && steps.build.outcome != 'success' }}
+        run: |
+          tar -cf build.tar --exclude="\.git" .


Thanks, will address in the next PR.

wu-hui · 2023-08-16T18:19:31Z

.github/workflows/test-changed-firestore.yml

+          path: build.tar.gz
+          retention-days: ${{ env.artifactRetentionDays }}
+
+  compat-test-chrome:


Thanks, will address in the next PR.

wu-hui · 2023-08-16T18:19:37Z

.github/workflows/test-changed-firestore.yml

+      - name: Bump Node memory limit
+        run: echo "NODE_OPTIONS=--max_old_space_size=4096" >> $GITHUB_ENV
+      - name: Test setup and yarn install
+        run: |


Thanks, will address in the next PR.

wu-hui added 15 commits July 31, 2023 11:55

Setup firestore tests for named db

1774754

fix error

1b92147

Fail CI intentionally

2ff3e0f

Fix

05816aa

Run named db lite only

00c2ef5

browser:prod

29fce8f

try more

48ce3de

more more

5e9e606

try sequential

37b690e

both parallelization

32e9780

Better parallelization

558efd7

more tuning

8dd9483

separate

27d56c4

sequential

11dbbfc

all sequential

1aeb561

wu-hui requested review from a team, dwyfrequency and hsubox76 as code owners August 3, 2023 18:23

wu-hui force-pushed the wuandy/ParallelByGH branch 3 times, most recently from dc65363 to a595f19 Compare August 3, 2023 18:39

Setup separate jobs

0809b3a

wu-hui force-pushed the wuandy/ParallelByGH branch from a595f19 to 0809b3a Compare August 3, 2023 18:42

goes further

500b745

wu-hui added 2 commits August 4, 2023 09:45

Skip script

36112e6

Fix job failure

1d19498

wu-hui added 2 commits August 14, 2023 16:03

no yarn

50379f6

no node_modules

774a6fa

wu-hui force-pushed the wuandy/ParallelByGH branch 7 times, most recently from 7fa128d to 3aa8e95 Compare August 15, 2023 19:44

detect changed

f6442ef

wu-hui force-pushed the wuandy/ParallelByGH branch from 3aa8e95 to f6442ef Compare August 15, 2023 20:05

wu-hui added 4 commits August 15, 2023 16:25

No firestore change

6d86c84

fix

8e7f95c

firestore change

df87cac

no firestore

46dfa4e

wu-hui commented Aug 15, 2023

View reviewed changes

DellaBitta reviewed Aug 16, 2023

View reviewed changes

wu-hui added 2 commits August 16, 2023 11:22

empty lines

d8e476b

Add todo

1e17b96

dconeybe requested changes Aug 16, 2023

View reviewed changes

dconeybe approved these changes Aug 16, 2023

View reviewed changes

dconeybe reviewed Aug 16, 2023

View reviewed changes

DellaBitta approved these changes Aug 16, 2023

View reviewed changes

wu-hui commented Aug 16, 2023

View reviewed changes

wu-hui merged commit 4ae8041 into master Aug 16, 2023

wu-hui deleted the wuandy/ParallelByGH branch August 16, 2023 18:21

This was referenced Aug 17, 2023

run_tests_in_ci.js: fix truncated log output #7553

Closed

Firestore: Run 'Firestore Integration Tests' jobs in parallel #7558

Merged

firebase locked and limited conversation to collaborators Sep 16, 2023

Setup separate GA jobs for firestore #7523

Setup separate GA jobs for firestore #7523

Uh oh!

Conversation

wu-hui commented Aug 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot bot commented Aug 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

google-oss-bot commented Aug 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Size Report 1

Affected Products

Test Logs

Uh oh!

google-oss-bot commented Aug 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Size Analysis Report 1

Affected Products

Test Logs

Uh oh!

wu-hui left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wu-hui commented Aug 15, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dconeybe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wu-hui left a comment

Choose a reason for hiding this comment

Uh oh!

wu-hui commented Aug 3, 2023 •

edited

Loading

changeset-bot bot commented Aug 3, 2023 •

edited

Loading

google-oss-bot commented Aug 3, 2023 •

edited

Loading

Size Report ¹

google-oss-bot commented Aug 3, 2023 •

edited

Loading

Size Analysis Report ¹