canonical / checkbox

Checkbox
https://checkbox.readthedocs.io
GNU General Public License v3.0
30 stars 44 forks source link

Improvements in EDID cycling with Zapper (BugFix) #1229

Closed kissiel closed 2 weeks ago

kissiel commented 1 month ago

Description

TL;DR: A more reliable way to do EDID cycling with Zapper.

When debugging the Zapperized jobs that tested the changes in resolutions of the connected monitor (as emulated with Zapper), I noticed that the regexes failed to detect the right port on which the Zapper was connected. I also discovered there's plenty of corner cases which would be very hard to cover with just a regex extraction. Thus, I wrote a fully fledged parser for xrandr's and gnome-randr's output. Both parsers are used in the new function I'm proposing here to land in checkbox-support where it would return the information about display modes available on given video output.

This in turn, when plugged into the edid_cycle test script started yielding true results from the job.

Resolved issues

Fixes: https://warthogs.atlassian.net/browse/ZAP-677 Fixes: https://warthogs.atlassian.net/browse/ZAP-678

Tests

I tested the code by running it on multiple devices running both X11 and Wayland. I also propose unit tests with the code.

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 98.11321% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 43.50%. Comparing base (646fe88) to head (89ec219). Report is 6 commits behind head on main.

Files Patch % Lines
...ox-support/checkbox_support/parsers/gnome_randr.py 96.96% 0 Missing and 1 partial :warning:
...heckbox-support/checkbox_support/parsers/xrandr.py 97.14% 0 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1229 +/- ## ========================================== + Coverage 43.36% 43.50% +0.13% ========================================== Files 357 361 +4 Lines 38684 38782 +98 Branches 6560 6576 +16 ========================================== + Hits 16775 16871 +96 Misses 21245 21245 - Partials 664 666 +2 ``` | [Flag](https://app.codecov.io/gh/canonical/checkbox/pull/1229/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=canonical) | Coverage Δ | | |---|---|---| | [checkbox-support](https://app.codecov.io/gh/canonical/checkbox/pull/1229/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=canonical) | `53.12% <97.95%> (+0.90%)` | :arrow_up: | | [provider-base](https://app.codecov.io/gh/canonical/checkbox/pull/1229/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=canonical) | `16.71% <100.00%> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=canonical#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Hook25 commented 2 weeks ago

I'm closing this as it doesn't solve the issue we had and this was solved another way by @p-gentili