-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Treat SuperPMI failures in AzDO pipelines as pipeline failures #68196
Treat SuperPMI failures in AzDO pipelines as pipeline failures #68196
Conversation
Now, any SuperPMI failure in the replay, asmdiffs, and checked/release asmdiffs pipeplines is ignored, and doesn't cause AzDO to treat the entire job as failed. The job does get a "!" icon, but that's only if you visit the AzDO UI: the GitHub page on a PR doesn't see that failure. Now that we have more experience with these pipelines, and we are using replay and asmdiffs with JIT PRs, we want the failures to be more visible, and treated as failures. Note that asm diffs should not be treated as failures.
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
Tagging subscribers to this area: @JulieLeeMSFT Issue DetailsNow, any SuperPMI failure in the replay, asmdiffs, and checked/release Now that we have more experience with these pipelines, and we are using Note that asm diffs should not be treated as failures.
|
@jakobbotsch @kunalspathak @dotnet/jit-contrib PTAL |
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.
Note that asm diffs should not be treated as failures.
However, I still see that you changed the asmdiff file as well? If there are asmdiffs, do you remember if the script's exit code is 0. If yes, then your change seems good. We will only show it failed when anything really fails during asmdiff.
@kunalspathak superpmi.py already does the right thing: it returns 0 (success) on MISSING data, or if there are asm diffs. Actually, for chk/rel asm diffs, if there are asm diffs we do consider it a failure (return 1). This is because there should never be chk/rel asmdiffs, so that is a real failure we want a pipeline to report. |
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
Note: the spmi replays will fail because of an assert that was added with #68161 (until that is fixed). |
…t#68196) Now, any SuperPMI failure in the replay, asmdiffs, and checked/release asmdiffs pipeplines is ignored, and doesn't cause AzDO to treat the entire job as failed. The job does get a "!" icon, but that's only if you visit the AzDO UI: the GitHub page on a PR doesn't see that failure. Now that we have more experience with these pipelines, and we are using replay and asmdiffs with JIT PRs, we want the failures to be more visible, and treated as failures. Note that asm diffs should not be treated as failures.
Now, any SuperPMI failure in the replay, asmdiffs, and checked/release
asmdiffs pipeplines is ignored, and doesn't cause AzDO to treat the entire
job as failed. The job does get a "!" icon, but that's only if you visit the
AzDO UI: the GitHub page on a PR doesn't see that failure.
Now that we have more experience with these pipelines, and we are using
replay and asmdiffs with JIT PRs, we want the failures to be more visible,
and treated as failures.
Note that asm diffs should not be treated as failures.