Enable some kind of gradual typing in test_runner (#222)

It's not realistic to enable full-blown type checks
within test_runner's codebase, since the amount of
warnings revealed by mypy is overwhelming.

Tests are supposed to be easy to use, so we can't
cripple everybody's workflow for the sake of imaginary benefit.

Ultimately, the purpose of this attempt is three-fold:

* Facilitate code navigation when paired with python-language-server.
* Make method signatures apparent to a fellow programmer.
* Occasionally catch some obvious type errors.
This commit is contained in:
Dmitry Ivanov
2021-06-10 22:53:15 +03:00
committed by GitHub
parent 7a3794ef18
commit 96c7594d29
7 changed files with 197 additions and 61 deletions

View File

@@ -10,6 +10,7 @@ psycopg2 = "*"
[dev-packages]
yapf = "*"
flake8 = "*"
mypy = "*"
[requires]
# we need at least 3.6, but pipenv doesn't allow to say this directly

View File

@@ -1,7 +1,7 @@
{
"_meta": {
"hash": {
"sha256": "c0de1f00310b584a272dda58386939d53098e49d14db518804a9e56269bfec1d"
"sha256": "2cc3a75a3846ba41f9dc08903061ac587a4f0fb6864ae2073f871a944533dfe4"
},
"pipfile-spec": 6,
"requires": {
@@ -117,6 +117,41 @@
],
"version": "==0.6.1"
},
"mypy": {
"hashes": [
"sha256:0d0a87c0e7e3a9becdfbe936c981d32e5ee0ccda3e0f07e1ef2c3d1a817cf73e",
"sha256:25adde9b862f8f9aac9d2d11971f226bd4c8fbaa89fb76bdadb267ef22d10064",
"sha256:28fb5479c494b1bab244620685e2eb3c3f988d71fd5d64cc753195e8ed53df7c",
"sha256:2f9b3407c58347a452fc0736861593e105139b905cca7d097e413453a1d650b4",
"sha256:33f159443db0829d16f0a8d83d94df3109bb6dd801975fe86bacb9bf71628e97",
"sha256:3f2aca7f68580dc2508289c729bd49ee929a436208d2b2b6aab15745a70a57df",
"sha256:499c798053cdebcaa916eef8cd733e5584b5909f789de856b482cd7d069bdad8",
"sha256:4eec37370483331d13514c3f55f446fc5248d6373e7029a29ecb7b7494851e7a",
"sha256:552a815579aa1e995f39fd05dde6cd378e191b063f031f2acfe73ce9fb7f9e56",
"sha256:5873888fff1c7cf5b71efbe80e0e73153fe9212fafdf8e44adfe4c20ec9f82d7",
"sha256:61a3d5b97955422964be6b3baf05ff2ce7f26f52c85dd88db11d5e03e146a3a6",
"sha256:674e822aa665b9fd75130c6c5f5ed9564a38c6cea6a6432ce47eafb68ee578c5",
"sha256:7ce3175801d0ae5fdfa79b4f0cfed08807af4d075b402b7e294e6aa72af9aa2a",
"sha256:9743c91088d396c1a5a3c9978354b61b0382b4e3c440ce83cf77994a43e8c521",
"sha256:9f94aac67a2045ec719ffe6111df543bac7874cee01f41928f6969756e030564",
"sha256:a26f8ec704e5a7423c8824d425086705e381b4f1dfdef6e3a1edab7ba174ec49",
"sha256:abf7e0c3cf117c44d9285cc6128856106183938c68fd4944763003decdcfeb66",
"sha256:b09669bcda124e83708f34a94606e01b614fa71931d356c1f1a5297ba11f110a",
"sha256:cd07039aa5df222037005b08fbbfd69b3ab0b0bd7a07d7906de75ae52c4e3119",
"sha256:d23e0ea196702d918b60c8288561e722bf437d82cb7ef2edcd98cfa38905d506",
"sha256:d65cc1df038ef55a99e617431f0553cd77763869eebdf9042403e16089fe746c",
"sha256:d7da2e1d5f558c37d6e8c1246f1aec1e7349e4913d8fb3cb289a35de573fe2eb"
],
"index": "pypi",
"version": "==0.812"
},
"mypy-extensions": {
"hashes": [
"sha256:090fedd75945a69ae91ce1303b5824f428daf5a028d2f6ab8a299250a846f15d",
"sha256:2d82818f5bb3e369420cb3c4060a7970edba416647068eb4c5343488a6c604a8"
],
"version": "==0.4.3"
},
"pycodestyle": {
"hashes": [
"sha256:514f76d918fcc0b55c6680472f0a37970994e07bbb80725808c17089be302068",
@@ -133,6 +168,49 @@
"markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3'",
"version": "==2.3.1"
},
"typed-ast": {
"hashes": [
"sha256:01ae5f73431d21eead5015997ab41afa53aa1fbe252f9da060be5dad2c730ace",
"sha256:067a74454df670dcaa4e59349a2e5c81e567d8d65458d480a5b3dfecec08c5ff",
"sha256:0fb71b8c643187d7492c1f8352f2c15b4c4af3f6338f21681d3681b3dc31a266",
"sha256:1b3ead4a96c9101bef08f9f7d1217c096f31667617b58de957f690c92378b528",
"sha256:2068531575a125b87a41802130fa7e29f26c09a2833fea68d9a40cf33902eba6",
"sha256:209596a4ec71d990d71d5e0d312ac935d86930e6eecff6ccc7007fe54d703808",
"sha256:2c726c276d09fc5c414693a2de063f521052d9ea7c240ce553316f70656c84d4",
"sha256:398e44cd480f4d2b7ee8d98385ca104e35c81525dd98c519acff1b79bdaac363",
"sha256:52b1eb8c83f178ab787f3a4283f68258525f8d70f778a2f6dd54d3b5e5fb4341",
"sha256:5feca99c17af94057417d744607b82dd0a664fd5e4ca98061480fd8b14b18d04",
"sha256:7538e495704e2ccda9b234b82423a4038f324f3a10c43bc088a1636180f11a41",
"sha256:760ad187b1041a154f0e4d0f6aae3e40fdb51d6de16e5c99aedadd9246450e9e",
"sha256:777a26c84bea6cd934422ac2e3b78863a37017618b6e5c08f92ef69853e765d3",
"sha256:95431a26309a21874005845c21118c83991c63ea800dd44843e42a916aec5899",
"sha256:9ad2c92ec681e02baf81fdfa056fe0d818645efa9af1f1cd5fd6f1bd2bdfd805",
"sha256:9c6d1a54552b5330bc657b7ef0eae25d00ba7ffe85d9ea8ae6540d2197a3788c",
"sha256:aee0c1256be6c07bd3e1263ff920c325b59849dc95392a05f258bb9b259cf39c",
"sha256:af3d4a73793725138d6b334d9d247ce7e5f084d96284ed23f22ee626a7b88e39",
"sha256:b36b4f3920103a25e1d5d024d155c504080959582b928e91cb608a65c3a49e1a",
"sha256:b9574c6f03f685070d859e75c7f9eeca02d6933273b5e69572e5ff9d5e3931c3",
"sha256:bff6ad71c81b3bba8fa35f0f1921fb24ff4476235a6e94a26ada2e54370e6da7",
"sha256:c190f0899e9f9f8b6b7863debfb739abcb21a5c054f911ca3596d12b8a4c4c7f",
"sha256:c907f561b1e83e93fad565bac5ba9c22d96a54e7ea0267c708bffe863cbe4075",
"sha256:cae53c389825d3b46fb37538441f75d6aecc4174f615d048321b716df2757fb0",
"sha256:dd4a21253f42b8d2b48410cb31fe501d32f8b9fbeb1f55063ad102fe9c425e40",
"sha256:dde816ca9dac1d9c01dd504ea5967821606f02e510438120091b84e852367428",
"sha256:f2362f3cb0f3172c42938946dbc5b7843c2a28aec307c49100c8b38764eb6927",
"sha256:f328adcfebed9f11301eaedfa48e15bdece9b519fb27e6a8c01aa52a17ec31b3",
"sha256:f8afcf15cc511ada719a88e013cec87c11aff7b91f019295eb4530f96fe5ef2f",
"sha256:fb1bbeac803adea29cedd70781399c99138358c26d05fcbd23c13016b7f5ec65"
],
"version": "==1.4.3"
},
"typing-extensions": {
"hashes": [
"sha256:0ac0f89795dd19de6b97debb0c6af1c70987fd80a2d62d1958f7e56fcc31b497",
"sha256:50b6f157849174217d0656f99dc82fe932884fb250826c18350e159ec6cdf342",
"sha256:779383f6086d90c99ae41cf0ff39aac8a7937a9283ce0a414e5dd782f4c94a84"
],
"version": "==3.10.0.0"
},
"yapf": {
"hashes": [
"sha256:408fb9a2b254c302f49db83c59f9aa0b4b0fd0ec25be3a5c51181327922ff63d",

View File

@@ -99,5 +99,6 @@ Before submitting a patch, please consider:
* Writing a couple of docstrings to clarify the reasoning behind a new test.
* Running `flake8` (or a linter of your choice, e.g. `pycodestyle`) and fixing possible defects, if any.
* Formatting the code with `yapf -r -i .` (TODO: implement an opt-in pre-commit hook for that).
* (Optional) Typechecking the code with `mypy .`. Currently this mostly affects `fixtures/zenith_fixtures.py`.
The tools can be installed with `pipenv install --dev`.

View File

@@ -17,7 +17,7 @@ def test_createuser(zenith_cli, pageserver, postgres, pg_bin):
with conn.cursor() as cur:
# Cause a 'relmapper' change in the original branch
cur.execute('CREATE USER testuser with password %s', ('testpwd',))
cur.execute('CREATE USER testuser with password %s', ('testpwd', ))
cur.execute('CHECKPOINT')

View File

@@ -1,13 +1,15 @@
import os
import subprocess
from typing import Any, List
def get_self_dir():
def get_self_dir() -> str:
""" Get the path to the directory where this script lives. """
return os.path.dirname(os.path.abspath(__file__))
def mkdir_if_needed(path):
def mkdir_if_needed(path: str) -> None:
""" Create a directory if it doesn't already exist
Note this won't try to create intermediate directories.
@@ -18,7 +20,7 @@ def mkdir_if_needed(path):
os.mkdir(path)
def subprocess_capture(capture_dir, cmd, **kwargs):
def subprocess_capture(capture_dir: str, cmd: List[str], **kwargs: Any) -> None:
""" Run a process and capture its output
Output will go to files named "cmd_NNN.stdout" and "cmd_NNN.stderr"
@@ -42,7 +44,7 @@ def subprocess_capture(capture_dir, cmd, **kwargs):
_global_counter = 0
def global_counter():
def global_counter() -> int:
""" A really dumb global counter.
This is useful for giving output files a unique number, so if we run the

View File

@@ -4,6 +4,8 @@ import pytest
import shutil
import subprocess
from typing import Any, Callable, Dict, Iterator, List, Optional, TypeVar, Literal
from .utils import (get_self_dir, mkdir_if_needed, subprocess_capture)
"""
This file contains pytest fixtures. A fixture is a test resource that can be
@@ -23,15 +25,18 @@ Don't import functions from this file, or pytest will emit warnings. Instead
put directly-importable functions into utils.py or another separate file.
"""
Env = Dict[str, str]
Fn = TypeVar('Fn', bound=Callable[..., Any])
DEFAULT_OUTPUT_DIR = 'test_output'
DEFAULT_POSTGRES_DIR = 'tmp_install'
def determine_scope(fixture_name, config):
def determine_scope(fixture_name: str, config: Any) -> str:
return 'session'
def zenfixture(func):
def zenfixture(func: Fn) -> Fn:
"""
This is a python decorator for fixtures with a flexible scope.
@@ -43,15 +48,14 @@ def zenfixture(func):
stored in a directory called "shared".
"""
if os.environ.get('TEST_SHARED_FIXTURES') is None:
scope = 'function'
else:
scope = 'session'
scope: Literal['session', 'function'] = \
'function' if os.environ.get('TEST_SHARED_FIXTURES') is None else 'session'
return pytest.fixture(func, scope=scope)
@pytest.fixture(autouse=True, scope='session')
def safety_check():
def safety_check() -> None:
""" Ensure that no unwanted daemons are running before we start testing. """
# does not use -c as it is not supported on macOS
@@ -71,7 +75,7 @@ class ZenithCli:
We also store an environment that will tell the CLI to operate
on a particular ZENITH_REPO_DIR.
"""
def __init__(self, binpath, repo_dir, pg_distrib_dir):
def __init__(self, binpath: str, repo_dir: str, pg_distrib_dir: str):
assert os.path.isdir(binpath)
self.binpath = binpath
self.bin_zenith = os.path.join(binpath, 'zenith')
@@ -79,7 +83,7 @@ class ZenithCli:
self.env['ZENITH_REPO_DIR'] = repo_dir
self.env['POSTGRES_DISTRIB_DIR'] = pg_distrib_dir
def run(self, arguments):
def run(self, arguments: List[str]) -> Any:
"""
Run "zenith" with the specified arguments.
@@ -93,6 +97,7 @@ class ZenithCli:
"""
assert type(arguments) == list
args = [self.bin_zenith] + arguments
print('Running command "{}"'.format(' '.join(args)))
return subprocess.run(args,
@@ -104,41 +109,58 @@ class ZenithCli:
@zenfixture
def zenith_cli(zenith_binpath, repo_dir, pg_distrib_dir):
def zenith_cli(zenith_binpath: str, repo_dir: str, pg_distrib_dir: str) -> ZenithCli:
return ZenithCli(zenith_binpath, repo_dir, pg_distrib_dir)
class ZenithPageserver:
""" An object representing a running pageserver. """
def __init__(self, zenith_cli):
def __init__(self, zenith_cli: ZenithCli):
self.zenith_cli = zenith_cli
self.running = False
# Initialize the repository, i.e. run "zenith init"
def init(self):
self.zenith_cli.run(['init'])
def init(self) -> 'ZenithPageserver':
"""
Initialize the repository, i.e. run "zenith init".
Returns self.
"""
self.zenith_cli.run(['init'])
return self
def start(self) -> 'ZenithPageserver':
"""
Start the page server.
Returns self.
"""
# Start the page server
def start(self):
self.zenith_cli.run(['start'])
self.running = True
return self
# Stop the page server
def stop(self):
self.zenith_cli.run(['stop'])
self.running = True
def stop(self) -> 'ZenithPageserver':
"""
Stop the page server.
Returns self.
"""
if self.running:
self.zenith_cli.run(['stop'])
self.running = False
return self
# The page server speaks the Postgres FE/BE protocol, so you can connect
# to it with any Postgres client, and run special commands. This function
# returns a libpq connection string for connecting to it.
def connstr(self):
def connstr(self) -> str:
username = getpass.getuser()
conn_str = 'host={} port={} dbname=postgres user={}'.format('localhost', 64000, username)
return conn_str
@zenfixture
def pageserver(zenith_cli):
def pageserver(zenith_cli: ZenithCli) -> Iterator[ZenithPageserver]:
"""
The 'pageserver' fixture provides a Page Server that's up and running.
@@ -151,12 +173,12 @@ def pageserver(zenith_cli):
test called 'test_foo' would create and use branches with the 'test_foo' prefix.
"""
ps = ZenithPageserver(zenith_cli)
ps.init()
ps.start()
ps = ZenithPageserver(zenith_cli).init().start()
# For convenience in tests, create a branch from the freshly-initialized cluster.
zenith_cli.run(["branch", "empty", "main"])
yield ps
# After the yield comes any cleanup code we need.
print('Starting pageserver cleanup')
ps.stop()
@@ -164,43 +186,45 @@ def pageserver(zenith_cli):
class Postgres:
""" An object representing a running postgres daemon. """
def __init__(self, zenith_cli, repo_dir, instance_num):
def __init__(self, zenith_cli: ZenithCli, repo_dir: str, instance_num: int):
self.zenith_cli = zenith_cli
self.instance_num = instance_num
self.running = False
self.username = getpass.getuser()
self.host = 'localhost'
self.port = 55431 + instance_num
self.port = 55431 + instance_num # TODO: find a better way
self.repo_dir = repo_dir
self.branch = None
self.branch: Optional[str] = None # dubious, see asserts below
# path to conf is <repo_dir>/pgdatadirs/<branch_name>/postgresql.conf
def create(self, branch, config_lines=None):
def create(self, branch: str, config_lines: Optional[List[str]] = None) -> 'Postgres':
"""
Create the pg data directory.
Returns self.
"""
if not config_lines:
config_lines = []
self.zenith_cli.run(['pg', 'create', branch])
self.branch = branch
if config_lines is None:
config_lines = []
self.config(config_lines)
return self
def start(self):
def start(self) -> 'Postgres':
"""
Start the Postgres instance.
Returns self.
"""
assert self.branch is not None
self.zenith_cli.run(['pg', 'start', self.branch])
self.running = True
return self
def config(self, lines):
def config(self, lines: List[str]) -> 'Postgres':
"""
Add lines to postgresql.conf.
Lines should be an array of valid postgresql.conf rows.
@@ -216,28 +240,31 @@ class Postgres:
return self
def stop(self):
def stop(self) -> 'Postgres':
"""
Stop the Postgres instance if it's running.
Returns self.
"""
if self.running:
assert self.branch is not None
self.zenith_cli.run(['pg', 'stop', self.branch])
self.running = False
return self
def stop_and_destroy(self):
def stop_and_destroy(self) -> 'Postgres':
"""
Stop the Postgres instance, then destroy it.
Returns self.
"""
assert self.branch is not None
self.zenith_cli.run(['pg', 'stop', '--destroy', self.branch])
return self
def create_start(self, branch, config_lines=None):
def create_start(self, branch: str, config_lines: Optional[List[str]] = None) -> 'Postgres':
"""
Create a Postgres instance, then start it.
Returns self.
@@ -247,41 +274,49 @@ class Postgres:
return self
def connstr(self, dbname='postgres', username=None):
def connstr(self, dbname: str = 'postgres', username: Optional[str] = None) -> str:
"""
Build a libpq connection string for the Postgres instance.
"""
conn_str = 'host={} port={} dbname={} user={}'.format(self.host, self.port, dbname,
username or self.username)
(username or self.username))
return conn_str
class PostgresFactory:
""" An object representing multiple running postgres daemons. """
def __init__(self, zenith_cli, repo_dir):
def __init__(self, zenith_cli: ZenithCli, repo_dir: str):
self.zenith_cli = zenith_cli
self.host = 'localhost'
self.repo_dir = repo_dir
self.num_instances = 0
self.instances = []
self.instances: List[Postgres] = []
def create_start(self,
branch: str = "main",
config_lines: Optional[List[str]] = None) -> Postgres:
def create_start(self, branch="main", config_lines=None):
pg = Postgres(self.zenith_cli, self.repo_dir, self.num_instances + 1)
self.num_instances += 1
self.instances.append(pg)
return pg.create_start(branch, config_lines)
def stop_all(self):
def stop_all(self) -> 'PostgresFactory':
for pg in self.instances:
pg.stop()
return self
@zenfixture
def postgres(zenith_cli, repo_dir):
def postgres(zenith_cli: ZenithCli, repo_dir: str) -> Iterator[PostgresFactory]:
pgfactory = PostgresFactory(zenith_cli, repo_dir)
yield pgfactory
# After the yield comes any cleanup code we need.
print('Starting postgres cleanup')
pgfactory.stop_all()
@@ -289,25 +324,25 @@ def postgres(zenith_cli, repo_dir):
class PgBin:
""" A helper class for executing postgres binaries """
def __init__(self, log_dir, pg_distrib_dir):
def __init__(self, log_dir: str, pg_distrib_dir: str):
self.log_dir = log_dir
self.pg_install_path = pg_distrib_dir
self.pg_bin_path = os.path.join(self.pg_install_path, 'bin')
self.env = os.environ.copy()
self.env['LD_LIBRARY_PATH'] = os.path.join(self.pg_install_path, 'lib')
def _fixpath(self, command):
def _fixpath(self, command: List[str]) -> None:
if '/' not in command[0]:
command[0] = os.path.join(self.pg_bin_path, command[0])
def _build_env(self, env_add):
def _build_env(self, env_add: Optional[Env]) -> Env:
if env_add is None:
return self.env
env = self.env.copy()
env.update(env_add)
return env
def run(self, command, env=None, cwd=None):
def run(self, command: List[str], env: Optional[Env] = None, cwd: Optional[str] = None) -> None:
"""
Run one of the postgres binaries.
@@ -326,7 +361,10 @@ class PgBin:
env = self._build_env(env)
subprocess.run(command, env=env, cwd=cwd, check=True)
def run_capture(self, command, env=None, cwd=None):
def run_capture(self,
command: List[str],
env: Optional[Env] = None,
cwd: Optional[str] = None) -> None:
"""
Run one of the postgres binaries, with stderr and stdout redirected to a file.
@@ -340,12 +378,12 @@ class PgBin:
@zenfixture
def pg_bin(test_output_dir, pg_distrib_dir):
def pg_bin(test_output_dir: str, pg_distrib_dir: str) -> PgBin:
return PgBin(test_output_dir, pg_distrib_dir)
@zenfixture
def base_dir():
def base_dir() -> str:
""" find the base directory (currently this is the git root) """
base_dir = os.path.normpath(os.path.join(get_self_dir(), '../..'))
@@ -354,7 +392,7 @@ def base_dir():
@zenfixture
def top_output_dir(base_dir):
def top_output_dir(base_dir: str) -> str:
""" Compute the top-level directory for all tests. """
env_test_output = os.environ.get('TEST_OUTPUT')
@@ -367,7 +405,7 @@ def top_output_dir(base_dir):
@zenfixture
def test_output_dir(request, top_output_dir):
def test_output_dir(request: Any, top_output_dir: str) -> str:
""" Compute the working directory for an individual test. """
if os.environ.get('TEST_SHARED_FIXTURES') is None:
@@ -385,7 +423,7 @@ def test_output_dir(request, top_output_dir):
@zenfixture
def repo_dir(request, test_output_dir):
def repo_dir(request: Any, test_output_dir: str) -> str:
"""
Compute the test repo_dir.
@@ -398,7 +436,7 @@ def repo_dir(request, test_output_dir):
@zenfixture
def zenith_binpath(base_dir):
def zenith_binpath(base_dir: str) -> str:
""" Find the zenith binaries. """
env_zenith_bin = os.environ.get('ZENITH_BIN')
@@ -412,7 +450,7 @@ def zenith_binpath(base_dir):
@zenfixture
def pg_distrib_dir(base_dir):
def pg_distrib_dir(base_dir: str) -> str:
""" Find the postgres install. """
env_postgres_bin = os.environ.get('POSTGRES_DISTRIB_DIR')

View File

@@ -10,3 +10,19 @@ max-line-length = 100
[yapf]
based_on_style = pep8
column_limit = 100
[mypy]
# some tests don't typecheck when this flag is set
check_untyped_defs = false
disallow_incomplete_defs = false
disallow_untyped_calls = false
disallow_untyped_decorators = false
disallow_untyped_defs = false
strict = true
[mypy-psycopg2.*]
ignore_missing_imports = true
[mypy-pytest.*]
ignore_missing_imports = true