neovim / pynvim

Python client and plugin host for Nvim
http://pynvim.readthedocs.io/en/latest/
Apache License 2.0
1.54k stars 120 forks source link

Fix broken tests on Windows due to EOF error and other minor bugs #543

Closed wookayin closed 1 year ago

wookayin commented 1 year ago

🎉✅ TESTS on WINDOWS ARE NOW ALL GREEN ✅🎉

Note: This PR consists of several commits and bugfixes, each of which is all required to fix the broken tests on windows platform for such a long time. I would like to request @justinmk that please merge this PR without squashing (either merge commit or rebasing), retaining all the individual commits.

fix: EOF error on piped stderr being closed on Windows

Most importantly, this PR should fix #505. Pipe (subprocess) based pynvim was broken since neovim 0.5.0, probably due to https://github.com/neovim/neovim/commit/c86d5fa981b0651167f789aaff685b42cad7aaca (neovim/neovim#11390): on Windows, stderr is closed for an embedded nvim.

See the commit message for more details.

fix: do not leak resources across tests so as to prevent side effects

See the commit message for more details. asyncio event loops were not closing properly during tests, and were leaking in subsequent EventLoop, Session, Nvim instances.

wookayin commented 1 year ago

I dropped one commit (fix: monkey-patch _ProactorBasePipeTransport to suppress warning) from this PR. Tests are all passing anyway (but with many warnings).

The warnings on _ProactorBasePipeTransport.__del__, despite "fix: do not leak resources across tests so as to prevent side effects", means that there are still leaking and unclosed transports. Indeed, there are a few places where transports are leaking, which I already fixed in my working branches with a major refactoring on AsyncioEventLoop but didn't include in this PR because I didn't want to make this PR too big to review easily. (will follow after this is merged) -> #545