Merge #543 from wookayin/fix-windows

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

Author: justinmk

.github/workflows/test.yml

@@ -27,37 +27,31 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        python-version: ['3.10', '3.11', '3.12-dev']
+        python-version: ['3.12', '3.11', '3.10', '3.9', '3.8', '3.7']
         os: ['ubuntu-latest', 'macos-latest', 'windows-latest']
+        exclude:
+          - os: 'ubuntu-latest'
+            python-version: '3.7'
         include:
-        - os: ubuntu-20.04
-          python-version: '3.7'
-          NIGHTLY: nvim-linux64.tar.gz
-          NVIM_BIN_PATH: nvim-linux64/bin
-          EXTRACT: tar xzf
-        - os: ubuntu-latest
-          python-version: '3.8'
-          NIGHTLY: nvim-linux64.tar.gz
-          NVIM_BIN_PATH: nvim-linux64/bin
-          EXTRACT: tar xzf
-        - os: ubuntu-latest
-          python-version: '3.9'
-          NIGHTLY: nvim-linux64.tar.gz
-          NVIM_BIN_PATH: nvim-linux64/bin
-          EXTRACT: tar xzf
-        - os: ubuntu-latest
-          NIGHTLY: nvim-linux64.tar.gz
-          NVIM_BIN_PATH: nvim-linux64/bin
-          EXTRACT: tar xzf
-        - os: macos-latest
-          NIGHTLY: nvim-macos.tar.gz
-          NVIM_BIN_PATH: nvim-macos/bin
-          EXTRACT: tar xzf
-        - os: windows-latest
-          NIGHTLY: nvim-win64.zip
-          NVIM_BIN_PATH: nvim-win64/bin
-          EXTRACT: unzip
+          - os: 'ubuntu-20.04'
+            python-version: '3.7'
+            NIGHTLY: nvim-linux64.tar.gz
+            NVIM_BIN_PATH: nvim-linux64/bin
+            EXTRACT: tar xzf
+          - os: 'ubuntu-latest'
+            NIGHTLY: nvim-linux64.tar.gz
+            NVIM_BIN_PATH: nvim-linux64/bin
+            EXTRACT: tar xzf
+          - os: 'macos-latest'
+            NIGHTLY: nvim-macos.tar.gz
+            NVIM_BIN_PATH: nvim-macos/bin
+            EXTRACT: tar xzf
+          - os: 'windows-latest'
+            NIGHTLY: nvim-win64.zip
+            NVIM_BIN_PATH: nvim-win64/bin
+            EXTRACT: unzip
 
+    name: "test (python ${{ matrix.python-version }}, ${{ matrix.os }})"
     runs-on: ${{ matrix.os }}
     steps:
     - uses: actions/checkout@v3
@@ -66,12 +60,6 @@ jobs:
         cache: 'pip'
         python-version: ${{ matrix.python-version }}
 
-    - name: install neovim
-      run: |
-        curl -LO 'https://github.com/neovim/neovim/releases/download/nightly/${{ matrix.NIGHTLY }}'
-        ${{ matrix.EXTRACT }} ${{ matrix.NIGHTLY }}
-        echo '${{ runner.os }}'
-
     - name: update path (bash)
       if: runner.os != 'Windows'
       run: echo "$(pwd)/${{ matrix.NVIM_BIN_PATH }}" >> $GITHUB_PATH
@@ -80,16 +68,27 @@ jobs:
       if: runner.os == 'Windows'
       run: echo "$(pwd)/${{ matrix.NVIM_BIN_PATH }}" | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append
 
+    - name: install neovim
+      run: |
+        curl -LO 'https://github.com/neovim/neovim/releases/download/nightly/${{ matrix.NIGHTLY }}'
+        ${{ matrix.EXTRACT }} ${{ matrix.NIGHTLY }}
+        echo '${{ runner.os }}'
+        nvim --version
+
     - name: install dependencies
       run: |
         python3 -m pip install -U pip
         python3 -m pip install tox tox-gh-actions
 
+    - name: check neovim
+      run: |
+        python3 -m pip install -e .   # install pynvim
+        nvim --headless --clean -c 'checkhealth | %+print | q'
+
     - name: test with tox
       run: |
         echo $PATH
         which nvim
-        nvim --version
         which -a python3
         python3 --version
         tox run

pynvim/msgpack_rpc/event_loop/asyncio.py

@@ -14,7 +14,7 @@
 import sys
 from collections import deque
 from signal import Signals
-from typing import Any, Callable, Deque, List
+from typing import Any, Callable, Deque, List, Optional
 
 from pynvim.msgpack_rpc.event_loop.base import BaseEventLoop
 
@@ -37,6 +37,8 @@ class AsyncioEventLoop(BaseEventLoop, asyncio.Protocol,
     """`BaseEventLoop` subclass that uses `asyncio` as a backend."""
 
     _queued_data: Deque[bytes]
+    if os.name != 'nt':
+        _child_watcher: Optional['asyncio.AbstractChildWatcher']
 
     def connection_made(self, transport):
         """Used to signal `asyncio.Protocol` of a successful connection."""
@@ -58,12 +60,17 @@ def data_received(self, data: bytes) -> None:
 
     def pipe_connection_lost(self, fd, exc):
         """Used to signal `asyncio.SubprocessProtocol` of a lost connection."""
+        debug("pipe_connection_lost: fd = %s, exc = %s", fd, exc)
+        if os.name == 'nt' and fd == 2:  # stderr
+            # On windows, ignore piped stderr being closed immediately (#505)
+            return
         self._on_error(exc.args[0] if exc else 'EOF')
 
     def pipe_data_received(self, fd, data):
         """Used to signal `asyncio.SubprocessProtocol` of incoming data."""
         if fd == 2:  # stderr fd number
-            self._on_stderr(data)
+            # Ignore stderr message, log only for debugging
+            debug("stderr: %s", str(data))
         elif self._on_data:
             self._on_data(data)
         else:
@@ -78,6 +85,7 @@ def _init(self) -> None:
         self._queued_data = deque()
         self._fact = lambda: self
         self._raw_transport = None
+        self._child_watcher = None
 
     def _connect_tcp(self, address: str, port: int) -> None:
         coroutine = self._loop.create_connection(self._fact, address, port)
@@ -145,6 +153,9 @@ def _close(self) -> None:
         if self._raw_transport is not None:
             self._raw_transport.close()
         self._loop.close()
+        if self._child_watcher is not None:
+            self._child_watcher.close()
+            self._child_watcher = None
 
     def _threadsafe_call(self, fn: Callable[[], Any]) -> None:
         self._loop.call_soon_threadsafe(fn)

test/conftest.py

@@ -1,5 +1,9 @@
+"""Configs for pytest."""
+
+import gc
 import json
 import os
+from typing import Generator
 
 import pytest
 
@@ -9,7 +13,10 @@
 
 
 @pytest.fixture
-def vim() -> pynvim.Nvim:
+def vim() -> Generator[pynvim.Nvim, None, None]:
+    """Create an embedded, sub-process Nvim fixture instance."""
+    editor: pynvim.Nvim
+
     child_argv = os.environ.get('NVIM_CHILD_ARGV')
     listen_address = os.environ.get('NVIM_LISTEN_ADDRESS')
     if child_argv is None and listen_address is None:
@@ -28,4 +35,13 @@ def vim() -> pynvim.Nvim:
         assert listen_address is not None and listen_address != ''
         editor = pynvim.attach('socket', path=listen_address)
 
-    return editor
+    try:
+        yield editor
+
+    finally:
+        # Ensure all internal resources (pipes, transports, etc.) are always
+        # closed properly. Otherwise, during GC finalizers (__del__) will raise
+        # "Event loop is closed" error.
+        editor.close()
+
+        gc.collect()  # force-run GC, to early-detect potential leakages

test/test_host.py

@@ -11,15 +11,19 @@
 
 def test_host_imports(vim):
     h = ScriptHost(vim)
-    assert h.module.__dict__['vim']
-    assert h.module.__dict__['vim'] == h.legacy_vim
-    assert h.module.__dict__['sys']
+    try:
+        assert h.module.__dict__['vim']
+        assert h.module.__dict__['vim'] == h.legacy_vim
+        assert h.module.__dict__['sys']
+    finally:
+        h.teardown()
 
 
 def test_host_import_rplugin_modules(vim):
     # Test whether a Host can load and import rplugins (#461).
     # See also $VIMRUNTIME/autoload/provider/pythonx.vim.
     h = Host(vim)
+
     plugins: Sequence[str] = [  # plugin paths like real rplugins
         os.path.join(__PATH__, "./fixtures/simple_plugin/rplugin/python3/simple_nvim.py"),
         os.path.join(__PATH__, "./fixtures/module_plugin/rplugin/python3/mymodule/"),
@@ -56,7 +60,10 @@ def test_host_async_error(vim):
 
 def test_legacy_vim_eval(vim):
     h = ScriptHost(vim)
-    assert h.legacy_vim.eval('1') == '1'
-    assert h.legacy_vim.eval('v:null') is None
-    assert h.legacy_vim.eval('v:true') is True
-    assert h.legacy_vim.eval('v:false') is False
+    try:
+        assert h.legacy_vim.eval('1') == '1'
+        assert h.legacy_vim.eval('v:null') is None
+        assert h.legacy_vim.eval('v:true') is True
+        assert h.legacy_vim.eval('v:false') is False
+    finally:
+        h.teardown()

test/test_vim.py

@@ -1,4 +1,3 @@
-# -*- coding: utf-8 -*-
 import os
 import tempfile
 from typing import Any
@@ -31,7 +30,10 @@ def test_command(vim: Nvim) -> None:
     assert os.path.isfile(fname)
     with open(fname) as f:
         assert f.read() == 'testing\npython\napi\n'
-    os.unlink(fname)
+    try:
+        os.unlink(fname)
+    except OSError:
+        pass  # on windows, this can be flaky; ignore it
 
 
 def test_command_output(vim: Nvim) -> None:

tox.ini

@@ -29,8 +29,10 @@ deps =
 # setenv =
 #   cov: PYTEST_ADDOPTS=--cov=. {env:PYTEST_ADDOPTS:}
 # passenv = PYTEST_ADDOPTS
+
+# Note: Use python instead of python3 due to tox-dev/tox#2801
 commands =
-  python3 -m pytest --color yes -s -vv {posargs}
+  python -m pytest --color yes -s --timeout 5 -vv {posargs}
 
 [testenv:checkqa]
 deps =