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

Fix tests #982

Merged
merged 1 commit into from
Oct 29, 2024
Merged

Fix tests #982

merged 1 commit into from
Oct 29, 2024

Conversation

HK-Mattew
Copy link
Contributor

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: unbit/uwsgi@896ee57

@HK-Mattew
Copy link
Contributor Author

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
Copy link

coveralls commented Oct 29, 2024

Coverage Status

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

@agronholm
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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 HK-Mattew closed this Oct 29, 2024
@HK-Mattew HK-Mattew reopened this Oct 29, 2024
@agronholm
Copy link
Owner

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

@HK-Mattew
Copy link
Contributor Author

Actually, in new versions of uwsgi I don't know yet when the value of has_threads(

if not getattr(uwsgi_module, "has_threads", True):
) can be negative.

@HK-Mattew
Copy link
Contributor Author

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

@HK-Mattew
Copy link
Contributor Author

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
Copy link
Owner

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

@HK-Mattew
Copy link
Contributor Author

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

Done!

@agronholm agronholm merged commit 8660590 into agronholm:master Oct 29, 2024
14 checks passed
@agronholm
Copy link
Owner

Thanks!

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.

3 participants