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

Add Fix workflow #6

Merged
merged 29 commits into from
Feb 24, 2024
Merged

Add Fix workflow #6

merged 29 commits into from
Feb 24, 2024

Conversation

yschimke
Copy link
Collaborator

@yschimke yschimke commented Feb 24, 2024

Runnable on a fork of the repository such as

https://github.com/yschimke/ComposeOClock/actions/workflows/fix_workflow.yaml

When run on a branch, generates updated screenshots and commits them.

Example https://github.com/yschimke/ComposeOClock/actions/runs/8030224947

@yschimke yschimke marked this pull request as ready for review February 24, 2024 11:42
@yschimke
Copy link
Collaborator Author

Diffs should be one offs from running on ubuntu-latest e32c56b

@LouisCAD
Copy link
Member

Do you know why the resulting screenshots are different, yet looking the same to the eye?

@yschimke
Copy link
Collaborator Author

@yschimke
Copy link
Collaborator Author

More canonical link robolectric/robolectric#8457

@LouisCAD
Copy link
Member

Hum… this mismatch is not such an ideal situation…
I'd like the workflow to not be so dependent on GitHub actions or a specific desktop OS that I might not be using myself.

@LouisCAD
Copy link
Member

BTW, what's the reason you programmed it to not run when on the default (main) branch?

@yschimke
Copy link
Collaborator Author

I didn't think having it make changes to main was safe. But it's manual, so up to you

@yschimke
Copy link
Collaborator Author

Hum… this mismatch is not such an ideal situation…
I'd like the workflow to not be so dependent on GitHub actions or a specific desktop OS that I might not be using myself.

I don't have a good solution then.

Options

This) Linux is canonical and updated in CI
B) relax tolerances so pass on both, and refresh occasionally in CI
C) make Mac the master and change runner?

Don't test screenshots in CI, just update locally

@LouisCAD
Copy link
Member

LouisCAD commented Feb 24, 2024

@yschimke I think I'd be happy with option B, but I don't know where/how to setup that tolerance, and which number to give it.
Do you have any clue?

@yschimke
Copy link
Collaborator Author

Yep, will do.

@yschimke
Copy link
Collaborator Author

0.01 or 1% but can be tuned.

If you temporarily set it to 0, and run verifyRoborazziDebug, you'll see the diffs. It's mostly antialiasing of curves etc.

@LouisCAD
Copy link
Member

Thank you very much, Yuri!

That's a very welcome addition (plus it's fixing the CI 😄)

@LouisCAD LouisCAD merged commit 9adf5ad into Splitties:main Feb 24, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants