Repository: aio-libs/aiohttp-devtools

Issue:
Cleanup context is not run on reload on Windows
* **aiohttp-devtools version:** 1.0.post0
* **aiohttp version:** 3.8.4
* **python version:** 3.11.3
* **Platform:** Windows 10

I have discovered two cases in which the second half of a [Cleanup Context](https://docs.aiohttp.org/en/stable/web_advanced.html#cleanup-context) after the `yield` is not run. The first is in #547, and the second is in this issue.

The documentation says:

> *aiohttp* guarantees that *cleanup code* is called if and only if *startup code* was successfully finished.

I have this simple code, which just has one `on_startup`, one `on_shutdown`, and one `cleanup_ctx` handler.

```python3
from aiohttp import web

routes = web.RouteTableDef()

@routes.get('/')
async def get_events(_request):
    return web.Response(text="Hello, world")

app = web.Application()
async def startup(_app):
    print("====> STARTUP", flush=True)
app.on_startup.append(startup)
async def context(_app):
    print("====> BEFORE", flush=True)
    yield
    print("=====> AFTER", flush=True)
    with open('test.log','a') as fh: fh.write("Hello\n")
app.cleanup_ctx.append(context)
async def shutdown(_app):
    print("=====> SHUTDOWN", flush=True)
app.on_shutdown.append(shutdown)
app.add_routes(routes)

if __name__ == '__main__':
    web.run_app(app)
```

On Linux, when I run the server using `adev runserver --no-livereload testserv.py`, and then edit some whitespace in the file and save it, the output is as follows:

```
[19:03:10] Starting aux server at http://localhost:8001 ◆
[19:03:10] Starting dev server at http://localhost:8000 ●
======== Running on http://0.0.0.0:8001 ========
(Press CTRL+C to quit)
====> BEFORE
====> STARTUP
=====> SHUTDOWN
=====> AFTER
[19:03:16] Restarting dev server at http://localhost:8000 ●
====> BEFORE
====> STARTUP
```

This is what I would expect. However, on Windows 10, the output is as follows:

```
[21:04:01] Starting aux server at http://localhost:8001 ◆
[21:04:01] Starting dev server at http://localhost:8000 ●
======== Running on http://0.0.0.0:8001 ========
(Press CTRL+C to quit)
====> BEFORE
====> STARTUP
[21:04:06] Restarting dev server at http://localhost:8000 ●
====> BEFORE
====> STARTUP
```

Like in issue #547, the second half of the cleanup context is never run. Also, the shutdown handler is not run either.

Discussion:
codecov[bot]: ## [Codecov](https://codecov.io/gh/aio-libs/aiohttp-devtools/pull/549?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aio-libs) Report
> Merging [#549](https://codecov.io/gh/aio-libs/aiohttp-devtools/pull/549?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aio-libs) (106ed21) into [master](https://codecov.io/gh/aio-libs/aiohttp-devtools/commit/7ecebe3d8cc2d28664111c3ae942cf8fa6586224?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aio-libs) (7ecebe3) will **decrease** coverage by `2.14%`.
> The diff coverage is `47.05%`.

```diff
@@            Coverage Diff             @@
##           master     #549      +/-   ##
==========================================
- Coverage   86.33%   84.20%   -2.14%     
==========================================
  Files          12       12              
  Lines         754      785      +31     
  Branches      124      106      -18     
==========================================
+ Hits          651      661      +10     
- Misses         81       97      +16     
- Partials       22       27       +5     
```

| Flag | Coverage Δ | |
|---|---|---|
| unit | `84.20% <47.05%> (-1.88%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aio-libs#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/aio-libs/aiohttp-devtools/pull/549?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aio-libs) | Coverage Δ | |
|---|---|---|
| [aiohttp\_devtools/runserver/serve.py](https://codecov.io/gh/aio-libs/aiohttp-devtools/pull/549?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aio-libs#diff-YWlvaHR0cF9kZXZ0b29scy9ydW5zZXJ2ZXIvc2VydmUucHk=) | `77.16% <0.00%> (-2.51%)` | :arrow_down: |
| [aiohttp\_devtools/runserver/watch.py](https://codecov.io/gh/aio-libs/aiohttp-devtools/pull/549?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aio-libs#diff-YWlvaHR0cF9kZXZ0b29scy9ydW5zZXJ2ZXIvd2F0Y2gucHk=) | `78.83% <58.33%> (-3.93%)` | :arrow_down: |
| [aiohttp\_devtools/runserver/config.py](https://codecov.io/gh/aio-libs/aiohttp-devtools/pull/549?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aio-libs#diff-YWlvaHR0cF9kZXZ0b29scy9ydW5zZXJ2ZXIvY29uZmlnLnB5) | `76.27% <100.00%> (+0.40%)` | :arrow_up: |

... and [2 files with indirect coverage changes](https://codecov.io/gh/aio-libs/aiohttp-devtools/pull/549/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aio-libs)

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aio-libs)

Dreamsorcerer: There's no `AsyncMock` in Python 3.7, so you'll probably have to skip a test on that version.
haukex: > There's no `AsyncMock` in Python 3.7, so you'll probably have to skip a test on that version.

Interesting, I set up a Python 3.7 on Windows and a corresponding venv, and the tests passed - I'll have to investigate on Ubuntu next.
Dreamsorcerer: > > There's no `AsyncMock` in Python 3.7, so you'll probably have to skip a test on that version.
> 
> Interesting, I set up a Python 3.7 on Windows and a corresponding venv, and the tests passed - I'll have to investigate on Ubuntu next.

Weird, I'd just skip and not worry about it. We'll drop support for 3.7 after the next release anyway...
haukex: > > Interesting, I set up a Python 3.7 on Windows and a corresponding venv, and the tests passed - I'll have to investigate on Ubuntu next.
> 
> Weird, I'd just skip and not worry about it. We'll drop support for 3.7 after the next release anyway...

Oh, I see what happened, `test_python_no_server` is the one that failed and that's marked `@non_windows_test`, which is why it worked on my Windows box. I'll try adding a skip on Python <3.8.
haukex: > Codecov says the new code hasn't been run. Do you need to remove a non_windows_test marker?

The coverage report I looked at locally did show that the code that sends and receives the requests is run, but what is missing is coverage of the error case (request is sent but isn't successful in shutting down the server). I'll look into improving the coverage of that next.

haukex: Thanks very much for the full review and sorry for the late reply - at the moment I have a different project with higher priority, so I can't yet say for sure when I will get back to this, but I hope very much that will be the week after next - then I will integrate your comments and work on the code coverage.
haukex: The test in 322ceaaca68593d65f4632c707e0996ecc0fd3e6 works on Linux, but is skipped on Windows due to the `@forked` - that's as far as I was able to get this evening. When I get back to this in a day or two, I'll investigate how to get the test working on Windows.
Dreamsorcerer: Ah, if forked doesn't work on Windows, then maybe it'll be too tricky to test...
We also have the problem that nobody knows how to get pytest-forked working with newer pytest releases. Could really do with an alternative...
haukex: I can always manually fire up the server as a `subprocess`, I was just trying to stick with the way the other tests were written.
haukex: Ok, at least now the GitHub tests pass, though that's still because the new test is being skipped on Windows, so I'll try getting that working next...
haukex: Ok, this is as far as I've gotten with the test (54eced8532ae520fc12c5fe67d196b767964f68f), and unfortunately this doesn't work either. I'm beginning to think that this is really difficult to automatically test on Windows.
Dreamsorcerer: Yeah, we may have to skip it in this case.
haukex: Ok, so now the `pytest` tests all pass on Windows and Linux, Python 3.7 thru 3.11. 

The manual test procedure for the shutdown endpoint in `tests/cleanup_app.py` works on Windows, Python 3.7 thru 3.11.

Obviously the coverage has dropped (apparently -1.95%) because as discussed not everything can be tested automatically.

AFAICT, I'm done with everything on my end, feel free to squash merge, or let me know if there's anything else.
Dreamsorcerer: > Obviously the coverage has dropped (apparently -1.95%) because as discussed not everything can be tested automatically.

Can we test successfully on Linux? i.e. with an explicit `shutdown_by_url=True`? Maybe even patching `os.kill()` so it doesn't kill the app.
haukex: Sorry, I was once again busy with other things, but I will get back to this when I'm able to.

Patch:
diff --git a/aiohttp_devtools/cli.py b/aiohttp_devtools/cli.py
index a87fee0..e09c4d4 100644
--- a/aiohttp_devtools/cli.py
+++ b/aiohttp_devtools/cli.py
@@ -46,6 +46,9 @@ root_help = ('Root directory project used to qualify other paths. '
              'This can be used to watch a parent directory for changes in a more complex application. '
              'env variable: AIO_ROOT')
 static_url_help = 'URL path to serve static files from, default "/static/". env variable: AIO_STATIC_URL'
+shutdown_by_url_help = ("The development server will be stopped via a request to an endpoint that is "
+                        "added to the server, instead of via signals (this is the default on Windows). "
+                        "env variable: AIO_SHUTDOWN_BY_URL")
 host_help = ('host used when referencing livereload and static files, if blank host is taken from the request header '
              'with default of localhost. env variable AIO_HOST')
 app_factory_help = ('name of the app factory to create an aiohttp.web.Application with, if missing default app-factory '
@@ -62,6 +65,8 @@ aux_port_help = 'Port to serve auxiliary app (reload and static) on, default por
 @click.option('-s', '--static', 'static_path', envvar='AIO_STATIC_PATH', type=_dir_existing, help=static_help)
 @click.option('--root', 'root_path', envvar='AIO_ROOT', type=_dir_existing, help=root_help)
 @click.option('--static-url', envvar='AIO_STATIC_URL', help=static_url_help)
+@click.option("--shutdown-by-url/--no-shutdown-by-url", default=sys.platform.startswith("win32"),
+              envvar="AIO_SHUTDOWN_BY_URL", help=shutdown_by_url_help)
 @click.option('--livereload/--no-livereload', envvar='AIO_LIVERELOAD', default=None, help=livereload_help)
 @click.option('--host', default=INFER_HOST, help=host_help)
 @click.option('--app-factory', 'app_factory_name', envvar='AIO_APP_FACTORY', help=app_factory_help)
diff --git a/aiohttp_devtools/runserver/config.py b/aiohttp_devtools/runserver/config.py
index 9a131a6..e4c4e1c 100644
--- a/aiohttp_devtools/runserver/config.py
+++ b/aiohttp_devtools/runserver/config.py
@@ -38,6 +38,8 @@ class Config:
                  python_path: Optional[str] = None,
                  static_url: str = '/static/',
                  livereload: bool = True,
+                 shutdown_by_url: bool = sys.platform.startswith("win32"),
+                 path_prefix: str = "/_devtools",
                  app_factory_name: Optional[str] = None,
                  host: str = INFER_HOST,
                  main_port: int = 8000,
@@ -66,6 +68,8 @@ class Config:
         self.static_path = self._resolve_path(static_path, "is_dir", "static-path") if static_path else None
         self.static_url = static_url
         self.livereload = livereload
+        self.shutdown_by_url = shutdown_by_url
+        self.path_prefix = path_prefix
         self.app_factory_name = app_factory_name
         self.infer_host = host == INFER_HOST
         self.host = 'localhost' if self.infer_host else host
@@ -183,6 +187,6 @@ class Config:
         return app
 
     def __str__(self) -> str:
-        fields = ('py_file', 'static_path', 'static_url', 'livereload',
-                  'app_factory_name', 'host', 'main_port', 'aux_port')
+        fields = ("py_file", "static_path", "static_url", "livereload", "shutdown_by_url",
+                  "path_prefix", "app_factory_name", "host", "main_port", "aux_port")
         return 'Config:\n' + '\n'.join('  {0}: {1!r}'.format(f, getattr(self, f)) for f in fields)
diff --git a/aiohttp_devtools/runserver/serve.py b/aiohttp_devtools/runserver/serve.py
index 1a7c3e7..39ebd83 100644
--- a/aiohttp_devtools/runserver/serve.py
+++ b/aiohttp_devtools/runserver/serve.py
@@ -5,12 +5,13 @@ import mimetypes
 import sys
 from errno import EADDRINUSE
 from pathlib import Path
-from typing import Any, Iterator, Optional
+from typing import Any, Iterator, Optional, NoReturn
 
 from aiohttp import WSMsgType, web
 from aiohttp.hdrs import LAST_MODIFIED, CONTENT_LENGTH
 from aiohttp.typedefs import Handler
 from aiohttp.web_exceptions import HTTPNotFound, HTTPNotModified
+from aiohttp.web_runner import GracefulExit
 from aiohttp.web_urldispatcher import StaticResource
 from yarl import URL
 
@@ -39,7 +40,7 @@ def _change_static_url(app: web.Application, url: str) -> None:
         _change_static_url(subapp, url)
 
 
-def modify_main_app(app: web.Application, config: Config) -> None:
+def modify_main_app(app: web.Application, config: Config) -> None:  # noqa: C901
     """
     Modify the app we're serving to make development easier, eg.
     * modify responses to add the livereload snippet
@@ -79,6 +80,18 @@ def modify_main_app(app: web.Application, config: Config) -> None:
 
         app.middlewares.insert(0, static_middleware)
 
+    # Fallback option to shutdown the application if signals don't work (e.g. Windows).
+    if config.shutdown_by_url:
+        async def do_shutdown(request: web.Request) -> web.Response:
+            def shutdown() -> NoReturn:
+                raise GracefulExit()
+            asyncio.get_running_loop().call_soon(shutdown)
+            return web.Response()
+
+        path = config.path_prefix + "/shutdown"
+        app.router.add_route("GET", path, do_shutdown, name="_devtools.shutdown")
+        dft_logger.debug("Created shutdown endpoint at http://{}:{}{}".format(config.host, config.main_port, path))
+
     if config.static_path is not None:
         static_url = 'http://{}:{}/{}'.format(config.host, config.aux_port, static_path)
         dft_logger.debug('settings app static_root_url to "%s"', static_url)
diff --git a/aiohttp_devtools/runserver/watch.py b/aiohttp_devtools/runserver/watch.py
index 8b5079c..dc84d0a 100644
--- a/aiohttp_devtools/runserver/watch.py
+++ b/aiohttp_devtools/runserver/watch.py
@@ -5,8 +5,10 @@ import sys
 from multiprocessing import Process
 from pathlib import Path
 from typing import AsyncIterator, Iterable, Optional, Tuple, Union
+from contextlib import suppress
 
 from aiohttp import ClientSession, web
+from aiohttp.client_exceptions import ClientError, ClientConnectionError
 from watchfiles import awatch
 
 from ..exceptions import AiohttpDevException
@@ -71,7 +73,7 @@ class AppTask(WatchTask):
                 self._reloads += 1
                 if any(f.endswith('.py') for _, f in changes):
                     logger.debug('%d changes, restarting server', len(changes))
-                    self._stop_dev_server()
+                    await self._stop_dev_server()
                     self._start_dev_server()
                     await self._src_reload_when_live(live_checks)
                 elif len(changes) == 1 and is_static(changes):
@@ -119,10 +121,34 @@ class AppTask(WatchTask):
         self._process = Process(target=serve_main_app, args=(self._config, tty_path))
         self._process.start()
 
-    def _stop_dev_server(self) -> None:
+    async def _stop_dev_server(self) -> None:
         if self._process.is_alive():
             logger.debug('stopping server process...')
+            if self._config.shutdown_by_url:  # Workaround for signals not working on Windows
+                url = "http://localhost:{}{}/shutdown".format(self._config.main_port, self._config.path_prefix)
+                logger.debug("Attempting to stop process via shutdown endpoint {}".format(url))
+                try:
+                    with suppress(ClientConnectionError):
+                        async with ClientSession() as session:
+                            async with session.get(url):
+                                pass
+                except (ConnectionError, ClientError, asyncio.TimeoutError) as ex:
+                    if self._process.is_alive():
+                        msg = "shutdown endpoint caused an error (will try signals next)"
+                        logger.warning(msg.format(type(ex), ex), exc_info=True)
+                    else:
+                        msg = "process stopped (despite error at shutdown endpoint)"
+                        logger.warning(msg.format(type(ex), ex), exc_info=True)
+                        return
+                else:
+                    self._process.join(5)
+                    if self._process.exitcode is None:
+                        logger.warning("shutdown endpoint did not terminate process, trying signals")
+                    else:
+                        logger.debug("process stopped via shutdown endpoint")
+                        return
             if self._process.pid:
+                logger.debug("sending SIGINT")
                 os.kill(self._process.pid, signal.SIGINT)
             self._process.join(5)
             if self._process.exitcode is None:
@@ -136,7 +162,7 @@ class AppTask(WatchTask):
 
     async def close(self, *args: object) -> None:
         self.stopper.set()
-        self._stop_dev_server()
+        await self._stop_dev_server()
         if self._session is None:
             raise RuntimeError("Object not started correctly before calling .close()")
         await asyncio.gather(super().close(), self._session.close())
diff --git a/requirements.txt b/requirements.txt
index 1e63ec1..7c60478 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -9,7 +9,7 @@ pytest-aiohttp==1.0.4
 pytest-asyncio==0.20.3
 pytest-cov==4.1.0
 pytest-forked==1.6.0
-pytest-mock==3.10.0
+pytest-mock==3.11.1
 pytest-sugar==0.9.7
 pytest-timeout==2.1.0
 pytest-toolbox==0.4


Test patch:
diff --git a/tests/cleanup_app.py b/tests/cleanup_app.py
new file mode 100644
index 0000000..e4a12a1
--- /dev/null
+++ b/tests/cleanup_app.py
@@ -0,0 +1,45 @@
+"""
+Test script for checking if cleanup/shutdown handlers are called.
+Used in test_runserver_cleanup.py.
+
+This test has proved to be quite difficult to automate in Windows
+(see discussion at https://github.com/aio-libs/aiohttp-devtools/pull/549)
+so it must be done manually as per the protocol below.
+
+Test Protocol:
+
+1. Run a Windows console such as Git Bash; ensure a working Python version is available
+2. ``cd`` to the main aiohttp-devtools directory (root of the git repo)
+3. Run ``python -c "from aiohttp_devtools.cli import runserver; runserver()" -v tests/cleanup_app.py``
+4. The console output should show the "CTX BEFORE" and "STARTUP" output from the code below
+5. Edit this file, e.g. a simple whitespace change, and save
+6. The console output should show the "SHUTDOWN" and "CTX AFTER" output, followed by the startup output
+7. End the process in the console with Ctrl-C
+8. The console should again show the shutdown output
+"""
+from aiohttp import web
+
+
+async def hello(_request):
+    return web.Response(text="hello, world")
+
+
+async def startup(_app):
+    print("====> STARTUP")
+
+
+async def context(_app):
+    print("====> CTX BEFORE")
+    yield
+    print("====> CTX AFTER")
+
+
+async def shutdown(_app):
+    print("====> SHUTDOWN")
+
+
+app = web.Application()
+app.router.add_get("/", hello)
+app.on_startup.append(startup)
+app.cleanup_ctx.append(context)
+app.on_shutdown.append(shutdown)
diff --git a/tests/test_runserver_cleanup.py b/tests/test_runserver_cleanup.py
new file mode 100644
index 0000000..b48e796
--- /dev/null
+++ b/tests/test_runserver_cleanup.py
@@ -0,0 +1,42 @@
+import asyncio
+import sys
+from pathlib import Path
+
+from aiohttp import ClientSession, ClientTimeout
+
+from .conftest import forked
+
+
+@forked  # forked doesn't run on Windows and is skipped - see cleanup_app.py instead
+async def test_server_cleanup_by_url() -> None:
+    async def is_server_running() -> bool:
+        async with ClientSession(timeout=ClientTimeout(total=1)) as session:
+            for i in range(30):
+                try:
+                    async with session.get("http://localhost:8000/") as r:
+                        assert r.status == 200
+                        text = await r.text()
+                        assert "hello, world" in text
+                        return True
+                except OSError:
+                    await asyncio.sleep(0.5)
+                except asyncio.TimeoutError:
+                    pass
+        return False
+
+    proc = await asyncio.create_subprocess_exec(
+        sys.executable, "-c", "from aiohttp_devtools.cli import runserver; runserver()",  # same as `adev runserver`
+        str(Path(__file__).parent / "cleanup_app.py"), "--shutdown-by-url", "-v",  # TODO(PY38): Remove str()
+        stdout=asyncio.subprocess.PIPE,
+        stderr=asyncio.subprocess.PIPE,
+    )
+    try:
+        assert await is_server_running()
+    finally:
+        proc.terminate()
+
+    await asyncio.sleep(2)
+    stdout, stderr = await proc.communicate()
+    assert b"process stopped via shutdown endpoint" in stderr
+    lines = tuple(x[6:] for x in stdout.decode('UTF-8').splitlines() if x.startswith('====> '))
+    assert lines == ("CTX BEFORE", "STARTUP", "SHUTDOWN", "CTX AFTER")
diff --git a/tests/test_runserver_main.py b/tests/test_runserver_main.py
index 6c0ae46..25b1009 100644
--- a/tests/test_runserver_main.py
+++ b/tests/test_runserver_main.py
@@ -1,8 +1,5 @@
 import asyncio
 import json
-import os
-import signal
-import time
 from unittest import mock
 
 import aiohttp
@@ -115,11 +112,6 @@ app.router.add_get('/', hello)
     assert len(aux_app.cleanup_ctx) == 1
 
 
-def kill_parent_soon(pid):
-    time.sleep(0.2)
-    os.kill(pid, signal.SIGINT)
-
-
 @forked
 async def test_run_app_aiohttp_client(tmpworkdir, aiohttp_client):
     mktree(tmpworkdir, SIMPLE_APP)
diff --git a/tests/test_runserver_serve.py b/tests/test_runserver_serve.py
index a8c8535..d89fbca 100644
--- a/tests/test_runserver_serve.py
+++ b/tests/test_runserver_serve.py
@@ -1,7 +1,6 @@
 import json
 import pathlib
 import socket
-from platform import system as get_os_family
 from typing import Dict
 from unittest.mock import MagicMock
 
@@ -16,18 +15,12 @@ from aiohttp_devtools.runserver.serve import check_port_open, cleanup_aux_app, m
 
 from .conftest import SIMPLE_APP, create_future
 
-non_windows_test = pytest.mark.skipif(
-    get_os_family() == 'Windows',
-    reason='This only works under UNIX-based OS and gets stuck under Windows',
-)
-
 
 async def test_check_port_open(unused_tcp_port_factory):
     port = unused_tcp_port_factory()
     await check_port_open(port, 0.001)
 
 
-@non_windows_test  # FIXME: probably needs some sock options
 async def test_check_port_not_open(unused_tcp_port_factory):
     port = unused_tcp_port_factory()
     with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock:
diff --git a/tests/test_runserver_watch.py b/tests/test_runserver_watch.py
index 2c50a3d..0205102 100644
--- a/tests/test_runserver_watch.py
+++ b/tests/test_runserver_watch.py
@@ -1,6 +1,6 @@
+import sys
 import asyncio
 from functools import partial
-from platform import system as get_os_family
 from unittest.mock import MagicMock, call
 
 import pytest
@@ -11,9 +11,10 @@ from aiohttp_devtools.runserver.watch import AppTask, LiveReloadTask
 
 from .conftest import create_future
 
-non_windows_test = pytest.mark.skipif(
-    get_os_family() == 'Windows',
-    reason='This only works under UNIX-based OS and gets stuck under Windows',
+
+needs_py38_test = pytest.mark.skipif(
+    sys.version_info < (3, 8),
+    reason="This only works on Python >=3.8 because otherwise MagicMock can't be used in 'await' expression",
 )
 
 
@@ -75,7 +76,7 @@ async def test_multiple_file_change(event_loop, mocker):
     await app_task._session.close()
 
 
-@non_windows_test
+@needs_py38_test
 async def test_python_no_server(event_loop, mocker):
     mocked_awatch = mocker.patch('aiohttp_devtools.runserver.watch.awatch')
     mocked_awatch.side_effect = create_awatch_mock({('x', '/path/to/file.py')})
@@ -159,7 +160,7 @@ class FakeProcess:
         pass
 
 
-def test_stop_process_dead(smart_caplog, mocker):
+async def test_stop_process_dead(smart_caplog, mocker):
     mock_kill = mocker.patch('aiohttp_devtools.runserver.watch.os.kill')
     mocker.patch('aiohttp_devtools.runserver.watch.awatch')
     mocker.patch('asyncio.Event')
@@ -167,12 +168,12 @@ def test_stop_process_dead(smart_caplog, mocker):
     app_task._process = MagicMock()
     app_task._process.is_alive = MagicMock(return_value=False)
     app_task._process.exitcode = 123
-    app_task._stop_dev_server()
+    await app_task._stop_dev_server()
     assert 'server process already dead, exit code: 123' in smart_caplog
     assert mock_kill.called is False
 
 
-def test_stop_process_clean(mocker):
+async def test_stop_process_clean(mocker):
     mock_kill = mocker.patch('aiohttp_devtools.runserver.watch.os.kill')
     mocker.patch('aiohttp_devtools.runserver.watch.awatch')
     mocker.patch('asyncio.Event')
@@ -181,11 +182,10 @@ def test_stop_process_clean(mocker):
     app_task._process.is_alive = MagicMock(return_value=True)
     app_task._process.pid = 321
     app_task._process.exitcode = 123
-    app_task._stop_dev_server()
+    await app_task._stop_dev_server()
     assert mock_kill.called_once_with(321, 2)
 
 
-@non_windows_test  # There's no signals in Windows
 async def test_stop_process_dirty(mocker):
     mock_kill = mocker.patch('aiohttp_devtools.runserver.watch.os.kill')
     mocker.patch('aiohttp_devtools.runserver.watch.awatch')
@@ -195,6 +195,6 @@ async def test_stop_process_dirty(mocker):
     process_mock.is_alive = MagicMock(return_value=True)
     process_mock.pid = 321
     process_mock.exitcode = None
-    app_task._stop_dev_server()
+    await app_task._stop_dev_server()
     assert mock_kill.call_args_list == [call(321, 2)]
     assert process_mock.kill.called_once()