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

Introduce automated UI testing based on asciinema #117

Merged
merged 4 commits into from
Nov 26, 2023

Conversation

hartwork
Copy link
Collaborator

This will prevent the next case of #102 right in the pull request introducing the regression.

This took quite a while to build, I ran into many interesting walls in the process and now finally have it working. Happy to answer questions about it.

@hartwork hartwork force-pushed the ui-testing-ci-asciinema branch from 0cd5138 to 5ecce0f Compare November 25, 2023 00:38
Copy link
Collaborator

@edgar-bonet edgar-bonet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated visual inspection in CI? Wow! That's awesome! I find this really great, but at the same time I am a bit worried this could cause some friction for contributions that legitimately change the rendering. I understand in such a case one could push the change, wait for the CI to fail, download the generated artifacts, commit the actual images as the expected ones, and push again. It would be a bit cumbersome though.

Could one instead just run record.sh to get the expected images? I can't find the agg command on the Ubuntu repos, is there an alternative?

.github/workflows/linux_and_macos.yml Show resolved Hide resolved
ttyplot.c Outdated Show resolved Hide resolved
@hartwork
Copy link
Collaborator Author

hartwork commented Nov 25, 2023

Automated visual inspection in CI? Wow! That's awesome!

@edgar-bonet glad you like the idea!

I find this really great, but at the same time I am a bit worried this could cause some friction for contributions that legitimately change the rendering. I understand in such a case one could push the change, wait for the CI to fail, download the generated artifacts, commit the actual images as the expected ones, and push again. It would be a bit cumbersome though. Could one instead just run record.sh to get the expected images?

Yes!

The fail-and-download approach would work, but I would personally see it as a fallback, as a second option, not the primary way, at least that was not intended. What I did locally to produce fresh images was basically this:

cd recordings/
./record.sh
for i in actual-*.png ; do cp ${i} ${i/actual/expected}; done
for i in expected-*.png ; do zopflipng -y ${i} ${i}; done  # https://github.com/google/zopfli
git add expected-*.png

I can make that a second script sync.sh or so if that changes something. Maybe without the git addto not interfere with the developers workflow. What do you think?

I can't find the agg command on the Ubuntu repos, is there an alternative?

agg is https://github.com/asciinema/agg and Gentoo doesn't have a package either. (It does have x11-libs/agg but that's something else). What I did and considered okay at least for myself was…

cargo install --git https://github.com/asciinema/agg
export PATH="${PATH}:${HOME}/.cargo/bin"

…and then command agg is readily available with a version fully compiled from source.

@hartwork hartwork force-pushed the ui-testing-ci-asciinema branch 4 times, most recently from c64eb16 to 8189a28 Compare November 25, 2023 23:35
@hartwork hartwork changed the base branch from master to development November 25, 2023 23:37
@hartwork hartwork force-pushed the ui-testing-ci-asciinema branch from 8189a28 to a187c2e Compare November 26, 2023 00:15
@hartwork hartwork force-pushed the ui-testing-ci-asciinema branch from a187c2e to 8677913 Compare November 26, 2023 00:24
@hartwork hartwork requested a review from edgar-bonet November 26, 2023 00:24
@hartwork hartwork force-pushed the ui-testing-ci-asciinema branch from 8677913 to 86b87f2 Compare November 26, 2023 03:18
@hartwork hartwork force-pushed the ui-testing-ci-asciinema branch from 86b87f2 to 84e89d9 Compare November 26, 2023 03:25
Copy link
Collaborator

@edgar-bonet edgar-bonet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

CONTRIBUTING.md Show resolved Hide resolved
@hartwork hartwork merged commit 01211e7 into tenox7:development Nov 26, 2023
5 checks passed
@hartwork hartwork deleted the ui-testing-ci-asciinema branch November 27, 2023 03:09
@hartwork hartwork added this to the 1.6.0 milestone Nov 28, 2023
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