Skip to content
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

[ui, ci] Ember Exam failures should cause overall test failures #24603

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+17 −5
Open

[ui, ci] Ember Exam failures should cause overall test failures #24603

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7b8ca14
Continue-on-error placement was causing failing exams to get a green …
philrenaud Dec 4, 2024
a6946a7
no-op to trigger ui tests
philrenaud Dec 4, 2024
b8d6e1c
Maybe I dont need explicit test failure steps after all?
philrenaud Dec 6, 2024
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
11 changes: 10 additions & 1 deletion .github/workflows/test-ui.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ jobs:
- pre-test
runs-on: ${{ endsWith(github.repository, '-enterprise') && fromJSON('["self-hosted", "ondemand", "linux", "type=m7a.2xlarge;m6a.2xlarge"]') || 'ubuntu-latest' }}
timeout-minutes: 30
continue-on-error: true
defaults:
run:
working-directory: ui
Expand All @@ -61,15 +62,23 @@ jobs:
secrets: |-
kv/data/teams/nomad/ui PERCY_TOKEN ;
- name: ember exam
id: exam
env:
PERCY_TOKEN: ${{ env.PERCY_TOKEN || secrets.PERCY_TOKEN }}
PERCY_PARALLEL_NONCE: ${{ needs.pre-test.outputs.nonce }}
run: |
yarn exam:parallel --split=${{ matrix.split }} --partition=${{ matrix.partition }} --json-report=test-results/test-results.json
continue-on-error: true
# We have continue-on-error set to true, but we still want to alert the author if
# there are test failures or timeouts. Without it, we'll get errors in our output,
# but the workflow will still succeed / have a green checkmark.
- name: Express timeout failure
if: ${{ failure() }}
run: exit 1
- name: Check test status
if: steps.exam.outcome != 'success'
run: |
echo "Tests failed or timed out in partition ${{ matrix.partition }}"
exit 1
Copy link
Member

Choose a reason for hiding this comment

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

if we're checking the exam step's outcome specifically, do we need "Express timeout failure" above?

also, if you exit 1 here, the Upload step below wouldn't run. presumably you'd want the test results even if something failed?

but also, does continue-on-error at the job level, moved up to line 39, mean that all steps are continued, so this exit 1 would be ignored just like any error that yarn exam might have spit out?

I don't recall exactly the nuances of when which type of results are skipped, and I don't think I've done a job-level continue-on-error... if you're not sure either, I'd suggest making a little repo that's just for testing github actions, and creating little toy workflows so you can iterate more quickly. I suppose you could do that in a branch in this repo, but I like to do it in branches in my own lil experiment repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

also, if you exit 1 here, the Upload step below wouldn't run. presumably you'd want the test results even if something failed?

I think probably I wouldn't want to upload the results in that case, actually; the point of uploading partition results outside of the running test step is to do look-backs and measure test timing, etc.

Although: if I also wanted to measure flakiness, I guess I would still want to upload things if there was a failure.

I think I need to think about this a bit more.

- name: Upload partition test results
if: github.event_name == 'push' && github.ref == 'refs/heads/main'
uses: actions/upload-artifact@b4b15b8c7c6ac21ea08fcf65892d2ee8f75cf882 # v4.4.3
Expand Down
1 change: 0 additions & 1 deletion ui/app/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
{{content-for "body"}}

<script src="{{rootURL}}assets/vendor.js"></script>

Copy link
Member

Choose a reason for hiding this comment

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

🙄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is because of

    paths:
      - "ui/**"

But I'm going to try to make this workflow file part of that, too.

<script src="{{rootURL}}assets/nomad-ui.js"></script>

{{content-for "body-footer"}}
Expand Down
Loading