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

Update compare_resolution function (BugFix) #1683

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mckees
Copy link
Contributor

@mckees mckees commented Jan 16, 2025

Description

The current compare_resolution function uses deprecated functions, and it appears that any solution using Gdk with Wayland will fail to locate a primary monitor (https://gitlab.gnome.org/GNOME/gtk/-/issues/1028). Additionally, this change adds a hint if an error arises

Resolved issues

https://bugs.launchpad.net/kobuk/+bug/2083892

Documentation

Tests

I tested the function on its own locally, but @LiaoU3 will run further tests before merging

@LiaoU3
Copy link
Contributor

LiaoU3 commented Jan 16, 2025

There is no tkinter module in checkbox, hence, for snap you could add python3-tk here (Don't forget to do this on all series), and for debian you could add python3-tk here

-----------------------------[ Running job 2 / 2 ]------------------------------
[ Test that Intel Corporation Alder Lake-N [UHD Graphics] meets minimum resolution requirement ]
ID: com.canonical.certification::graphics/1_minimum_resolution_Alder_Lake-N__UHD_Graphics_
Category: Graphics tests
--------------------------------------------------------------------------------
Traceback (most recent call last):
  File "/tmp/nest-7yg8i7lh.9ba92f19ab6b2ddd1fb5e91797dde60f8915295419d34e1f9cc51f5d2f9be579/resolution_test.py", line 5, in <module>
    import tkinter as tk
ModuleNotFoundError: No module named 'tkinter'
--------------------------------------------------------------------------------
Outcome: job failed

@mckees mckees force-pushed the compare_resolution branch 2 times, most recently from e67e952 to 67b21d9 Compare January 16, 2025 03:51
The current compare_resolution function uses deprecated functions, and
it appears that any solution using Gdk with Wayland will fail to locate
a primary monitor (https://gitlab.gnome.org/GNOME/gtk/-/issues/1028).
Additionally, this change adds a hint if an error arises
@mckees mckees force-pushed the compare_resolution branch from 67b21d9 to 7d62796 Compare January 16, 2025 04:06
Copy link

codecov bot commented Jan 16, 2025

Codecov Report

Attention: Patch coverage is 0% with 10 lines in your changes missing coverage. Please review.

Project coverage is 48.99%. Comparing base (9253900) to head (7d62796).

Files with missing lines Patch % Lines
providers/base/bin/resolution_test.py 0.00% 10 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1683      +/-   ##
==========================================
- Coverage   48.99%   48.99%   -0.01%     
==========================================
  Files         372      372              
  Lines       40321    40327       +6     
  Branches     6811     6811              
==========================================
  Hits        19757    19757              
- Misses      19842    19848       +6     
  Partials      722      722              
Flag Coverage Δ
provider-base 24.79% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@LiaoU3
Copy link
Contributor

LiaoU3 commented Jan 16, 2025

You may also need to add a unittest file for this resolution_test.py like other unittest under this folder

@LiaoU3
Copy link
Contributor

LiaoU3 commented Jan 16, 2025

submissions

@mckees Is the failed message as you expected? Since it looks like somehow it can still create a window, but the resolution is 0*0

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