-
Notifications
You must be signed in to change notification settings - Fork 940
Setup separate GA jobs for firestore #7523
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
Changes from 45 commits
1774754
1b92147
2ff3e0f
05816aa
00c2ef5
29fce8f
48ce3de
5e9e606
37b690e
32e9780
558efd7
8dd9483
27d56c4
11dbbfc
1aeb561
0809b3a
500b745
36112e6
1d19498
2c67e63
68c8611
63e25ed
ccf3bc6
0696e93
a73cef1
12f63d1
03d71ec
55c8ff1
692850f
2f3e297
5891d7c
f1b7044
af32139
94bbb00
068179d
74e62e3
7d9f850
76ffe85
50379f6
774a6fa
f6442ef
6d86c84
8e7f95c
df87cac
46dfa4e
d8e476b
1e17b96
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,10 +2,16 @@ name: Test Firestore | |
|
||
on: pull_request | ||
|
||
env: | ||
artifactRetentionDays: 14 | ||
|
||
jobs: | ||
test-chrome: | ||
name: Test Firestore on Chrome and Node If Changed | ||
build: | ||
name: Build Firestore | ||
|
||
runs-on: ubuntu-latest | ||
outputs: | ||
changed: ${{ steps.set-output.outputs.CHANGED }} | ||
|
||
steps: | ||
- name: Checkout Repo | ||
|
@@ -28,27 +34,115 @@ jobs: | |
cp config/ci.config.json config/project.json | ||
yarn | ||
- name: build | ||
wu-hui marked this conversation as resolved.
Show resolved
Hide resolved
|
||
run: yarn build:changed firestore | ||
- 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" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens if there's a build failure that doesn't produce the output Skipping All. Does it still generate an error in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, the tests will run and they will fail for missing dependencies because build failed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So if there's a build failure then 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, by piping the output from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, we could make For now, can I get by with a TODO? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You'd probably want to redirect stderr as well, so that it shows up inline. Using the
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 |
||
# Continue when "Skipping all" is not found | ||
continue-on-error: true | ||
- name: set output | ||
# This means "Skipping all" was not found | ||
if: steps.build.outcome != 'success' | ||
id: set-output | ||
run: echo "CHANGED=true" >> "$GITHUB_OUTPUT"; | ||
- name: Archive build | ||
if: ${{ !cancelled() && steps.build.outcome != 'success' }} | ||
run: | | ||
tar -cf build.tar --exclude="\.git" . | ||
wu-hui marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Change There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, will address in the next PR. |
||
gzip build.tar | ||
- name: Upload build archive | ||
if: ${{ !cancelled() && steps.build.outcome != 'success' }} | ||
uses: actions/upload-artifact@v3 | ||
with: | ||
name: build.tar.gz | ||
path: build.tar.gz | ||
retention-days: ${{ env.artifactRetentionDays }} | ||
|
||
compat-test-chrome: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we also add a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, will address in the next PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a reminder to make sure to add |
||
name: Test Firestore Compatible | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove empty lines please. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
runs-on: ubuntu-latest | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove empty line. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
needs: build | ||
if: ${{ needs.build.outputs.changed == 'true'}} | ||
DellaBitta marked this conversation as resolved.
Show resolved
Hide resolved
|
||
steps: | ||
- name: Set up Node (14) | ||
uses: actions/setup-node@v3 | ||
with: | ||
node-version: 14.x | ||
- name: install Chrome stable | ||
run: | | ||
sudo apt-get update | ||
sudo apt-get install google-chrome-stable | ||
- name: Download build archive | ||
uses: actions/download-artifact@v3 | ||
with: | ||
name: build.tar.gz | ||
- name: Unzip build artifact | ||
run: tar xf build.tar.gz | ||
- name: Bump Node memory limit | ||
run: echo "NODE_OPTIONS=--max_old_space_size=4096" >> $GITHUB_ENV | ||
- name: Test setup and yarn install | ||
run: | | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: reduce this single-line bash script to one line (i.e. get rid of the pipe There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, will address in the next PR. |
||
cp config/ci.config.json config/project.json | ||
- name: Run compat tests | ||
run: cd packages/firestore-compat && yarn run test:ci | ||
|
||
test-chrome: | ||
name: Test Firestore | ||
strategy: | ||
matrix: | ||
test-name: ["test:browser", "test:travis", "test:lite:browser", "test:browser:prod:nameddb", "test:lite:browser:nameddb"] | ||
DellaBitta marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
wu-hui marked this conversation as resolved.
Show resolved
Hide resolved
|
||
runs-on: ubuntu-latest | ||
|
||
needs: build | ||
if: ${{ needs.build.outputs.changed == 'true'}} | ||
steps: | ||
- name: Set up Node (14) | ||
uses: actions/setup-node@v3 | ||
with: | ||
node-version: 14.x | ||
- name: install Chrome stable | ||
run: | | ||
sudo apt-get update | ||
sudo apt-get install google-chrome-stable | ||
- name: Download build archive | ||
uses: actions/download-artifact@v3 | ||
with: | ||
name: build.tar.gz | ||
- name: Unzip build artifact | ||
run: tar xf build.tar.gz | ||
- name: Bump Node memory limit | ||
run: echo "NODE_OPTIONS=--max_old_space_size=4096" >> $GITHUB_ENV | ||
- name: Test setup and yarn install | ||
run: | | ||
cp config/ci.config.json config/project.json | ||
- name: Run tests | ||
run: cd packages/firestore && yarn run ${{ matrix.test-name }} | ||
|
||
|
||
test-firefox: | ||
name: Test Firestore on Firefox If Changed | ||
name: Test Firestore on Firefox | ||
strategy: | ||
matrix: | ||
test-name: ["test:browser", "test:travis", "test:lite:browser", "test:browser:prod:nameddb", "test:lite:browser:nameddb"] | ||
# Whatever version of Firefox comes with 22.04 is causing Firefox | ||
# startup to hang when launched by karma. Need to look further into | ||
# why. | ||
runs-on: ubuntu-20.04 | ||
|
||
needs: build | ||
if: ${{ needs.build.outputs.changed == 'true'}} | ||
steps: | ||
- name: install Firefox stable | ||
run: | | ||
sudo apt-get update | ||
sudo apt-get install firefox | ||
- name: Checkout Repo | ||
uses: actions/checkout@master | ||
- name: Download build archive | ||
uses: actions/download-artifact@v3 | ||
with: | ||
# This makes Actions fetch all Git history so run-changed script can diff properly. | ||
fetch-depth: 0 | ||
name: build.tar.gz | ||
- name: Unzip build artifact | ||
run: tar xf build.tar.gz | ||
- name: Set up Node (14) | ||
uses: actions/setup-node@v3 | ||
with: | ||
|
@@ -58,10 +152,7 @@ jobs: | |
- name: Test setup and yarn install | ||
run: | | ||
cp config/ci.config.json config/project.json | ||
yarn | ||
- name: build | ||
run: yarn build:changed firestore | ||
- name: Run tests if firestore or its dependencies has changed | ||
run: xvfb-run yarn test:changed firestore | ||
- name: Run tests | ||
run: cd packages/firestore && xvfb-run yarn run ${{ matrix.test-name }} | ||
env: | ||
BROWSERS: 'Firefox' |
Uh oh!
There was an error while loading. Please reload this page.