feat(ci): analyses snapshot skip pr on fail label #15792

y3rsh · 2024-07-24T22:11:44Z

Overview

Keep making the analyses snapshot test workflow more flexible allowing a label on the PR to prevent failure PRs from opening. Adding the label no-analyses-snapshot-pr on your PR will cause the workflow to skip opening a PR for snapshot updates if the snapshot tests fail.
Added a step to diff the snapshots in the PR target branch against the PR branch. No action is taken based on the outcome. Purely informational. Will this information be noticed and used as a reminder to synchronize with the target branch?

Updates from feedback

default OPEN_PR_ON_FAILURE to false
as is the case currently if the analyses snapshot test fails you will see a red X on that test and you can read the logs or download and view the HTML test report.
if you want to open a PR into your branch that would heal the snapshots, add gen-analyses-snapshot-pr label and a PR will get opened
create a specific step to handle PRs for the overnight scheduled test of edge.

Risk

No risk or change to application code, CI only

y3rsh · 2024-07-24T22:32:22Z

Proof

https://github.com/Opentrons/opentrons/actions/runs/10084931967/job/27884729239
- Has successful run of the new informational step.
https://github.com/Opentrons/opentrons/actions/runs/10085015669/job/27884972771?pr=15792#step:2:20
- Shows if you have the label on the PR the Flag is set correctly.

SyntaxColoring

Cool, thanks!

I have been thinking that this behavior should be the default. Like, we'd somehow get it so that if a snapshot test fails, you get just a normal red X in the CI section. And if you want a PR, you need to take some deliberate action, like commenting with a /slashcommand, tagging the PR with a magic tag, clicking a magic link, merging from a magic branch, or just doing it locally.

Because a recurring situation seems to be:

Analysis snapshot tests start failing on edge for whatever reason
5 people branch their PRs off of edge
The snapshot tests fail in those 5 people's PRs
The system creates 5 snapshot PRs

(1)-(3) happen occasionally for normal tests and it's merely mildly annoying; (4) is the thing that's been uniquely noisy about snapshot tests.

DerekMaggio

oooh... fancy.
I do agree with Max about how noisy the snapshots are.
I like the idea of a /command on the PR

sfoster1 · 2024-07-25T13:59:56Z

Agree with @DerekMaggio and @SyntaxColoring

Also can we get the opened PR (if it does exist) to link to the pr for the branch that it's targeting, if possible? And ideally backlink to itself in the comment on that PR?

y3rsh · 2024-07-25T19:25:43Z

Addressed the comments. Went with label as it is more visible than a comment and easier to handle for this binary circumstance.

github-actions · 2024-07-25T19:46:58Z

A PR has been opened to address analyses snapshot changes. Please review the changes here: https://github.com/Opentrons/opentrons/pull/

SyntaxColoring

Thank you!

SyntaxColoring · 2024-07-25T19:54:22Z

...es_snapshot_test/test_analysis_snapshot[0a9ef592c8][Flex_S_v2_18_Illumina_DNA_Prep_48x].json

-    {
-      "commandType": "home",
-      "completedAt": "TIMESTAMP",
-      "createdAt": "TIMESTAMP",
-      "id": "UUID",
-      "key": "50c7ae73a4e3f7129874f39dfb514803",
-      "notes": [],
-      "params": {},
-      "result": {},
-      "startedAt": "TIMESTAMP",
-      "status": "succeeded"
-    },


Reminder to undo this if it was just a change for testing.

SyntaxColoring · 2024-07-25T19:55:36Z

.github/workflows/analyses-snapshot-test.yaml

+          commit-message: 'fix(analyses-snapshot-testing): heal analyses snapshots'
+          title: 'fix(analyses-snapshot-testing): heal ${{ env.ANALYSIS_REF }} snapshots'
+          body: 'This PR was requested on the PR https://github.com/${{ github.repository }}/pull/${{ github.event.pull_request.number }}'


github-actions · 2024-07-25T20:15:38Z

A PR has been opened to address analyses snapshot changes. Please review the changes here: https://github.com/Opentrons/opentrons/pull/

fixed this issue where the pr number was not being added to the URL correctly

github-actions · 2024-07-25T20:50:28Z

A PR has been opened to address analyses snapshot changes. Please review the changes here: #15803

The above was on purpose to see the PR was opened with a link correctly after adding the tag.

feat(ci): analyses snapshot skip pr on fail label

200f9fa

y3rsh self-assigned this Jul 24, 2024

y3rsh requested a review from a team as a code owner July 24, 2024 22:11

y3rsh added 2 commits July 24, 2024 17:21

re-order and remove HEAD

3a5ff4c

try again

a567950

y3rsh added the no-analyses-snapshot-pr label Jul 24, 2024

testing the label

4282173

SyntaxColoring approved these changes Jul 25, 2024

View reviewed changes

DerekMaggio approved these changes Jul 25, 2024

View reviewed changes

y3rsh added 3 commits July 25, 2024 13:31

Merge branch 'edge' into analyses-snapshots-more-options

2a489da

address feedback

7152683

run the workflow when pr labled

aaf2853

y3rsh requested review from SyntaxColoring and DerekMaggio July 25, 2024 19:22

y3rsh added 3 commits July 25, 2024 14:27

break a snapshot

9509226

make a change that will trigger

4d59bd9

|| true will let it keep going?

0d6e22a

github-actions bot mentioned this pull request Jul 25, 2024

fix(analyses-snapshot-testing): heal analyses-snapshots-more-options snapshots #15801

Closed

revert snapshot

a3dfe42

SyntaxColoring approved these changes Jul 25, 2024

View reviewed changes

y3rsh added 2 commits July 25, 2024 14:56

bad snap and better diff capture

32a2737

had the if reversed with new conditional

e6fc09d

fix conditionals

18eafa8

y3rsh added the gen-analyses-snapshot-pr Generate a healing PR if the analyses snapshot test fails label Jul 25, 2024

github-actions bot mentioned this pull request Jul 25, 2024

fix(analyses-snapshot-testing): heal analyses-snapshots-more-options snapshots #15803

Closed

fix the snapshot

dd969d2

y3rsh removed the gen-analyses-snapshot-pr Generate a healing PR if the analyses snapshot test fails label Jul 25, 2024

y3rsh merged commit eaf9657 into edge Jul 25, 2024
6 checks passed

y3rsh deleted the analyses-snapshots-more-options branch July 25, 2024 22:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ci): analyses snapshot skip pr on fail label #15792

feat(ci): analyses snapshot skip pr on fail label #15792

y3rsh commented Jul 24, 2024 •

edited

Loading

y3rsh commented Jul 24, 2024 •

edited

Loading

SyntaxColoring left a comment •

edited

Loading

DerekMaggio left a comment

sfoster1 commented Jul 25, 2024

y3rsh commented Jul 25, 2024

github-actions bot commented Jul 25, 2024

SyntaxColoring left a comment

SyntaxColoring Jul 25, 2024

SyntaxColoring Jul 25, 2024

github-actions bot commented Jul 25, 2024 •

edited by y3rsh

Loading

github-actions bot commented Jul 25, 2024 •

edited by y3rsh

Loading

feat(ci): analyses snapshot skip pr on fail label #15792

feat(ci): analyses snapshot skip pr on fail label #15792

Conversation

y3rsh commented Jul 24, 2024 • edited Loading

Overview

Updates from feedback

Risk

y3rsh commented Jul 24, 2024 • edited Loading

Proof

SyntaxColoring left a comment • edited Loading

Choose a reason for hiding this comment

DerekMaggio left a comment

Choose a reason for hiding this comment

sfoster1 commented Jul 25, 2024

y3rsh commented Jul 25, 2024

github-actions bot commented Jul 25, 2024

SyntaxColoring left a comment

Choose a reason for hiding this comment

SyntaxColoring Jul 25, 2024

Choose a reason for hiding this comment

SyntaxColoring Jul 25, 2024

Choose a reason for hiding this comment

github-actions bot commented Jul 25, 2024 • edited by y3rsh Loading

github-actions bot commented Jul 25, 2024 • edited by y3rsh Loading

y3rsh commented Jul 24, 2024 •

edited

Loading

y3rsh commented Jul 24, 2024 •

edited

Loading

SyntaxColoring left a comment •

edited

Loading

github-actions bot commented Jul 25, 2024 •

edited by y3rsh

Loading

github-actions bot commented Jul 25, 2024 •

edited by y3rsh

Loading