Unplug the Zapper monitor at the end of the EDID test (BugFix)

p-gentili commented 3 months ago

Description

The Zapper EDID job leaves the "monitor" connected at the end of the test, making tests like "resolution_after_suspend" fail. This PR introduces a simple context manager to clear the environment at the end of the test.

Resolved issues

Resolves CHECKBOX-1436

Documentation

Tests are included and functions documented.

Tests

Locally, side-loading the provider and running in sequence

DISPLAY=:0 xrandr -q | grep "[*]" | awk '{{print $1}}'
DISPLAY=:0 ZAPPER_HOST=<IP> checkbox.checkbox-cli run com.canonical.certification::monitor/zapper-edid
DISPLAY=:0 xrandr -q | grep "[*]" | awk '{{print $1}}'

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 43.77%. Comparing base (60578f6) to head (ca4d677). Report is 115 commits behind head on main.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #1269 +/- ## ========================================== + Coverage 43.76% 43.77% +0.01% ========================================== Files 358 358 Lines 38734 38741 +7 Branches 6565 6566 +1 ========================================== + Hits 16951 16958 +7 Misses 21118 21118 Partials 665 665 ``` | [Flag](https://app.codecov.io/gh/canonical/checkbox/pull/1269/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=canonical) | Coverage Δ | | |---|---|---| | [provider-base](https://app.codecov.io/gh/canonical/checkbox/pull/1269/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=canonical) | `17.84% <100.00%> (+0.03%)` | :arrow_up: | 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.

p-gentili commented 3 months ago

LGTM not merging as I have a question: is this the only reason why the "cycle" is not a cycle? Can't it happen that we unintentionally change the resolution and never put it back to the original value? That is what the diff was reporting in my imagination

Three things to keep in mind:

the idle state of Zapper monitor is disconnected
the other jobs, like resolution_after_suspend are not Zapper-aware. It means that the Zapper jobs should clean the environment when they exit (similarly the USB hot-plugging job VS usb-detect job)
this job is not modifying the resolution of the internal monitor (for now at least)

So the cycle is OFF -> resolution 1 -> res. 2 -> res. 3 and, with this, -> OFF.

For instance, the device linked in the Jira card was reporting before suspend only a FHD monitor and, after suspend, two FHD monitors.

Hook25 commented 3 months ago

ty for the explanation

canonical / checkbox