-
Notifications
You must be signed in to change notification settings - Fork 1
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 scaling tests #140
Add scaling tests #140
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #140 +/- ##
==========================================
- Coverage 68.45% 68.37% -0.08%
==========================================
Files 16 16
Lines 875 876 +1
Branches 126 126
==========================================
Hits 599 599
- Misses 250 251 +1
Partials 26 26 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only looked at the CI results for now, but the direct cause for the failure is:
E AssertionError: gtk4-demo died without being waited for or killed
But that's because it's just not installed (we should improve that error report to say the executable wasn't found).
That's weird, I've been using it to test for the past two days. I recall seeing another error in the output of the robot near the top. Have you seen that? |
On your machine, yes - but who installed |
mir-ci/mir_ci/tests/robot/suites/fractional_scale_v1/fractional_scale_v1.robot
Outdated
Show resolved
Hide resolved
@tarek-y-ismail and with these tweaks your tests pass (the failures are only on neverputt, that's a known issue fixed in Mir already.) You can see the test results better in the "Test results" check. It sometimes gets attached to On failure, you can also download the https://github.com/canonical/mir-ci/actions/runs/10166013391 You'll then see a video of the run as well as the screenshot that failed to match - this way you can cut out exactly the template you need in CI. All this... will get better soon when we integrate it all with the certification team's tooling. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you should be able to reduce the number of templates needed to just two per scale:
- top-left portion of the app that fits at the scale in the minimum resolution
- the expected window title after clicking the button
You can then use a static offset (multiplied by scale) to reach the intended button.
Updating the templates will require less work, then.
You got hung up on the "Combo Boxes" button, but at 1024x768 at scale >=1.75 it's not on screen when maximized, so maybe use/expect a different button? One that's on screen across all the cases you want to handle?
And I think you might have uncovered a bug in the initial placement logic - we probably should place the window's (0, 0) on screen, not it's lower right corner - when it doesn't fit.
Regarding this, I wanted to aim for a button on a far edge of the application as that's where issues usually pop up when it comes to scaling, hence why I originally used "simple constraints". I worked around the button not being visible at higher scales by scrolling the list down and matching again if the button is not found
I think I'll try this out. Thanks for the suggestion :) |
I think it would be appropriate to just go for the last visible button, at the minimum supported resolution. You can also aim for different buttons on different scales. |
7e491b2
to
04009e2
Compare
04009e2
to
6713e68
Compare
b14edfa
to
51cdaff
Compare
51cdaff
to
eb5a2be
Compare
eb5a2be
to
3de898a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be appropriate to just go for the last visible button, at the minimum supported resolution. You can also aim for different buttons on different scales.
Ping on this one.
mir-ci/mir_ci/tests/robot/suites/fractional_scale_v1/2.0-gtk4-demo-screenshot-floating.png
Outdated
Show resolved
Hide resolved
121c6eb
to
4589cfd
Compare
1fafc63
to
b06e541
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handful of nits.
I'm looking into why they're failing in CI - there's some subtle differences, we may want to avoid having the full screenshots as templates, as e.g. Frame will always maximize.
mir-ci/mir_ci/tests/robot/suites/fractional_scale_v1/fractional_scale_v1.robot
Outdated
Show resolved
Hide resolved
c6dabfb
to
7c7389b
Compare
Additional changes reviewed, LGTM! Will just rebase to clean up the commit history then merge. |
This is because the latter goes out of view at 2.0 scale.
Co-authored-by: Michał Sawicz <[email protected]>
Co-authored-by: Michał Sawicz <[email protected]>
Also drop "fractional", it's not only testing that.
7c7389b
to
31dbc8a
Compare
No description provided.