Skip to content

Commit

Permalink
Fix "pypy3" testenv on Windows, Fix MacOS flakeyness (#254)
Browse files Browse the repository at this point in the history
## Change shared ctypes to MP.Queue

PyPy has problems with shared ctypes Value, especially on Windows. Not
sure why. This resulted in flakey pypy3-* tests.

Converting to MP.Queue might be slightly slower, but it's much more
stable/deterministic. And since this change impacts only 3 tests out of
several hundreds, it's worth it for the expansion of Windows test scope.

## Activate pypy3 testing in tox.ini

## Specify non-"win32" platforms for "static"

This enables us to add "static" into tox/envlist, for easier invocation.

## Change "_dynamic" to "_dump"

## Make ENV dump ".py"

So we can leverage syntax-highlighting when checking

## Also delete temporary "parallel" coverage files

## Move coverage/diffcov files

Temporary/intermediate files go to _dump

Reports go to htmlcov

## Exclude _dump from pytype

## Prefix ENV dump file with "ENV = \"

To reduce IDE protests

## Extend watched process delay by 50%

Exact same with AUTOSTOP_DELAY makes the test flakey.

## Up version to 1.4.0a4 and NEWS.rst

## Replace ConnectionError with OSError (for MacOS)

Refs:
- racitup/static-ranges#1
- benoitc/gunicorn#1487
- http://erickt.github.io/blog/2014/11/19/adventures-in-debugging-a-potential-osx-kernel-bug/

## Update README.rst

- Now pypy3 is no longer a stepchild.
- Notes on which testenvs work/don't work on Cygwin.
  • Loading branch information
pepoluan committed Feb 24, 2021
1 parent b7122a0 commit be24d86
Show file tree
Hide file tree
Showing 10 changed files with 117 additions and 73 deletions.
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ docs/_build/
nosetests.xml
__pycache__/
htmlcov/
_dynamic/
_dump/
coverage.xml
coverage-*.xml
diffcov.html
Expand Down
16 changes: 8 additions & 8 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ You need **at least Python 3.6** to use this library.
Supported Platforms
-----------------------

``aiosmtpd`` has been tested on the following platforms (in alphabetical order):
``aiosmtpd`` has been tested on **CPython** and **PyPy3.7**
for the following platforms (in alphabetical order):

* Cygwin (on Windows 10) [1]
* FreeBSD 12 [2]
Expand Down Expand Up @@ -177,11 +178,8 @@ have been configured and tested:
- ``profile`` = no coverage testing, but code profiling instead.
This must be **invoked manually** using the ``-e`` parameter

**Note 1:** Due to possible 'complications' when setting up PyPy on
systems without pyenv, ``pypy3`` tests also will not be automatically
run; you must invoke them manually. For example::

$ tox -e pypy3-nocov
**Note 1:** As of 2021-02-23,
only the ``{py36,py38}-{nocov,cov}`` combinations work on **Cygwin**.

**Note 2:** It is also possible to use whatever Python version is used when
invoking ``tox`` by using the ``py`` target, but you must explicitly include
Expand Down Expand Up @@ -226,8 +224,10 @@ have been configured and tested:
**Note 1:** Please ensure that `all pytype dependencies`_ have been installed before
executing this testenv.

**Note 2:** Because ``pytype`` does not run on Windows,
This testenv must be invoked explicitly; it will not automatically run.
**Note 2:** This testenv will be _SKIPPED_ on Windows,
because ``pytype`` currently cannot run on Windows.

**Note 3:** This testenv does NOT work on **Cygwin**.

.. _`all pytype dependencies`: https://github.com/google/pytype/blob/2021.02.09/CONTRIBUTING.md#pytype-dependencies

Expand Down
2 changes: 1 addition & 1 deletion aiosmtpd/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright 2014-2021 The aiosmtpd Developers
# SPDX-License-Identifier: Apache-2.0

__version__ = "1.4.0a3"
__version__ = "1.4.0a4"
5 changes: 5 additions & 0 deletions aiosmtpd/docs/NEWS.rst
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ Added
.. _`PROXY Protocol`: https://www.haproxy.com/blog/using-haproxy-with-the-proxy-protocol-to-better-secure-your-database/
.. |PROXY Protocol| replace:: **PROXY Protocol**

Fixed/Improved
--------------
* ``pypy3`` testenv for tox can now run on Windows
* ``static`` testenv now auto-skipped on Windows


1.3.2 (2021-02-20)
==================
Expand Down
104 changes: 54 additions & 50 deletions aiosmtpd/tests/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import multiprocessing as MP
import os
import time
from ctypes import c_bool
from contextlib import contextmanager
from smtplib import SMTP as SMTPClient
from smtplib import SMTP_SSL
from typing import Generator
Expand Down Expand Up @@ -92,43 +92,59 @@ def setuid(mocker):
# region ##### Helper Funcs ###########################################################


def watch_for_tls(ready_flag, has_tls, req_tls):
has_tls.value = False
req_tls.value = False
def watch_for_tls(ready_flag, retq: MP.Queue):
has_tls = False
req_tls = False
ready_flag.set()
start = time.monotonic()
while (time.monotonic() - start) <= AUTOSTOP_DELAY:
delay = AUTOSTOP_DELAY * 1.5
while (time.monotonic() - start) <= delay:
try:
with SMTPClient("localhost", 8025) as client:
resp = client.docmd("HELP", "HELO")
if resp == S.S530_STARTTLS_FIRST:
req_tls.value = True
req_tls = True
client.ehlo("exemple.org")
if "starttls" in client.esmtp_features:
has_tls.value = True
return
has_tls = True
break
except Exception:
time.sleep(0.05)
retq.put(has_tls)
retq.put(req_tls)


def watch_for_smtps(ready_flag, has_smtps):
has_smtps.value = False
def watch_for_smtps(ready_flag, retq: MP.Queue):
has_smtps = False
ready_flag.set()
start = time.monotonic()
while (time.monotonic() - start) <= AUTOSTOP_DELAY:
delay = AUTOSTOP_DELAY * 1.5
while (time.monotonic() - start) <= delay:
try:
with SMTP_SSL("localhost", 8025) as client:
client.ehlo("exemple.org")
has_smtps.value = True
return
has_smtps = True
break
except Exception:
time.sleep(0.05)
retq.put(has_smtps)


def main_n(*args):
main(("-n",) + args)


@contextmanager
def watcher_process(func):
redy = MP.Event()
retq = MP.Queue()
proc = MP.Process(target=func, args=(redy, retq))
proc.start()
redy.wait()
yield retq
proc.join()


# endregion


Expand Down Expand Up @@ -196,47 +212,35 @@ def test_debug_3(self):

class TestMainByWatcher:
def test_tls(self, temp_event_loop):
ready_flag = MP.Event()
has_starttls = MP.Value(c_bool)
require_tls = MP.Value(c_bool)
p = MP.Process(
target=watch_for_tls, args=(ready_flag, has_starttls, require_tls)
)
p.start()
ready_flag.wait()
temp_event_loop.call_later(AUTOSTOP_DELAY, temp_event_loop.stop)
main_n("--tlscert", str(SERVER_CRT), "--tlskey", str(SERVER_KEY))
p.join()
assert has_starttls.value is True
assert require_tls.value is True
with watcher_process(watch_for_tls) as retq:
temp_event_loop.call_later(AUTOSTOP_DELAY, temp_event_loop.stop)
main_n("--tlscert", str(SERVER_CRT), "--tlskey", str(SERVER_KEY))
has_starttls = retq.get()
assert has_starttls is True
require_tls = retq.get()
assert require_tls is True

def test_tls_noreq(self, temp_event_loop):
ready_flag = MP.Event()
has_starttls = MP.Value(c_bool)
require_tls = MP.Value(c_bool)
p = MP.Process(
target=watch_for_tls, args=(ready_flag, has_starttls, require_tls)
)
p.start()
ready_flag.wait()
temp_event_loop.call_later(AUTOSTOP_DELAY, temp_event_loop.stop)
main_n(
"--tlscert", str(SERVER_CRT), "--tlskey", str(SERVER_KEY), "--no-requiretls"
)
p.join()
assert has_starttls.value is True
assert require_tls.value is False
with watcher_process(watch_for_tls) as retq:
temp_event_loop.call_later(AUTOSTOP_DELAY, temp_event_loop.stop)
main_n(
"--tlscert",
str(SERVER_CRT),
"--tlskey",
str(SERVER_KEY),
"--no-requiretls",
)
has_starttls = retq.get()
assert has_starttls is True
require_tls = retq.get()
assert require_tls is False

def test_smtps(self, temp_event_loop):
ready_flag = MP.Event()
has_smtps = MP.Value(c_bool)
p = MP.Process(target=watch_for_smtps, args=(ready_flag, has_smtps))
p.start()
ready_flag.wait()
temp_event_loop.call_later(AUTOSTOP_DELAY, temp_event_loop.stop)
main_n("--smtpscert", str(SERVER_CRT), "--smtpskey", str(SERVER_KEY))
p.join()
assert has_smtps.value is True
with watcher_process(watch_for_smtps) as retq:
temp_event_loop.call_later(AUTOSTOP_DELAY, temp_event_loop.stop)
main_n("--smtpscert", str(SERVER_CRT), "--smtpskey", str(SERVER_KEY))
has_smtps = retq.get()
assert has_smtps is True


class TestParseArgs:
Expand Down
29 changes: 27 additions & 2 deletions aiosmtpd/tests/test_proxyprotocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# SPDX-License-Identifier: Apache-2.0

import asyncio
import errno
import logging
import operator
import random
Expand Down Expand Up @@ -1003,11 +1004,23 @@ def test_timeout(self, plain_controller, handshake):
# Try resending the handshake. Should also fail (because connection has
# been closed by the server.
# noinspection PyTypeChecker
with pytest.raises(ConnectionError):
with pytest.raises(OSError) as exc_info:
sock.send(handshake)
resp = sock.recv(4096)
if resp == b"":
raise ConnectionAbortedError
# MacOS sometimes raises EPROTOTYPE, which won't result in ConnectionError
# but in OSError(errno=EPROTOTYPE). Let's check that here.
# Refs:
# - https://github.com/racitup/static-ranges/issues/1
# - https://github.com/benoitc/gunicorn/issues/1487
# - http://erickt.github.io/blog/2014/11/19/adventures-in-debugging-\
# a-potential-osx-kernel-bug/
exc = exc_info.value
if isinstance(exc, ConnectionError):
pass
else:
assert exc.errno in (errno.EPROTOTYPE, errno.EPIPE)
# Assert that we can connect properly afterwards (that is, server is not
# terminated)
self._okay(handshake)
Expand All @@ -1028,11 +1041,23 @@ def test_incomplete(self, plain_controller, handshake):
# Try resending the handshake. Should also fail (because connection has
# been closed by the server.
# noinspection PyTypeChecker
with pytest.raises(ConnectionError):
with pytest.raises(OSError) as exc_info:
sock.send(handshake)
resp = sock.recv(4096)
if resp == b"":
raise ConnectionError
# MacOS sometimes raises EPROTOTYPE, which won't result in ConnectionError
# but in OSError(errno=EPROTOTYPE). Let's check that here.
# Refs:
# - https://github.com/racitup/static-ranges/issues/1
# - https://github.com/benoitc/gunicorn/issues/1487
# - http://erickt.github.io/blog/2014/11/19/adventures-in-debugging-\
# a-potential-osx-kernel-bug/
exc = exc_info.value
if isinstance(exc, ConnectionError):
pass
else:
assert exc.errno in (errno.EPROTOTYPE, errno.EPIPE)
# Assert that we can connect properly afterwards (that is, server is not
# terminated)
self._okay(handshake)
Expand Down
19 changes: 13 additions & 6 deletions housekeep.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,24 +31,29 @@ class Style:
RESET_ALL = "\x1b[0m"


DUMP_DIR = "_dump"
TOX_ENV_NAME = os.environ.get("TOX_ENV_NAME", None)

# These dirs will be processed if exists, so no need to remove old entries.
# I suggest keeping them to clean up old artefacts just in case.
WORKDIRS = (
".mypy_cache",
".pytype",
".pytest-cache",
".pytest_cache",
".pytest-cache", # <-+-- One of these is a typo
".pytest_cache", # <-+ Keep them both just in case
".tox",
"_dynamic",
DUMP_DIR,
"_dynamic", # Pre 1.4.0a4
"aiosmtpd.egg-info",
"build",
"dist",
"htmlcov",
"prof",
"prof", # Only if "profile" testenv ran
)

WORKFILES = (
".coverage",
".coverage.*",
"coverage.xml",
"diffcov.html",
"coverage-*.xml",
Expand Down Expand Up @@ -87,8 +92,10 @@ def deldir(targ: Path, verbose: bool = True):


def dump_env():
os.makedirs("_dynamic", exist_ok=True)
with open(f"_dynamic/ENV.{TOX_ENV_NAME}", "wt") as fout:
dumpdir = Path(DUMP_DIR)
dumpdir.mkdir(exist_ok=True)
with (dumpdir / f"ENV.{TOX_ENV_NAME}.py").open("wt") as fout:
print("ENV = \\", file=fout)
pprint.pprint(dict(os.environ), stream=fout)


Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ directory = "htmlcov/${TOX_ENV_NAME}"
title = "aiosmtpd coverage for ${TOX_ENV_NAME}"

[tool.coverage.xml]
output = "coverage-${INTERP}.xml"
output = "_dump/coverage-${INTERP}.xml"

[tool.check-manifest]
ignore = [
Expand Down
1 change: 1 addition & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ zip_ok = false
[pytype]
exclude =
aiosmtpd/docs/_exts/*
_dump/*
disable =
not-supported-yet

Expand Down
10 changes: 6 additions & 4 deletions tox.ini
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[tox]
minversion = 3.9.0
envlist = qa, docs, py{36,37,38,39}-{nocov,cov,diffcov}
envlist = qa, static, docs, py{36,37,38,39,py3}-{nocov,cov,diffcov}
skip_missing_interpreters = True

[testenv]
Expand All @@ -17,8 +17,8 @@ commands =
!diffcov: bandit -c bandit.yml -r aiosmtpd
nocov: pytest --verbose -p no:cov --tb=short {posargs}
cov: pytest --cov --cov-report=xml --cov-report=html --cov-report=term --tb=short {posargs}
diffcov: diff-cover coverage-{env:INTERP}.xml --html-report diffcov-{env:INTERP}.html
diffcov: diff-cover coverage-{env:INTERP}.xml --fail-under=100
diffcov: diff-cover _dump/coverage-{env:INTERP}.xml --html-report htmlcov/diffcov-{env:INTERP}.html
diffcov: diff-cover _dump/coverage-{env:INTERP}.xml --fail-under=100
profile: pytest --profile {posargs}
python housekeep.py --afterbar --afterbar gather
#sitepackages = True
Expand All @@ -37,7 +37,7 @@ deps =
pytest-sugar
diff_cover
setenv =
cov: COVERAGE_FILE={toxinidir}/.coverage
cov: COVERAGE_FILE={toxinidir}/_dump/.coverage
nocov: PYTHONASYNCIODEBUG=1
py36: INTERP=py36
py37: INTERP=py37
Expand Down Expand Up @@ -87,6 +87,8 @@ deps:

[testenv:static]
basepython = python3
# (?!...) is a negative-lookahed, means that it must NOT match
platform = (?!win32)
envdir = {toxworkdir}/static
commands =
python housekeep.py prep
Expand Down

0 comments on commit be24d86

Please sign in to comment.