rust/neon
1
0
Fork 0
mirror of https://github.com/neondatabase/neon.git synced 2026-01-05 20:42:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

test_runner: fix mypy errors and force it on CI (#774)

Browse Source 
* Fix bugs found by mypy
* Add some missing types and runtime checks, remove unused code
* Make ZenithPageserver start right away for better type safety
* Add `types-*` packages to Pipfile
* Pin mypy version and run it on CircleCI
This commit is contained in:
Egor Suvorov
2021-10-21 13:51:54 +03:00
committed by GitHub
parent 7f9d2a7d05
commit ff563ff080
11 changed files with 112 additions and 79 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
.circleci/config.yml
View File

@@ -194,8 +194,14 @@ jobs:
pipenv install --dev
- run:
name: Run yapf to ensure code format
when: always
working_directory: test_runner
command: pipenv run yapf --recursive --diff .
- run:
name: Run mypy to check types
when: always
working_directory: test_runner
command: pipenv run mypy .
run-pytest:
#description: "Run pytest"

7
test_runner/Pipfile
View File

@@ -14,11 +14,14 @@ asyncpg = "*"
cached-property = "*"
[dev-packages]
flake8 = "*"
mypy = "*"
# Behavior may change slightly between versions. These are run continuously,
# so we pin exact versions to avoid suprising breaks. Update if comfortable.
yapf = "==0.31.0"
mypy = "==0.910"
# Non-pinned packages follow.
flake8 = "*"
types-requests = "*"
types-psycopg2 = "*"
[requires]
# we need at least 3.6, but pipenv doesn't allow to say this directly

18
test_runner/Pipfile.lock generated
View File

@@ -1,7 +1,7 @@
{
"_meta": {
"hash": {
"sha256": "3645ae8d2dcf55bd2a54963c44cfeedf577f3b289d1077365214a80a7f36e643"
"sha256": "48eef51e8eb3c7f0d7284879dd68a83d9b0d9dc835f7a3ff0452b4ddfe9c560a"
},
"pipfile-spec": 6,
"requires": {
@@ -369,6 +369,22 @@
"markers": "python_version >= '2.6' and python_version not in '3.0, 3.1, 3.2, 3.3'",
"version": "==0.10.2"
},
"types-psycopg2": {
"hashes": [
"sha256:77ed80f2668582654623e04fb3d741ecce93effcc39c929d7e02f4a917a538ce",
"sha256:98a6e0e9580cd7eb4bd4d20f7c7063d154b2589a2b90c0ce4e3ca6085cde77c6"
],
"index": "pypi",
"version": "==2.9.1"
},
"types-requests": {
"hashes": [
"sha256:b279284e51f668e38ee12d9665e4d789089f532dc2a0be4a1508ca0efd98ba9e",
"sha256:ba1d108d512e294b6080c37f6ae7cb2a2abf527560e2b671d1786c1fc46b541a"
],
"index": "pypi",
"version": "==2.25.11"
},
"typing-extensions": {
"hashes": [
"sha256:49f75d16ff11f1cd258e1b988ccff82a3ca5570217d7ad8c5f48205dd99a677e",

7
test_runner/README.md
View File

@@ -95,13 +95,16 @@ Python destructors, e.g. `__del__()` aren't recommended for cleanup.
### Code quality
We force code formatting via yapf:
We force code formatting via yapf and type hints via mypy:
1. Install `yapf` and other tools (`flake8`, `mypy`) with `pipenv install --dev`.
1. Reformat all your code by running `pipenv run yapf -ri .` in the `test_runner/` directory.
1. Ensure there are no type errors by running `pipenv run mypy .` in the `test_runner/` directory.
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.
* (Optional) Typechecking the code with `mypy .`. Currently this mostly affects `fixtures/zenith_fixtures.py`.
* Adding more type hints to your code to avoid `Any`, especially:
* For fixture parameters, they are not automatically deduced.
* For function arguments and return values.

2
test_runner/batch_others/test_createdropdb.py
View File

@@ -87,12 +87,14 @@ def test_dropdb(
pg_before.connect(dbname='foodb').close()
# Test that database subdir exists on the branch before drop
assert pg_before.pgdata_dir
dbpath = pathlib.Path(pg_before.pgdata_dir) / 'base' / str(dboid)
log.info(dbpath)
assert os.path.isdir(dbpath) == True
# Test that database subdir doesn't exist on the branch after drop
assert pg_after.pgdata_dir
dbpath = pathlib.Path(pg_after.pgdata_dir) / 'base' / str(dboid)
log.info(dbpath)

7
test_runner/batch_others/test_pageserver_api.py
View File

@@ -3,7 +3,8 @@ from uuid import uuid4
import pytest
import psycopg2
import requests
from fixtures.zenith_fixtures import ZenithPageserver, ZenithPageserverHttpClient
from fixtures.zenith_fixtures import ZenithCli, ZenithPageserver, ZenithPageserverHttpClient
from typing import cast
pytest_plugins = ("fixtures.zenith_fixtures")
@@ -53,7 +54,7 @@ def test_branch_list_psql(pageserver: ZenithPageserver, zenith_cli):
conn.close()
def test_tenant_list_psql(pageserver: ZenithPageserver, zenith_cli):
def test_tenant_list_psql(pageserver: ZenithPageserver, zenith_cli: ZenithCli):
res = zenith_cli.run(["tenant", "list"])
res.check_returncode()
tenants = sorted(map(lambda t: t.split()[0], res.stdout.splitlines()))
@@ -74,7 +75,7 @@ def test_tenant_list_psql(pageserver: ZenithPageserver, zenith_cli):
cur.execute('tenant_list')
# compare tenants list
new_tenants = sorted(map(lambda t: t['id'], json.loads(cur.fetchone()[0])))
new_tenants = sorted(map(lambda t: cast(str, t['id']), json.loads(cur.fetchone()[0])))
assert sorted([pageserver.initial_tenant, tenant1]) == new_tenants

2
test_runner/batch_others/test_readonly_node.py
View File

@@ -84,6 +84,6 @@ def test_readonly_node(zenith_cli, pageserver: ZenithPageserver, postgres: Postg
# Create node at pre-initdb lsn
try:
zenith_cli.run(["pg", "start", "test_branch_preinitdb", "test_readonly_node@0/42"])
assert false, "compute node startup with invalid LSN should have failed"
assert False, "compute node startup with invalid LSN should have failed"
except Exception:
print("Node creation with pre-initdb LSN failed (as expected)")

16
test_runner/batch_others/test_zenith_cli.py
View File

@@ -1,19 +1,24 @@
import json
import uuid
from psycopg2.extensions import cursor as PgCursor
from fixtures.zenith_fixtures import ZenithCli, ZenithPageserver
from typing import cast
pytest_plugins = ("fixtures.zenith_fixtures")
def helper_compare_branch_list(page_server_cur, zenith_cli, initial_tenant: str):
def helper_compare_branch_list(page_server_cur: PgCursor,
zenith_cli: ZenithCli,
initial_tenant: str):
"""
Compare branches list returned by CLI and directly via API.
Filters out branches created by other tests.
"""
page_server_cur.execute(f'branch_list {initial_tenant}')
branches_api = sorted(map(lambda b: b['name'], json.loads(page_server_cur.fetchone()[0])))
branches_api = sorted(
map(lambda b: cast(str, b['name']), json.loads(page_server_cur.fetchone()[0])))
branches_api = [b for b in branches_api if b.startswith('test_cli_') or b in ('empty', 'main')]
res = zenith_cli.run(["branch"])
@@ -32,7 +37,7 @@ def helper_compare_branch_list(page_server_cur, zenith_cli, initial_tenant: str)
assert branches_api == branches_cli == branches_cli_with_tenant_arg
def test_cli_branch_list(pageserver: ZenithPageserver, zenith_cli):
def test_cli_branch_list(pageserver: ZenithPageserver, zenith_cli: ZenithCli):
page_server_conn = pageserver.connect()
page_server_cur = page_server_conn.cursor()
@@ -58,9 +63,10 @@ def test_cli_branch_list(pageserver: ZenithPageserver, zenith_cli):
assert 'test_cli_branch_list_nested' in branches_cli
def helper_compare_tenant_list(page_server_cur, zenith_cli: ZenithCli):
def helper_compare_tenant_list(page_server_cur: PgCursor, zenith_cli: ZenithCli):
page_server_cur.execute(f'tenant_list')
tenants_api = sorted(map(lambda t: t['id'], json.loads(page_server_cur.fetchone()[0])))
tenants_api = sorted(
map(lambda t: cast(str, t['id']), json.loads(page_server_cur.fetchone()[0])))
res = zenith_cli.run(["tenant", "list"])
assert res.stderr == ''

14
test_runner/fixtures/benchmark_fixture.py
View File

@@ -54,16 +54,10 @@ in the test initialization, or measure disk usage after the test query.
"""
# All the results are collected in this list, as a tuple:
# (test_name: str, metric_name: str, metric_value: float, unit: str)
#
# TODO: It would perhaps be better to store the results as additional
# properties in the pytest TestReport objects, to make them visible to
# other pytest tools.
global zenbenchmark_results
zenbenchmark_results = []
class ZenithBenchmarkResults:
""" An object for recording benchmark results. """
def __init__(self):
@@ -77,6 +71,10 @@ class ZenithBenchmarkResults:
self.results.append((test_name, metric_name, metric_value, unit))
# Will be recreated in each session.
zenbenchmark_results: ZenithBenchmarkResults = ZenithBenchmarkResults()
# Session scope fixture that initializes the results object
@pytest.fixture(autouse=True, scope='session')
def zenbenchmark_global(request) -> Iterator[ZenithBenchmarkResults]:
@@ -137,6 +135,7 @@ class ZenithBenchmarker:
matches = re.search(r'^pageserver_disk_io_bytes{io_operation="write"} (\S+)$',
all_metrics,
re.MULTILINE)
assert matches
return int(round(float(matches.group(1))))
def get_peak_mem(self, pageserver) -> int:
@@ -147,6 +146,7 @@ class ZenithBenchmarker:
all_metrics = pageserver.http_client().get_metrics()
# See comment in get_io_writes()
matches = re.search(r'^pageserver_maxrss_kb (\S+)$', all_metrics, re.MULTILINE)
assert matches
return int(round(float(matches.group(1))))
def get_timeline_size(self, repo_dir: str, tenantid: str, timelineid: str):

106
test_runner/fixtures/zenith_fixtures.py
View File

@@ -192,7 +192,7 @@ class ZenithCli:
self.env['ZENITH_REPO_DIR'] = repo_dir
self.env['POSTGRES_DISTRIB_DIR'] = pg_distrib_dir
def run(self, arguments: List[str]) -> Any:
def run(self, arguments: List[str]) -> 'subprocess.CompletedProcess[str]':
"""
Run "zenith" with the specified arguments.
@@ -249,12 +249,14 @@ class ZenithPageserverHttpClient(requests.Session):
def check_status(self):
self.get(f"http://localhost:{self.port}/v1/status").raise_for_status()
def branch_list(self, tenant_id: uuid.UUID) -> List[Dict]:
def branch_list(self, tenant_id: uuid.UUID) -> List[Dict[Any, Any]]:
res = self.get(f"http://localhost:{self.port}/v1/branch/{tenant_id.hex}")
res.raise_for_status()
return res.json()
res_json = res.json()
assert isinstance(res_json, list)
return res_json
def branch_create(self, tenant_id: uuid.UUID, name: str, start_point: str) -> Dict:
def branch_create(self, tenant_id: uuid.UUID, name: str, start_point: str) -> Dict[Any, Any]:
res = self.post(f"http://localhost:{self.port}/v1/branch",
json={
'tenant_id': tenant_id.hex,
@@ -262,17 +264,23 @@ class ZenithPageserverHttpClient(requests.Session):
'start_point': start_point,
})
res.raise_for_status()
return res.json()
res_json = res.json()
assert isinstance(res_json, dict)
return res_json
def branch_detail(self, tenant_id: uuid.UUID, name: str) -> Dict:
def branch_detail(self, tenant_id: uuid.UUID, name: str) -> Dict[Any, Any]:
res = self.get(f"http://localhost:{self.port}/v1/branch/{tenant_id.hex}/{name}", )
res.raise_for_status()
return res.json()
res_json = res.json()
assert isinstance(res_json, dict)
return res_json
def tenant_list(self) -> List[Dict]:
def tenant_list(self) -> List[Dict[Any, Any]]:
res = self.get(f"http://localhost:{self.port}/v1/tenant")
res.raise_for_status()
return res.json()
res_json = res.json()
assert isinstance(res_json, list)
return res_json
def tenant_create(self, tenant_id: uuid.UUID):
res = self.post(
@@ -361,20 +369,23 @@ class PageserverPort:
class ZenithPageserver(PgProtocol):
""" An object representing a running pageserver. """
def __init__(self, zenith_cli: ZenithCli, repo_dir: str, port: PageserverPort):
"""
An object representing a running pageserver.
Initializes the repository via `zenith init` and starts page server.
"""
def __init__(self,
zenith_cli: ZenithCli,
repo_dir: str,
port: PageserverPort,
enable_auth=False):
super().__init__(host='localhost', port=port.pg)
self.zenith_cli = zenith_cli
self.running = False
self.initial_tenant = None
self.initial_tenant: str = cast(str, None) # Will be fixed by self.start() below
self.repo_dir = repo_dir
self.service_port = port # do not shadow PgProtocol.port which is just int
def init(self, enable_auth: bool = False) -> 'ZenithPageserver':
"""
Initialize the repository, i.e. run "zenith init".
Returns self.
"""
cmd = [
'init',
f'--pageserver-pg-port={self.service_port.pg}',
@@ -383,13 +394,8 @@ class ZenithPageserver(PgProtocol):
if enable_auth:
cmd.append('--enable-auth')
self.zenith_cli.run(cmd)
return self
def repo_dir(self) -> str:
"""
Return path to repository dir
"""
return self.zenith_cli.repo_dir
self.start() # Required, otherwise self.initial_tenant is of wrong type
def start(self) -> 'ZenithPageserver':
"""
@@ -401,7 +407,11 @@ class ZenithPageserver(PgProtocol):
self.zenith_cli.run(['start'])
self.running = True
# get newly created tenant id
self.initial_tenant = self.zenith_cli.run(['tenant', 'list']).stdout.split()[0]
current_tenant = self.zenith_cli.run(['tenant', 'list']).stdout.split()[0]
if self.initial_tenant is None:
self.initial_tenant = current_tenant
else:
assert self.initial_tenant == current_tenant
return self
def stop(self, immediate=False) -> 'ZenithPageserver':
@@ -461,8 +471,7 @@ def pageserver(zenith_cli: ZenithCli, repo_dir: str,
By convention, the test branches are named after the tests. For example,
test called 'test_foo' would create and use branches with the 'test_foo' prefix.
"""
ps = ZenithPageserver(zenith_cli=zenith_cli, repo_dir=repo_dir,
port=pageserver_port).init().start()
ps = ZenithPageserver(zenith_cli=zenith_cli, repo_dir=repo_dir, port=pageserver_port)
# For convenience in tests, create a branch from the freshly-initialized cluster.
zenith_cli.run(["branch", "empty", "main"])
@@ -516,7 +525,7 @@ class PgBin:
command: List[str],
env: Optional[Env] = None,
cwd: Optional[str] = None,
**kwargs: Any) -> None:
**kwargs: Any) -> str:
"""
Run one of the postgres binaries, with stderr and stdout redirected to a file.
@@ -537,8 +546,10 @@ def pg_bin(test_output_dir: str, pg_distrib_dir: str) -> PgBin:
@pytest.fixture
def pageserver_auth_enabled(zenith_cli: ZenithCli, repo_dir: str, pageserver_port: PageserverPort):
with ZenithPageserver(zenith_cli=zenith_cli, repo_dir=repo_dir,
port=pageserver_port).init(enable_auth=True).start() as ps:
with ZenithPageserver(zenith_cli=zenith_cli,
repo_dir=repo_dir,
port=pageserver_port,
enable_auth=True) as ps:
# For convenience in tests, create a branch from the freshly-initialized cluster.
zenith_cli.run(["branch", "empty", "main"])
yield ps
@@ -623,6 +634,7 @@ class Postgres(PgProtocol):
def pg_data_dir_path(self) -> str:
""" Path to data directory """
assert self.node_name
path = pathlib.Path('pgdatadirs') / 'tenants' / self.tenant_id / self.node_name
return os.path.join(self.repo_dir, path)
@@ -693,9 +705,9 @@ class Postgres(PgProtocol):
"""
assert self.node_name is not None
assert self.tenant_id is not None
self.zenith_cli.run(
['pg', 'stop', '--destroy', self.node_name, f'--tenantid={self.tenant_id}'])
self.node_name = None
return self
@@ -793,29 +805,6 @@ class PostgresFactory:
config_lines=config_lines,
)
def config(self,
node_name: str = "main",
tenant_id: Optional[str] = None,
wal_acceptors: Optional[str] = None,
config_lines: Optional[List[str]] = None) -> Postgres:
pg = Postgres(
zenith_cli=self.zenith_cli,
repo_dir=self.repo_dir,
pg_bin=self.pg_bin,
tenant_id=tenant_id or self.initial_tenant,
port=self.port_distributor.get_port(),
)
self.num_instances += 1
self.instances.append(pg)
return pg.config(
node_name=node_name,
wal_acceptors=wal_acceptors,
config_lines=config_lines,
)
def stop_all(self) -> 'PostgresFactory':
for pg in self.instances:
pg.stop()
@@ -849,7 +838,7 @@ def postgres(zenith_cli: ZenithCli,
pgfactory.stop_all()
def read_pid(path: Path):
def read_pid(path: Path) -> int:
""" Read content of file into number """
return int(path.read_text())
@@ -955,7 +944,9 @@ class WalAcceptor:
cur.execute("JSON_CTRL " + request_json)
all = cur.fetchall()
log.info(f"JSON_CTRL response: {all[0][0]}")
return json.loads(all[0][0])
res = json.loads(all[0][0])
assert isinstance(res, dict)
return res
def http_client(self):
return WalAcceptorHttpClient(port=self.port.http)
@@ -1211,6 +1202,7 @@ def check_restored_datadir_content(zenith_cli: ZenithCli,
subprocess.check_call(cmd, shell=True)
# list files we're going to compare
assert pg.pgdata_dir
pgdata_files = list_files_to_compare(pg.pgdata_dir)
restored_files = list_files_to_compare(restored_dir_path)
@@ -1237,7 +1229,7 @@ def check_restored_datadir_content(zenith_cli: ZenithCli,
subprocess.run("xxd -b {} > {}.hex ".format(f1, f1), shell=True)
subprocess.run("xxd -b {} > {}.hex ".format(f2, f2), shell=True)
cmd = ['diff {}.hex {}.hex'.format(f1, f2)]
subprocess.run(cmd, stdout=stdout_f, shell=True)
cmd = 'diff {}.hex {}.hex'.format(f1, f2)
subprocess.run([cmd], stdout=stdout_f, shell=True)
assert (mismatch, error) == ([], [])

6
test_runner/setup.cfg
View File

@@ -22,7 +22,11 @@ disallow_untyped_decorators = false
disallow_untyped_defs = false
strict = true
[mypy-psycopg2.*]
[mypy-asyncpg.*]
# There is some work in progress, though: https://github.com/MagicStack/asyncpg/pull/577
ignore_missing_imports = true
[mypy-cached_property.*]
ignore_missing_imports = true
[mypy-pytest.*]