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.

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

chromaui/action doesn't support merge_group events #741

Closed
janeklb opened this issue Apr 20, 2023 · 7 comments
Closed

chromaui/action doesn't support merge_group events #741

janeklb opened this issue Apr 20, 2023 · 7 comments
Labels
bug Classification: Something isn't working needs triage Tracking: Issue needs confirmation

Comments

@janeklb
Copy link

janeklb commented Apr 20, 2023

Bug report

We would like to run the chromaui/action github action as part of our (new) merge queue workflow. Unfortunately the chromaui action errors out saying it doesn't support the merge_group event

image

Not sure if this should be called a bug or a feature request - please feel free to triage it accordingly!

@janeklb janeklb added bug Classification: Something isn't working needs triage Tracking: Issue needs confirmation labels Apr 20, 2023
@chinanderm
Copy link

chinanderm commented Aug 15, 2023

We are running into this issue now as well. We have our Chromatic workflows as required checks on our PRs, thus those checks need to also run in the merge queue checks.

However, I'm not exactly sure it is relevant to run Chromatic during merge queue checks. During these checks, a temporary branch is made for the purposes of running these checks with all the other PRs ahead in the queue. This temporary branch would wind up in Chromatic as a branch with snapshots -- is that desired?

And if there are snapshot changes detected during this process, how does that get resolved? Someone would have to manually accept/deny the changes and the checks would be in a pending state until that is done, holding up the entire queue until the timeout limit is reached (default 60 minutes).

@chinanderm
Copy link

Chromatic team -- is there any update on this? This is preventing us from leveraging merge queue because we have our Chromatic UI tests as required PR checks.

@mhemmings
Copy link
Contributor

It's such a shame this isn't working. Our Chromatic project was the main reason for us using merge queues, but we're not able to because of this issue. Manual races to see who can merge main and deploy first is probably the stickiest bit of our dev process.

For reference, our approach was:

  • To fail any merges in the queue if any unaccepted changes appear in Chromatic. It's hoped this never happens, but of course if it does (e.g. something new in main causes unexpected behaviour in feature-branch) we definitely want to stop and not merge it without human interaction!! (fix or accept Chromatic changes)
  • We hoped to use exitZeroOnChanges to achieve this, which I think is correct?

I can't see any reason the restrictions are put in place? Our next step is to look at using the Chromatic CLI directly to get around this. If we make any progress with that approach, I'll share in here.

If there's any other info I can provide or help with anything at all, please do reach out.

@thafryer
Copy link
Member

Hi Folks! We have added support for the merge_group event in v7.6.0! Thanks, @mhemmings, for the contribution!

@mhemmings
Copy link
Contributor

Hey @thafryer

We've having trouble using this with required Github checks. The "UI Review" check hangs indefinitely, as its status never gets reported by Chromatic. The status of "UI Tests" gets reported by Chromatic successfully.

Ideally, our workflow is as documented above, where we want Chromatic to check there are "not accepted" changes within the queue (changes get accepted at PR time, and PRs can't be merged without them)

While on this topic, do you understand the difference between "UI Tests" and "UI Review", and why our designer needs to accept all changes twice and in different ways, to get these checks passing? It's all very confusing!!

Thanks in advance for your help!

Screenshot 2023-11-01 at 13 32 24

@janeklb
Copy link
Author

janeklb commented Nov 1, 2023

While on this topic, do you understand the difference between "UI Tests" and "UI Review", and why our designer needs to accept all changes twice and in different ways, to get these checks passing? It's all very confusing!!

I'm still not 100% clear about the difference, but something that has helped my team is having engineers/developers take responsibility for "UI Tests" (they are incrementally updated each time you build a branch), and only involve the designer for "UI Review" (which is updated in full on every run)

@mhemmings
Copy link
Contributor

Hi @thafryer, did you see my above issue with status checks still not being reported correctly?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Classification: Something isn't working needs triage Tracking: Issue needs confirmation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants