-
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] retain artifacts from test runs including test timing #24555
Conversation
73d86fa
to
acdd210
Compare
@@ -24,6 +24,7 @@ | |||
{{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.
no-op to trigger the
pull_request:
paths:
- "ui/**"
part of the action.
Ember Test Audit comparison
|
52e500b
to
71b073d
Compare
0729338
to
3391750
Compare
3391750
to
fef9738
Compare
fef9738
to
73f3a32
Compare
73f3a32
to
8f358e6
Compare
8f358e6
to
699ed1d
Compare
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.
LGTM
c4dc4c8
to
8f81bf8
Compare
8f81bf8
to
0603cab
Compare
0603cab
to
040060f
Compare
040060f
to
e9bf68a
Compare
e9bf68a
to
ebfcad6
Compare
ebfcad6
to
2adc472
Compare
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.
LGTM!
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.
Super cool to see this work get picked up again 🤩
for (let i = 1; i <= NUM_PARTITIONS; i++) { | ||
try { | ||
const data = JSON.parse( | ||
fs.readFileSync(`../test-results/test-results-${i}/test-results.json`).toString() | ||
); |
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.
Will there be anything else in the test-results directory other than subdirectories for each partition?
I wonder if you can walk the dir tree instead of relying on synchronizing NUM_PARTITIONS
and the GHA matrix config.
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.
Yeah, I left a pretty verbose dir trail here with the hope that I'd populate this with further artifacts / test-run metadata. No explicit plans yet, though!
tests: results | ||
}; | ||
|
||
fs.writeFileSync('../ui/combined-test-results.json', JSON.stringify(output, null, 2)); |
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.
Doing a combine step that takes json and emits json is really nice. Should prove to be more flexible than trying to do all the collating, aggregating, and analysis in one script.
Description
Our
ember-test-audit
action is currently failing, and I'm using that as an excuse to improve our front-end testing story generally.This will be a two-step approach, and this PR is the first step:
test-ui
test results on merge tomain
This PR adds artifact uploads on the back of
test-ui
. Because those are partitioned/run concurrently 4x, part of this is combining those.The artifact generated looks like this:
In phase 2 of this, we'll be able to do test-by-test duration averages and comparisons, and tease out those tests with the greatest deltas, find flakes, etc.