agronholm / apscheduler

Task scheduling library for Python
MIT License
6.3k stars 712 forks source link

Fix tests #982

Closed HK-Mattew closed 3 weeks ago

HK-Mattew commented 3 weeks ago

Removed failed test that checked if the Scheduler was running on uwsgi with threads disabled. Now in new versions of uwsgi threads are enabled by default.

Reference to uwsgi change: https://github.com/unbit/uwsgi/commit/896ee575d0743e78237c7220007f4bbeaa8cedf8

HK-Mattew commented 3 weeks ago

After I opened the PR, I thought about the possible "fix". Instead of removing test, I could just skip testing for newer versions of Uwsgi.

I didn't think about this before, but if anyone prefers I can try to do it later.

coveralls commented 3 weeks ago

Coverage Status

coverage: 92.006%. first build when pulling 631a406389720f92660b61be2a912ba56f48b9f0 on HK-Mattew:master into a79e0275ba13bd733d974db0db67728c632781bc on agronholm:master.

agronholm commented 3 weeks ago

Perhaps you could just disable threads explicitly? The scheduler should be allowed to run if uwsgi has threads enabled, and fail if it doesn't, regardless of version.

HK-Mattew commented 3 weeks ago

Perhaps you could just disable threads explicitly? The scheduler should be allowed to run if uwsgi has threads enabled, and fail if it doesn't, regardless of version.

I had looked for an option in uwsgi to disable threads. And I believe there is no such option or I did not find it

HK-Mattew commented 3 weeks ago

Perhaps you could just disable threads explicitly? The scheduler should be allowed to run if uwsgi has threads enabled, and fail if it doesn't, regardless of version.

agronholm commented 3 weeks ago

In that case, the uwsgi test dependency should have an upper bound where it avoids any version with thread enablement by default.

HK-Mattew commented 3 weeks ago

Actually, in new versions of uwsgi I don't know yet when the value of has_threads(https://github.com/agronholm/apscheduler/blob/a79e0275ba13bd733d974db0db67728c632781bc/src/apscheduler/_schedulers/sync.py#L395) can be negative.

HK-Mattew commented 3 weeks ago

Sorry, I ended up closing the PR by accident 🤦‍♂️

HK-Mattew commented 3 weeks ago

In that case, the uwsgi test dependency should have an upper bound where it avoids any version with thread enablement by default.

Do you mean, skip this specific test for new versions of uwsgi?

agronholm commented 3 weeks ago

No, I mean install an old version of uwsgi that still disables threading by default.

HK-Mattew commented 3 weeks ago

No, I mean install an old version of uwsgi that still disables threading by default.

Done!

agronholm commented 3 weeks ago

Thanks!