Skip to content

Polish new Firestore GA jobs #7568

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

Merged
merged 12 commits into from
Aug 17, 2023
Merged

Polish new Firestore GA jobs #7568

Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
7535ea9
Polish new Firestore GA jobs
wu-hui Aug 16, 2023
4027432
build failure
wu-hui Aug 16, 2023
c5e40fb
break build steps down
wu-hui Aug 16, 2023
e12ec04
test build error
wu-hui Aug 16, 2023
96069c4
no firestore
wu-hui Aug 17, 2023
c94ff86
new verify job
wu-hui Aug 17, 2023
61fd8a8
build error
wu-hui Aug 17, 2023
dd97567
firestore change
wu-hui Aug 17, 2023
13b4b9d
failed test
wu-hui Aug 17, 2023
20dfd8b
build error
wu-hui Aug 17, 2023
d16070f
build error
wu-hui Aug 17, 2023
fd3a86c
no firestore
wu-hui Aug 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 57 additions & 13 deletions .github/workflows/test-changed-firestore.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,23 +35,27 @@ jobs:
yarn
- name: build
id: build
# TODO(wuandy): Separate yarn and egrep into steps, so build failure
# is captured by github actions.
run: yarn build:changed firestore | egrep "Skipping all"
run: |
set -o pipefail
yarn build:changed firestore | tee ${{ runner.temp }}/yarn.log.txt
Copy link
Contributor

Choose a reason for hiding this comment

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

I recommend adding 2>&1 so that stdout and stderr have their lines in the correct relative order.

i.e.

yarn build:changed firestore 2>&1 | ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apparently this does not work with yarn build:changed for reasons I don't want to find out.

continue-on-error: false
Copy link
Contributor

Choose a reason for hiding this comment

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

Is continue-on-error: false required? Isn't false the default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No required, but I feel safer to keep it there.

- name: Check if Firestore is changed
id: check-changed
run: egrep "Skipping all" ${{ runner.temp }}/yarn.log.txt
# 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'
if: steps.check-changed.outcome != 'success'
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't really like that failure of the egrep command is conflated with the absence of "skipping all" from the output. Is there a more robust way to test for skipping the tests that you can think of?

Update: What about something like this:

skip_count=$(grep --count "Skipping all" test.txt)
echo "skip_count=$skip_count" >> "$GITHUB_OUTPUT"

Then later on you can do something like this:

if: steps.check-changed.outputs.skip_count == 0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's more worthwhile to change the script give more explicit result than doing grepping at all...not today though..i had enough github actions work recently.

id: set-output
run: echo "CHANGED=true" >> "$GITHUB_OUTPUT";
- name: Archive build
if: ${{ !cancelled() && steps.build.outcome != 'success' }}
if: ${{ !cancelled() && steps.build.outcome == 'success' && steps.check-changed.outcome != 'success' }}
run: |
tar -cf build.tar --exclude="\.git" .
tar -cf build.tar --exclude=.git .
gzip build.tar
- name: Upload build archive
if: ${{ !cancelled() && steps.build.outcome != 'success' }}
if: ${{ !cancelled() && steps.build.outcome == 'success' && steps.check-changed.outcome != 'success' }}
uses: actions/upload-artifact@v3
with:
name: build.tar.gz
Expand Down Expand Up @@ -81,8 +85,7 @@ jobs:
- 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
run: cp config/ci.config.json config/project.json
- name: Run compat tests
run: cd packages/firestore-compat && yarn run test:ci

Expand Down Expand Up @@ -112,11 +115,41 @@ jobs:
- 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
run: cp config/ci.config.json config/project.json
- name: Run tests
run: cd packages/firestore && yarn run ${{ matrix.test-name }}

compat-test-firefox:
name: Test Firestore Compatible on Firefox
# 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: Set up Node (14)
uses: actions/setup-node@v3
with:
node-version: 14.x
- 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 compat tests
run: cd packages/firestore-compat && xvfb-run yarn run test:ci
env:
BROWSERS: 'Firefox'

test-firefox:
name: Test Firestore on Firefox
Expand Down Expand Up @@ -147,9 +180,20 @@ jobs:
- 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
run: cp config/ci.config.json config/project.json
- name: Run tests
run: cd packages/firestore && xvfb-run yarn run ${{ matrix.test-name }}
env:
BROWSERS: 'Firefox'

# A job that fails if any required job in the test matrix fails,
# to be used as a required check for merging.
check-required-tests:
runs-on: ubuntu-latest
if: always()
name: Check all required tests results
needs: [build, test-chrome, compat-test-chrome]
steps:
- name: Check test matrix
if: needs.build.result == 'failure' || needs.test-chrome.result == 'failure' || needs.compat-test-chrome.result == 'failure'
run: exit 1