diff --git a/test_runner/Pipfile b/test_runner/Pipfile index 7f33320c13..923aba9ad3 100644 --- a/test_runner/Pipfile +++ b/test_runner/Pipfile @@ -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 diff --git a/test_runner/Pipfile.lock b/test_runner/Pipfile.lock index 3ecb23fbef..31da752ece 100644 --- a/test_runner/Pipfile.lock +++ b/test_runner/Pipfile.lock @@ -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", diff --git a/test_runner/README.md b/test_runner/README.md index 8309900271..303acb8de0 100644 --- a/test_runner/README.md +++ b/test_runner/README.md @@ -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`. diff --git a/test_runner/batch_others/test_createuser.py b/test_runner/batch_others/test_createuser.py index 15ff2ff63d..649559e162 100644 --- a/test_runner/batch_others/test_createuser.py +++ b/test_runner/batch_others/test_createuser.py @@ -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') diff --git a/test_runner/fixtures/utils.py b/test_runner/fixtures/utils.py index 4ab283a974..6214503af5 100644 --- a/test_runner/fixtures/utils.py +++ b/test_runner/fixtures/utils.py @@ -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 diff --git a/test_runner/fixtures/zenith_fixtures.py b/test_runner/fixtures/zenith_fixtures.py index 628447e2ae..fee626b616 100644 --- a/test_runner/fixtures/zenith_fixtures.py +++ b/test_runner/fixtures/zenith_fixtures.py @@ -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 /pgdatadirs//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') diff --git a/test_runner/setup.cfg b/test_runner/setup.cfg index c2c1aa2176..578cb28efc 100644 --- a/test_runner/setup.cfg +++ b/test_runner/setup.cfg @@ -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