Skip to content

Added js vtest on CI #16237

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

Merged
merged 6 commits into from
Feb 9, 2023
Merged

Conversation

igorkorsukov
Copy link
Contributor

No description provided.

@igorkorsukov igorkorsukov force-pushed the tests/engr_draw_10 branch 11 times, most recently from d6442ad to 5cc0180 Compare February 9, 2023 13:17
@igorkorsukov igorkorsukov marked this pull request as ready for review February 9, 2023 13:17
@igorkorsukov igorkorsukov force-pushed the tests/engr_draw_10 branch 4 times, most recently from 0bd0255 to b9f34f8 Compare February 9, 2023 14:54
@igorkorsukov igorkorsukov merged commit ca41a6c into musescore:master Feb 9, 2023
@cbjeukendrup
Copy link
Member

@igorkorsukov I'm afraid I see a problem:
So now you need to update the reference files when the tests are changed (that's annoying, but not the problem I mean).

But when the reference files are updated, the test on CI will pass without generating diff images, because there is no diff anymore then. So, once you have updated the reference files, you can't see anymore what has changed relative to the master branch. That's very inconvenient. Is there a solution for this?

The purpose of the VTests was namely not only automatically detecting changes, but also visually (not automatically) determining whether the changes are correct, by looking not only at the new situation, but also at the diff. It is important that we can do that.

(Also, it's regrettable that the "GIF" diffs are gone now, because those were actually the most useful in some cases. But it looks like those can be added back relatively easily.)

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.

3 participants