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 UI Review hangs when it is a mandatory check with github merge queue enabled #871

Closed
adrianbruntonsagecom opened this issue Dec 7, 2023 · 11 comments · Fixed by #929
Labels
bug Classification: Something isn't working needs triage Tracking: Issue needs confirmation released Verdict: This issue/pull request has been released

Comments

@adrianbruntonsagecom
Copy link
Contributor

adrianbruntonsagecom commented Dec 7, 2023

Bug report

Following on from this comment:
#741 (comment)

We are experiencing the same thing.

Our main branch protections include:

  • Merge queue enabled
  • Required checks:
    • Validate / Chromatic
    • UI Review
    • UI Tests
    • Storybook Publish

Snippet of our workflow:

# .github\workflows\validate.yml

name: Validate

on:
  pull_request:
  merge_group:

jobs:
  chromatic:
    name: Chromatic
    runs-on: ubuntu-latest
    steps:
      - name: Checkout repository
        uses: actions/checkout@v4
        with:
          fetch-depth: 0

      - name: Install dependencies
        uses: "./.github/actions/install-dependencies"

      - uses: chromaui/action@v10
        id: chromatic
        with:
          projectToken: ${{ secrets.CHROMATIC_PROJECT_TOKEN }}
          exitOnceUploaded: true

We'd like to continue using the merge queue as we have historically been. But given we put greater importance on the validation result of "UI Review" and "UI Tests", we've had to disable it for the time being as it is currently hanging and unresponsive, even after the build finishes with all snapshots found.

Appreciate any feedback.

Editing to add screenshots for clarification. This is the view of a completed PR:
image

And this is the merge queue after the PR has been queued:
image

As you can see, all checks have completed except for the Chromatic "external" checks which appear to have reset since they were passed on the PR.

@adrianbruntonsagecom adrianbruntonsagecom added bug Classification: Something isn't working needs triage Tracking: Issue needs confirmation labels Dec 7, 2023
@adrianbruntonsagecom
Copy link
Contributor Author

Just a note further on this - it seems to work if a process it added to the workflow to amend the value of branchName to be the original branch name from the PR that's being merged through the merge queue. This can be found by parsing the merge queue branch name which contains the PR number, and then using the github CLI to get the original branch name.

It would be nice however if this was integrated into the GitHub action.

@ghengeveld
Copy link
Member

Hi @adrianbruntonsagecom, could you try using chromaui/[email protected]? That's the latest before 10.3.0 where we introduced a change to how we handle branch names in GitHub merge queues. It might not matter but I'd like to rule it out.

@ghengeveld
Copy link
Member

Actually, it seems we made that change after you initially reported the problem and must've been why we made the change in the first place. We probably just forgot to report back to you about it, sorry about that. Can you verify that using the latest (chromaui/action@latest) resolves the problem?

@adrianbruntonsagecom
Copy link
Contributor Author

Actually, it seems we made that change after you initially reported the problem and must've been why we made the change in the first place. We probably just forgot to report back to you about it, sorry about that. Can you verify that using the latest (chromaui/action@latest) resolves the problem?

Assuming @v10 grabs whatever the latest minor version is within that major version, this was still an issue as of last week where I put my own fix in to parse the PR number from the branch to find the original branch name.

@ghengeveld
Copy link
Member

Yes, @v10 uses the latest minor/patch version, currently v10.9.4.
Could you write in on our support chat, so we can dig a little deeper?

@adrianbruntonsagecom
Copy link
Contributor Author

@ghengeveld Just updating - I'll contact your support team, but just updating that it's still an issue. I made a separate github protected branch. Enabled my Chromatic job, plus the added public UI review and UI tests jobs as required checked and enabled the merge queue. I then made a PR with a visual regression change targetting that new branch and approved it within the PR. When it was added to the merge queue however, it appeared to not get accepted:
image

My assumption is this is because it's dealing with a new branch (refs/heads/gh-readonly-queue/merge-queue-target/pr-352-ca25cb9b8d998e205a88b9edaae28014a405a5ff) rather than the original merge-queue-source branch.

@ghengeveld
Copy link
Member

🚀 Issue was released in v11.0.1 🚀

@ghengeveld ghengeveld added the released Verdict: This issue/pull request has been released label Mar 5, 2024
@adrianbruntonsagecom
Copy link
Contributor Author

adrianbruntonsagecom commented Mar 6, 2024

🚀 Issue was released in v11.0.1 🚀

@ghengeveld What's the process of getting the GitHub action updated? As far as I can tell, this repo needs updating to align with the CLI version?
https://github.com/chromaui/action

@ghengeveld
Copy link
Member

ghengeveld commented Mar 6, 2024

Yeah, we used to roll that out automatically but we recently had an issue with that so now it's manual. I've updated it now.

@MieszkoWrzeszczynski
Copy link

It seems that the issue still occurs in 11.3.0 version.

image image

@cgbl-90
Copy link

cgbl-90 commented Apr 23, 2024

@MieszkoWrzeszczynski, please email or chat with Chromatic support for more details to review. If you have the build logs, please include them.

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 released Verdict: This issue/pull request has been released
Projects
None yet
4 participants