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 scaling tests #140

Merged
merged 19 commits into from
Aug 21, 2024
Merged

Add scaling tests #140

merged 19 commits into from
Aug 21, 2024

Conversation

tarek-y-ismail
Copy link
Contributor

No description provided.

@tarek-y-ismail tarek-y-ismail self-assigned this Jul 30, 2024
Copy link

codecov bot commented Jul 30, 2024

Codecov Report

Attention: Patch coverage is 25.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 68.37%. Comparing base (1a39b71) to head (31dbc8a).
Report is 24 commits behind head on main.

Files Patch % Lines
mir-ci/mir_ci/wayland/virtual_pointer.py 25.00% 3 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@Saviq Saviq left a 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).

@tarek-y-ismail
Copy link
Contributor Author

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?

@Saviq
Copy link
Collaborator

Saviq commented Jul 30, 2024

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 gtk-4-examples in CI? :)

@Saviq
Copy link
Collaborator

Saviq commented Jul 30, 2024

@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 PreCommit for some reason...

On failure, you can also download the test-results archive from the "Test" workflow and dig for the appropriate "log.html" for your test - e.g. at the bottom here for a previous run:

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.

Copy link
Collaborator

@Saviq Saviq left a 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:

  1. top-left portion of the app that fits at the scale in the minimum resolution
  2. 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.

mir-ci/mir_ci/tests/test_fractional_scale_v1.py Outdated Show resolved Hide resolved
mir-ci/mir_ci/tests/test_fractional_scale_v1.py Outdated Show resolved Hide resolved
@tarek-y-ismail
Copy link
Contributor Author

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?

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 you should be able to reduce the number of templates needed to just two per scale:

I think I'll try this out. Thanks for the suggestion :)

@Saviq
Copy link
Collaborator

Saviq commented Aug 5, 2024

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 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.

@Saviq Saviq force-pushed the test-fractional-scale-v1 branch from 7e491b2 to 04009e2 Compare August 13, 2024 21:05
@Saviq Saviq changed the base branch from main to add-edge August 13, 2024 21:09
@Saviq Saviq force-pushed the test-fractional-scale-v1 branch from 04009e2 to 6713e68 Compare August 13, 2024 21:09
@Saviq Saviq force-pushed the test-fractional-scale-v1 branch 2 times, most recently from b14edfa to 51cdaff Compare August 13, 2024 21:38
@Saviq Saviq force-pushed the test-fractional-scale-v1 branch from 51cdaff to eb5a2be Compare August 13, 2024 21:43
@Saviq Saviq force-pushed the test-fractional-scale-v1 branch from eb5a2be to 3de898a Compare August 13, 2024 22:43
Copy link
Collaborator

@Saviq Saviq left a 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/display_server_static_file.py Outdated Show resolved Hide resolved
mir-ci/mir_ci/tests/robot/platforms/wayland/WaylandHid.py Outdated Show resolved Hide resolved
mir-ci/mir_ci/tests/robot/resources/kvm/kvm.resource Outdated Show resolved Hide resolved
mir-ci/mir_ci/tests/test_display_configuration.py Outdated Show resolved Hide resolved
mir-ci/mir_ci/tests/test_fractional_scale_v1.py Outdated Show resolved Hide resolved
mir-ci/mir_ci/wayland/virtual_pointer.py Outdated Show resolved Hide resolved
mir-ci/mir_ci/wayland/virtual_pointer.py Outdated Show resolved Hide resolved
@Saviq Saviq marked this pull request as ready for review August 14, 2024 09:33
@Saviq Saviq requested a review from a team as a code owner August 14, 2024 09:33
@Saviq Saviq force-pushed the test-fractional-scale-v1 branch from 121c6eb to 4589cfd Compare August 14, 2024 16:41
@Saviq Saviq force-pushed the test-fractional-scale-v1 branch from 1fafc63 to b06e541 Compare August 15, 2024 15:17
Copy link
Collaborator

@Saviq Saviq left a 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/test_fractional_scale_v1.py Outdated Show resolved Hide resolved
mir-ci/mir_ci/tests/display_server_static_file.py Outdated Show resolved Hide resolved
mir-ci/mir_ci/wayland/virtual_pointer.py Show resolved Hide resolved
@Saviq Saviq force-pushed the test-fractional-scale-v1 branch from c6dabfb to 7c7389b Compare August 16, 2024 12:56
Base automatically changed from add-edge to main August 20, 2024 14:46
@tarek-y-ismail
Copy link
Contributor Author

Additional changes reviewed, LGTM! Will just rebase to clean up the commit history then merge.

@tarek-y-ismail tarek-y-ismail force-pushed the test-fractional-scale-v1 branch from 7c7389b to 31dbc8a Compare August 21, 2024 08:07
@tarek-y-ismail tarek-y-ismail added this pull request to the merge queue Aug 21, 2024
Merged via the queue into main with commit 2630b9a Aug 21, 2024
14 of 20 checks passed
@tarek-y-ismail tarek-y-ismail deleted the test-fractional-scale-v1 branch August 21, 2024 13:01
Saviq pushed a commit that referenced this pull request Jan 10, 2025
Saviq pushed a commit that referenced this pull request Jan 10, 2025
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