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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

0.25.0: Five failing tests #4762

Closed
2 tasks done
dvzrv opened this issue Apr 13, 2024 · 13 comments 路 Fixed by #4766
Closed
2 tasks done

0.25.0: Five failing tests #4762

dvzrv opened this issue Apr 13, 2024 · 13 comments 路 Fixed by #4766

Comments

@dvzrv
Copy link
Contributor

dvzrv commented Apr 13, 2024

Issue description

Hi 馃憢

I'm currently upgrading the package on Arch Linux to 0.25.0 and ran into failing tests:

=========================== short test summary info ============================
FAILED test/backend/x11/test_window.py::test_urgent_hook_fire[wayland-2] - Ke...
FAILED test/widgets/test_clock.py::test_clock_datetime_timezone - AssertionEr...
FAILED test/widgets/test_clock.py::test_clock_pytz_timezone - AssertionError:...
FAILED test/widgets/test_clock.py::test_clock_dateutil_timezone - AssertionEr...
FAILED test/widgets/test_clock.py::test_clock_change_timezones - AssertionErr...
= 5 failed, 1726 passed, 71 skipped, 4 xpassed, 2 warnings in 1017.94s (0:16:57) =

qtile-0.25.0-1-x86_64-check.log
qtile-0.25.0-1-x86_64-build.log

In regards to how tests are run, nothing has been changed since last upgrade:
https://gitlab.archlinux.org/archlinux/packaging/packages/qtile/-/blob/c9582a5362b017c90a6f3c06b57003dd1326901b/PKGBUILD

Version

0.25.0

Backend

X11 (default), Wayland (experimental)

Config

No response

Logs

No response

Required

  • I have searched past issues to see if this bug has already been reported, and it hasn't been.
  • I understand that people give their precious time for free, and thus I've done my very best to make this problem as easy as possible to investigate.
@tych0
Copy link
Member

tych0 commented Apr 13, 2024

Hmm, first one looks a little strange to me, can you try the above PR and see if that fixes it? The rest all look like "the same" bug, I will take a look at those in a bit.

@tych0
Copy link
Member

tych0 commented Apr 13, 2024

@elParaguayo I have spent a while staring at this, and I don't think I see why it fails. I expect that something about the arch packaging environment is causing this weirdness, but it looks to me like you've monkeypatched most (all?) of that.

@elParaguayo
Copy link
Member

Fair enough. Let me take a look.

@tych0
Copy link
Member

tych0 commented Apr 13, 2024

The math looks like it is pretty clearly ignoring the timezone delta, but I really don't understand why that would happen.

@elParaguayo
Copy link
Member

If I run pkgctl build --testing to build this in a chroot, all those clock tests pass successfully for me.

@jwijenbergh
Copy link
Contributor

jwijenbergh commented Apr 13, 2024

Fwiw the clock tests also fail for me on my machine (NixOS). They are also off with delta

@jwijenbergh
Copy link
Contributor

jwijenbergh commented Apr 14, 2024

tests worked for me before 13e9ac8

@elParaguayo
Copy link
Member

Yes. There's no doubt that that's the cause bit y weird that it's passing for me and on CI.

Will take a look later.

jwijenbergh added a commit to jwijenbergh/qtile that referenced this issue Apr 14, 2024
This was previously removed, and now this test seems to depend on the
environment it is being ran (?). This adds back the UTC offset of the
timezone such that the delta is taken into account. Fixes qtile#4762
jwijenbergh added a commit to jwijenbergh/qtile that referenced this issue Apr 14, 2024
This was previously removed, and now this test seems to depend on the
environment it is being ran (?). This adds back the UTC offset of the
timezone such that the delta is taken into account. Fixes qtile#4762
jwijenbergh added a commit to jwijenbergh/qtile that referenced this issue Apr 14, 2024
This was previously removed, and now this test seems to depend on the
environment it is being ran (?). This adds back the UTC offset of the
timezone such that the delta is taken into account. Fixes qtile#4762
@jwijenbergh
Copy link
Contributor

Give #4766 a shot

jwijenbergh added a commit to jwijenbergh/qtile that referenced this issue Apr 14, 2024
This was previously removed, and now this test seems to depend on the
environment it is being ran (?). This adds back the UTC offset of the
timezone such that the delta is taken into account. Fixes qtile#4762
jwijenbergh added a commit that referenced this issue Apr 14, 2024
This was previously removed, and now this test seems to depend on the
environment it is being ran (?). This adds back the UTC offset of the
timezone such that the delta is taken into account. Fixes #4762
@elParaguayo
Copy link
Member

@jwijenbergh did the first test pass for you? That's not related to the Clock widget.

@jwijenbergh
Copy link
Contributor

jwijenbergh commented Apr 14, 2024

Sorry GitHub auto closed this, the first one might be an issue yeah

@jwijenbergh jwijenbergh reopened this Apr 14, 2024
tych0 added a commit to tych0/qtile that referenced this issue Apr 21, 2024
We were previously using `manager_nospawn`, which is a fixture that enables
the test for both backends, even though this test is x11 specific. You can
see this with the failed test name+params here:

    FAILED test/backend/x11/test_window.py::test_urgent_hook_fire[wayland-2]

instead, let's introduce our own xmanager_nospawn, and use that.

This is the other half of qtile#4762

Signed-off-by: Tycho Andersen <tycho@tycho.pizza>
@tych0
Copy link
Member

tych0 commented Apr 21, 2024

@dvzrv can you try #4775 ? I am pretty sure that will fix it.

tych0 added a commit to tych0/qtile that referenced this issue Apr 21, 2024
We were previously using `manager_nospawn`, which is a fixture that enables
the test for both backends, even though this test is x11 specific. You can
see this with the failed test name+params here:

    FAILED test/backend/x11/test_window.py::test_urgent_hook_fire[wayland-2]

instead, let's introduce our own xmanager_nospawn, and use that.

This is the other half of qtile#4762

Signed-off-by: Tycho Andersen <tycho@tycho.pizza>
elParaguayo pushed a commit that referenced this issue May 2, 2024
We were previously using `manager_nospawn`, which is a fixture that enables
the test for both backends, even though this test is x11 specific. You can
see this with the failed test name+params here:

    FAILED test/backend/x11/test_window.py::test_urgent_hook_fire[wayland-2]

instead, let's introduce our own xmanager_nospawn, and use that.

This is the other half of #4762

Signed-off-by: Tycho Andersen <tycho@tycho.pizza>
@elParaguayo
Copy link
Member

I think these are all resolved now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants