Update Test Dependencies versions

[harshil21]

Updates to our test suite:
black 22.3.0: out of beta
pytest 7.1.2: major release
mypy 0.950: fixes and improvements
pyupgrade 2.31.0: drops py 3.6 support + bug fixes (can't find a changelog)
pylint 2.13.7: major release

Note about mypy:

• There are many ignores in Enums because of Overriding non-enumeration attributes in Enum subclass fails python/mypy#12132 . Edit: Resolved in mypy 0.942.

Note about pytest.approx:

• datetimes are not supported by approx (datetime and timedelta suport for approx pytest-dev/pytest#8395), so we should not use them.

TODO:
Update furo dependency. Lots of new features added. Including edit on github button.
Completed in #2969

• Add --enable=useless-suppression to pylint args. This will warn about unused pylint ignore comments
• Update pylint to new version 2.13.7
• Update mypy again to resolve above issue
• In c3eda6f / Updating the examples #2937 we added the hard pinned click dependency for black in requirements-dev.txt and .pre-commit-config.yaml, revert this here when upgrading black to => 22.3.0
• update pytest-asyncio and use the new functionality of auto-detecting async tests to remove the @pytest.mark.asyncio. Since some of the monkeypatched tests block with that, they'll have to be adjusted - see @a3f15f9
• update sphinx to 4.5.0
• migrate to pre-commit.ci for pre-commit tests
• maybe add isort to the pre-commit setup

[github-actions]

Hey there. Relax, I am just a little warning for the maintainers to release directly after merging your PR, otherwise we have broken examples and people might get confused :)

[github-actions]

Hey! Looks like you edited the (dev) requirements or the pre-commit hooks. I'm just a friendly reminder to keep the pre-commit hook versions in sync with the dev requirements and the additional dependencies for the hooks in sync with the requirements :)

[harshil21]

I had to add a disable import-error for pylint in setup.cfg since for some reason, pylint complains about that in all of the examples when running via git hook. But when I run using pre-commit run -a, & pylint examples/ pylint does not having any trouble importing...

Also examples run just fine without any import error when running manually

[Bibo-Joshi]

I had to add a disable import-error for pylint in setup.cfg since for some reason, pylint complains about that in all of the examples when running via git hook. But when I run using pre-commit run -a, & pylint examples/ pylint does not having any trouble importing...

Also examples run just fine without any import error when running manually

I guess the main issue here is that the examples directory is located as a sibling to the tg directory so running python examples/echobot.py without having PTB installed should indeed throw errors. IIRC I worked around that issue previously by adding - . as additional dependency in pre-commit which basically does an editable install

[Bibo-Joshi]

Can you check if this is still necessary even after changing the bogous monkeypatch?

[harshil21]

you mean the call_after function? I'm not sure of what the relation/issue was with monkeypatching, was it due to the event loop being closed?

[Bibo-Joshi]

ah sorry, I just saw that you change a monkeypatch in one of the commits but seems to have been reverted. The issue I had before coming up with call_after was that I was patching some functions that were required do run for proper init/shutdown. It's possible using a different loop policy resolves that, but I'm unsure whether this is treating cause or just the symptoms.
At first glance, I think would prefer to fiddle with the loop as little as possible, both to keep the test suite maintainable and to test on the default setting on windows. If using call_after wherever suitable does not solve the issue, I'm of course fine with doing this.

[harshil21]

yeah, unfortunately call_after seems to only treat the symptoms. This error would happen on many tests, irrespective of the monkeypatches. Since this is a bug in cpython, maybe it would be beneficial to switch the event loop policy on windows to the selectoreventloop in our actual code? httpx might raise a warning in future versions too (encode/httpx#914 (comment) ).

Another question is why this only showed up after upgrading pytest-asyncio, is because after >0.17.0, they no longer alter the event loop policy (https://github.com/pytest-dev/pytest-asyncio/blob/master/CHANGELOG.rst#0170-22-01-13). So in a way, the default setting on windows wasn't really tested until now :)

[Bibo-Joshi]

yeah, unfortunately call_after seems to only treat the symptoms. This error would happen on many tests, irrespective of the monkeypatches.

that's what I'm unsure about, as

• IIRC call_after had fixed the issues for me when I tried upgrading pytest-asyncio without changing any tests outside of test_application
• This mwe Windows - Python 3.8+ raises "RuntimeError: Event Loop is closed" on exit. encode/httpx#914 (comment) works fine for me in the default setting with the WindowsProactorEventLoopPolicy

I can try to have a closer look tonight

Since this is a bug in cpython, maybe it would be beneficial to switch the event loop policy on windows to the selectoreventloop in our actual code? httpx might raise a warning in future versions too (encode/httpx#914 (comment) ).

We can keep that in mind as an option, but I think trying something like that is a bit premature before getting some user feedback on v20.

Another question is why this only showed up after upgrading pytest-asyncio, is because after >0.17.0, they no longer alter the event loop policy (https://github.com/pytest-dev/pytest-asyncio/blob/master/CHANGELOG.rst#0170-22-01-13). So in a way, the default setting on windows wasn't really tested until now :)

Ah, I didn't know that. good finding 👍

[Bibo-Joshi]

Hmm but still since there's no proper traceback for this it's hard to know exactly how many tests are causing this error, and when exactly this is happening,,

I ended up doing a bisection in terms of commenting out tests to see which one cause that behavior.

adding a await bot.shutdown() fixes it..

jup, that's exactly what I used call_after for: make sure that bot.shutdown is actually called instead of just overriding it

[harshil21]

I ended up doing a bisection in terms of commenting out tests to see which one cause that behavior.

ha I had done the same thing

jup, that's exactly what I used call_after for: make sure that bot.shutdown is actually called instead of just overriding it

but that is treating the symptom isn't it? I say to leave the event loop policy mod here, anyway tests run fine on win 3.7.. so

[Bibo-Joshi]

but that is treating the symptom isn't it?

I tend to disagree. since calling bot.shutdown fixes the issue, this sounds to me like not properly closing the httpx.AsyncClient might the be issue. different event loops handling that better or worse is another thing, but ultimately it's better to do a clean shutdown

[Bibo-Joshi]

I did some digging and the problem seems to be indeed more deeply rooted than just some missing bot.shutdown calls. My observations/tries:

• the behavior is not deterministic. the tests sometimes fail, sometime they don't
• removing our workaround that makes the event_loop session scoped (thus using a fresh event loop for every test) didn't help
• I have a suspicion that there is an issue with pytest.mark.parametrize - at least there was one parametrized test that failed more often than when I removed the parametrization
• I inserted some prints into HTTPXReq.init/shutdown. on parametrized tests, the prints where sometimes mixed up, e.g. init - init - shutdown - shutdown instead of init-shutdown-init-shutdown. Also sometimes the shutdown prints where after the Event loop closed error

I'm okay with keeping the workaround as is, but I'd vote to at least open an issue that gathers some details, links to the cpython issue etc

[harshil21]

• the behavior is not deterministic. the tests sometimes fail, sometime they don't

I noticed that if you run just one test, that test won't fail, but you'll get the warning in the logs. However if you run two or more tests (where one or more causes the error), then it causes the last test to fail. But yes, it is hard to find the root cause, because if bot.shutdown() had some code which causes a problem, then running test_request should fail but it doesn't?

• I have a suspicion that there is an issue with pytest.mark.parametrize - at least there was one parametrized test that failed more often than when I removed the parametrization

mostly related to what I said above (running two or more tests will fail the test suite)

I'm okay with keeping the workaround as is, but I'd vote to at least open an issue that gathers some details, links to the cpython issue etc

Yeah, there is a small chance users can bump into this issue, specially since we still can't find the real reason why.