-
Notifications
You must be signed in to change notification settings - Fork 52
add a simple test to run #134
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
Conversation
@hosaka Please approve CI for this... thanks! Passing on my mac but not sure if on other platforms. |
@TheGreatCabbage since you're in this org wondering if you could approve me to run the workflows. @hosaka -- sorry, meant rerun not approve in the last comment. |
Your last assert is effectively never executed in a test run, you're just ignoring the the import asyncio
import os
import pytest
import qasync
def test_run_with_contextmanager(application):
async def coro():
event_loop = asyncio.get_event_loop()
assert (
type(event_loop).__name__ == "QIOCPEventLoop"
if os.name == "nt"
else "QSelectorEventLoop"
)
await asyncio.sleep(0)
asyncio.set_event_loop(None)
qasync.run(coro())
with pytest.raises(RuntimeError):
_ = asyncio.get_event_loop() |
@johnzhou721 Thanks for the fixes! I took the liberty to tidy up the changes. I am going to move the *.run tests into macos tests are flaky because of a subprocess timeout I think... |
@hosaka I don't kow, I don't have a Windows machine. Are you suggesting the macOS tests are failing becasue of this or not? Also is it true that you can't catch the access violation because you're somehow accessing None in asyncio or something... an error is expected there, we just had to catch it... not sure what I can do about this. Since the macOS stuff is failing because of probably unrelated versions, can I please get a rerun before I dig into the Windwos stuff? |
MacOs tests failure are unreleated and should be fixed elsewhere, by the looks of it, the failing windows test is also a different quirk. Let me know if you're happy with my changes and i'll merge this. |
LGTM! Let us merge. Guess we got really unlucky... 3 stuff failing at the same time. |
No description provided.