-
Notifications
You must be signed in to change notification settings - Fork 2k
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
[ui, ci] Ember Exam failures should cause overall test failures #24603
base: main
Are you sure you want to change the base?
[ui, ci] Ember Exam failures should cause overall test failures #24603
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have questions! (because github actions can be quite confusing)
ui/app/index.html
Outdated
@@ -24,7 +24,6 @@ | |||
{{content-for "body"}} | |||
|
|||
<script src="{{rootURL}}assets/vendor.js"></script> | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙄
There was a problem hiding this comment.
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.
.github/workflows/test-ui.yml
Outdated
if: steps.exam.outcome != 'success' | ||
run: | | ||
echo "Tests failed or timed out in partition ${{ matrix.partition }}" | ||
exit 1 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Description
Noticed that failing ember-exam tests were still yielding green checkmarks (see
ember exam
in https://github.com/hashicorp/nomad/actions/runs/12143968210/job/33862207314)This moves the continue-on-error declaration out of the sub block and:
This builds on the work done in #24555