improve io-tests

Tests are now run through unix socket by default allowing
parallel execution. Test setup configuration is done via
environment variables instead of config files.

nix/README.md

@@ -160,7 +160,21 @@ $ nix-shell --run postgrest-test-spec-all
 
 # Run the tests against a specific version of PostgreSQL (use tab-completion in
 # nix-shell to see all available versions):
-$ nix-shell --run postgrest-test-spec-postgresql-9.5
+$ nix-shell --run postgrest-test-spec-postgresql-13
+
+```
+
+The io-test that test PostgREST as a black box with inputs and outputs can be
+run with `postgrest-test-io`. The test runner under the hood is
+[pytest](https://docs.pytest.org/) and you can pass it the usual options:
+
+```bash
+# Filter the tests to run by name, including all that contain 'config':
+postgrest-test-io -k config
+
+# Run tests in parallel using xdist, specifying the number of processes:
+postgrest-test-io -n auto
+postgrest-test-io -n 8
 
 ```
 

nix/tests.nix

@@ -90,11 +90,12 @@ let
 
   ioTestPython =
     python3.withPackages (ps: [
+      ps.pyjwt
       ps.pytest
+      ps.pytest_xdist
+      ps.pyyaml
       ps.requests
       ps.requests-unixsocket
-      ps.pyjwt
-      ps.pyyaml
     ]);
 
   testIO =

test/io-tests/configs/app-settings.config

@@ -1,6 +1,4 @@
 db-pool = 1
 db-pool-timeout = 1
-server-host = "127.0.0.1"
-server-port = 49421
 
 app.settings.external_api_secret = "0123456789abcdef"

test/io-tests/configs/base64-secret-from-file.config

@@ -1,6 +1,4 @@
 db-pool = 1
-server-host = "127.0.0.1"
-server-port = 49421
 
 # Read secret from a file: /dev/stdin (alias for standard input)
 jwt-secret = "@/dev/stdin"

test/io-tests/configs/dburi-from-file.config

@@ -1,5 +1,3 @@
 db-uri = "@/dev/stdin"
 db-pool = 1
-server-host = "127.0.0.1"
-server-port = 49421
 jwt-secret = "reallyreallyreallyreallyverysafe"

test/io-tests/configs/role-claim-key.config

@@ -1,5 +1,3 @@
 db-pool = 1
-server-host = "127.0.0.1"
-server-port = 49421
 jwt-role-claim-key = "$(ROLE_CLAIM_KEY)"
 jwt-secret = "reallyreallyreallyreallyverysafe"

test/io-tests/configs/secret-from-file.config

@@ -1,6 +1,4 @@
 db-pool = 1
-server-host = "127.0.0.1"
-server-port = 49421
 
 # Read secret from a file: /dev/stdin (alias for standard input)
 jwt-secret = "@/dev/stdin"

test/io-tests/configs/sigusr2-settings.config

@@ -1,7 +1,5 @@
 db-schemas = "test"
 db-pool = 1
-server-host = "127.0.0.1"
-server-port = 49421
 
 app.settings.name_var = "John"
 jwt-secret = "invalidinvalidinvalidinvalidinvalid"

test/io-tests/configs/simple.config

@@ -1,4 +1,2 @@
 db-pool = 1
-server-host = "127.0.0.1"
-server-port = 49421
 jwt-secret = "reallyreallyreallyreallyverysafe"

test/io-tests/test_io.py

@@ -3,11 +3,14 @@
 import contextlib
 import dataclasses
 from datetime import datetime
-import pathlib
-import subprocess
 from operator import attrgetter
 import os
+import pathlib
+import shutil
 import signal
+import socket
+import subprocess
+import tempfile
 import time
 import urllib.parse
 
@@ -21,7 +24,7 @@ import yaml
 BASEDIR = pathlib.Path(os.path.realpath(__file__)).parent
 CONFIGSDIR = BASEDIR / "configs"
 FIXTURES = yaml.load((BASEDIR / "fixtures.yaml").read_text(), Loader=yaml.Loader)
-BASEURL = "http://127.0.0.1:49421"
+POSTGREST_BIN = shutil.which("postgrest")
 SECRET = "reallyreallyreallyreallyverysafe"
 
 
@@ -41,7 +44,9 @@ class PostgrestSession(requests_unixsocket.Session):
         self.baseurl = baseurl
 
     def request(self, method, url, *args, **kwargs):
-        fullurl = urllib.parse.urljoin(self.baseurl, url)
+        # Not using urllib.parse.urljoin to compose the url, as it doesn't play
+        # well with our 'http+unix://' unix domain socket urls.
+        fullurl = self.baseurl + url
         return super(PostgrestSession, self).request(method, fullurl, *args, **kwargs)
 
 
@@ -58,25 +63,27 @@ def dburi():
     return os.getenv("PGRST_DB_URI").encode("utf-8")
 
 
-def mkenv(moreenv):
-    """
-    Create env from os.environ and moreenv, while
-    filtering None values to allow overriding "unset".
-    """
-    env = {**os.environ, **(moreenv or {})}
-    return {k: v for k, v in env.items() if v is not None}
+@pytest.fixture
+def defaultenv():
+    "Default environment for PostgREST."
+    return {
+        "PGRST_DB_URI": os.environ["PGRST_DB_URI"],
+        "PGRST_DB_SCHEMAS": os.environ["PGRST_DB_SCHEMAS"],
+        "PGRST_DB_ANON_ROLE": os.environ["PGRST_DB_ANON_ROLE"],
+    }
 
 
-def dumpconfig(configpath=None, moreenv=None, stdin=None):
+def dumpconfig(configpath=None, env=None, stdin=None):
     "Dump the config as parsed by PostgREST."
+    command = [POSTGREST_BIN, "--dump-config"]
 
-    command = ["postgrest", "--dump-config"]
     if configpath:
-        command += [configpath]
+        command.append(configpath)
 
     process = subprocess.Popen(
-        command, env=mkenv(moreenv), stdin=subprocess.PIPE, stdout=subprocess.PIPE
+        command, env=env or {}, stdin=subprocess.PIPE, stdout=subprocess.PIPE
     )
+
     process.stdin.write(stdin or b"")
     result = process.communicate()[0]
     process.kill()
@@ -87,27 +94,44 @@ def dumpconfig(configpath=None, moreenv=None, stdin=None):
 
 
 @contextlib.contextmanager
-def run(configpath, stdin=None, moreenv=None, socket=None):
+def run(configpath=None, stdin=None, env=None, port=None):
     "Run PostgREST and yield an endpoint that is ready for connections."
 
-    if socket:
-        baseurl = "http+unix://" + urllib.parse.quote_plus(str(socket))
-    else:
-        baseurl = BASEURL
+    with tempfile.TemporaryDirectory() as tmpdir:
+        if port:
+            env["PGRST_SERVER_PORT"] = str(port)
+            env["PGRST_SERVER_HOST"] = "localhost"
+            baseurl = f"http://localhost:{port}"
+        else:
+            socketfile = pathlib.Path(tmpdir) / "postgrest.sock"
+            env["PGRST_SERVER_UNIX_SOCKET"] = str(socketfile)
+            baseurl = "http+unix://" + urllib.parse.quote_plus(str(socketfile))
 
-    command = ["postgrest", configpath]
-    process = subprocess.Popen(command, stdin=subprocess.PIPE, env=mkenv(moreenv))
+        command = [POSTGREST_BIN]
 
-    try:
-        process.stdin.write(stdin or b"")
-        process.stdin.close()
+        if configpath:
+            command.append(configpath)
 
-        wait_until_ready(baseurl)
+        process = subprocess.Popen(command, stdin=subprocess.PIPE, env=env or {})
 
-        yield PostgrestProcess(process=process, session=PostgrestSession(baseurl))
-    finally:
-        process.kill()
-        process.wait()
+        try:
+            process.stdin.write(stdin or b"")
+            process.stdin.close()
+
+            wait_until_ready(baseurl)
+
+            yield PostgrestProcess(process=process, session=PostgrestSession(baseurl))
+        finally:
+            process.kill()
+            process.wait()
+
+
+def freeport():
+    "Find a free port on localhost."
+    with contextlib.closing(socket.socket(socket.AF_INET, socket.SOCK_STREAM)) as s:
+        s.bind(("", 0))
+        s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+        return s.getsockname()[1]
 
 
 def wait_until_ready(url):
@@ -153,8 +177,7 @@ def test_expected_config(expectedconfig):
     expected = expectedconfig.read_text()
     config = CONFIGSDIR / expectedconfig.name
 
-    unset = {"PGRST_DB_URI": None, "PGRST_DB_ANON_ROLE": None, "PGRST_DB_SCHEMAS": None}
-    assert dumpconfig(config, moreenv=unset) == expected
+    assert dumpconfig(config) == expected
 
 
 def test_expected_config_from_environment():
@@ -164,7 +187,7 @@ def test_expected_config_from_environment():
     env = {k: str(v) for k, v in yaml.load(envfile, Loader=yaml.Loader).items()}
 
     expected = (CONFIGSDIR / "expected" / "no-defaults.config").read_text()
-    assert dumpconfig(moreenv=env) == expected
+    assert dumpconfig(env=env) == expected
 
 
 @pytest.mark.parametrize(
@@ -172,7 +195,7 @@ def test_expected_config_from_environment():
     [conf for conf in CONFIGSDIR.iterdir() if conf.suffix == ".config"],
     ids=attrgetter("name"),
 )
-def test_stable_config(tmp_path, config):
+def test_stable_config(tmp_path, config, defaultenv):
     """
     A dumped, re-read and re-dumped config should match the dumped config.
 
@@ -184,30 +207,27 @@ def test_stable_config(tmp_path, config):
     # Set environment variables that some of the configs expect. Using a
     # complex ROLE_CLAIM_KEY to make sure quoting works.
     env = {
+        **defaultenv,
         "ROLE_CLAIM_KEY": '."https://www.example.com/roles"[0].value',
         "POSTGREST_TEST_SOCKET": "/tmp/postgrest.sock",
+        "POSTGREST_TEST_PORT": "80",
     }
 
     # Some configs expect input from stdin, at least on base64.
     stdin = b"Y29ubmVjdGlvbl9zdHJpbmc="
 
-    dumped = dumpconfig(config, moreenv=env, stdin=stdin)
+    dumped = dumpconfig(config, env=env, stdin=stdin)
 
     tmpconfigpath = tmp_path / "config"
     tmpconfigpath.write_text(dumped)
-    redumped = dumpconfig(tmpconfigpath, moreenv=env)
+    redumped = dumpconfig(tmpconfigpath, env=env)
 
     assert dumped == redumped
 
 
-def test_socket_connection(tmp_path):
-    "Connections via unix domain sockets should work."
-    socket = tmp_path / "postgrest.sock"
-    env = {
-        "POSTGREST_TEST_SOCKET": str(socket),
-    }
-
-    with run(CONFIGSDIR / "unix-socket.config", socket=socket, moreenv=env):
+def test_port_connection(defaultenv):
+    "Connections via a port on localhost should work."
+    with run(env=defaultenv, port=freeport()):
         pass
 
 
@@ -216,7 +236,7 @@ def test_socket_connection(tmp_path):
     [path for path in (BASEDIR / "secrets").iterdir() if path.suffix != ".jwt"],
     ids=attrgetter("name"),
 )
-def test_read_secret_from_file(secretpath):
+def test_read_secret_from_file(secretpath, defaultenv):
     "Authorization should succeed when the secret is read from a file."
     if secretpath.suffix == ".b64":
         configfile = CONFIGSDIR / "base64-secret-from-file.config"
@@ -226,53 +246,59 @@ def test_read_secret_from_file(secretpath):
     secret = secretpath.read_bytes()
     headers = authheader(secretpath.with_suffix(".jwt").read_text())
 
-    with run(configfile, stdin=secret) as postgrest:
+    with run(configfile, stdin=secret, env=defaultenv) as postgrest:
         response = postgrest.session.get("/authors_only", headers=headers)
         assert response.status_code == 200
 
 
-def test_read_dburi_from_file_without_eol(dburi):
+def test_read_dburi_from_file_without_eol(dburi, defaultenv):
     "Reading the dburi from a file with a single line should work."
     config = CONFIGSDIR / "dburi-from-file.config"
-    unset = {"PGRST_DB_URI": None}
-    with run(config, moreenv=unset, stdin=dburi):
+    env = {key: value for key, value in defaultenv.items() if key != "PGRST_DB_URI"}
+    with run(config, env=env, stdin=dburi):
         pass
 
 
-def test_read_dburi_from_file_with_eol(dburi):
+def test_read_dburi_from_file_with_eol(dburi, defaultenv):
     "Reading the dburi from a file containing a newline should work."
     config = CONFIGSDIR / "dburi-from-file.config"
-    unset = {"PGRST_DB_URI": None}
-    with run(config, moreenv=unset, stdin=dburi + b"\n"):
+    env = {key: value for key, value in defaultenv.items() if key != "PGRST_DB_URI"}
+    with run(config, env=env, stdin=dburi + b"\n"):
         pass
 
 
 @pytest.mark.parametrize(
     "roleclaim", FIXTURES["roleclaims"], ids=lambda claim: claim["key"]
 )
-def test_role_claim_key(roleclaim):
+def test_role_claim_key(roleclaim, defaultenv):
     "Authorization should depend on a correct role-claim-key and JWT claim."
-    env = {"ROLE_CLAIM_KEY": roleclaim["key"]}
+    env = {
+        **defaultenv,
+        "ROLE_CLAIM_KEY": roleclaim["key"],
+    }
     headers = jwtauthheader(roleclaim["data"], SECRET)
 
-    with run(CONFIGSDIR / "role-claim-key.config", moreenv=env) as postgrest:
+    with run(CONFIGSDIR / "role-claim-key.config", env=env) as postgrest:
         response = postgrest.session.get("/authors_only", headers=headers)
         assert response.status_code == roleclaim["expected_status"]
 
 
 @pytest.mark.parametrize("invalidroleclaimkey", FIXTURES["invalidroleclaimkeys"])
-def test_invalid_role_claim_key(invalidroleclaimkey):
+def test_invalid_role_claim_key(invalidroleclaimkey, defaultenv):
     "Given an invalid role-claim-key, Postgrest should exit with a non-zero exit code."
-    env = {"ROLE_CLAIM_KEY": invalidroleclaimkey}
+    env = {
+        **defaultenv,
+        "ROLE_CLAIM_KEY": invalidroleclaimkey,
+    }
 
     with pytest.raises(PostgrestError):
-        dump = dumpconfig(CONFIGSDIR / "role-claim-key.config", moreenv=env)
+        dump = dumpconfig(CONFIGSDIR / "role-claim-key.config", env=env)
         for line in dump.split("\n"):
             if line.startswith("jwt-role-claim-key"):
                 print(line)
 
 
-def test_iat_claim():
+def test_iat_claim(defaultenv):
     """
     A claim with an 'iat' (issued at) attribute should be successful.
 
@@ -283,7 +309,7 @@ def test_iat_claim():
     claim = {"role": "postgrest_test_author", "iat": datetime.utcnow()}
     headers = jwtauthheader(claim, SECRET)
 
-    with run(CONFIGSDIR / "simple.config") as postgrest:
+    with run(CONFIGSDIR / "simple.config", env=defaultenv) as postgrest:
         for _ in range(10):
             response = postgrest.session.get("/authors_only", headers=headers)
             assert response.status_code == 200
@@ -291,14 +317,14 @@ def test_iat_claim():
             time.sleep(0.5)
 
 
-def test_app_settings():
+def test_app_settings(defaultenv):
     """
     App settings should not reset when the db pool times out.
 
     See: https://github.com/PostgREST/postgrest/issues/1141
 
     """
-    with run(CONFIGSDIR / "app-settings.config") as postgrest:
+    with run(CONFIGSDIR / "app-settings.config", env=defaultenv) as postgrest:
         # Wait for the db pool to time out, set to 1s in config
         time.sleep(2)
 
@@ -309,14 +335,14 @@ def test_app_settings():
         assert response.text == '"0123456789abcdef"'
 
 
-def test_app_settings_reload(tmp_path):
+def test_app_settings_reload(tmp_path, defaultenv):
     "App settings should be reloaded when PostgREST is sent SIGUSR2."
     config = (CONFIGSDIR / "sigusr2-settings.config").read_text()
     configfile = tmp_path / "test.config"
     configfile.write_text(config)
     uri = "/rpc/get_guc_value?name=app.settings.name_var"
 
-    with run(configfile) as postgrest:
+    with run(configfile, env=defaultenv) as postgrest:
         response = postgrest.session.get(uri)
         assert response.status_code == 200
         assert response.text == '"John"'
@@ -333,7 +359,7 @@ def test_app_settings_reload(tmp_path):
         assert response.text == '"Jane"'
 
 
-def test_jwt_secret_reload(tmp_path):
+def test_jwt_secret_reload(tmp_path, defaultenv):
     "JWT secret should be reloaded when PostgREST is sent SIGUSR2."
     config = (CONFIGSDIR / "sigusr2-settings.config").read_text()
     configfile = tmp_path / "test.config"
@@ -341,7 +367,7 @@ def test_jwt_secret_reload(tmp_path):
 
     headers = jwtauthheader({"role": "postgrest_test_author"}, SECRET)
 
-    with run(configfile) as postgrest:
+    with run(configfile, env=defaultenv) as postgrest:
         response = postgrest.session.get("/authors_only", headers=headers)
         assert response.status_code == 401
 
@@ -357,16 +383,16 @@ def test_jwt_secret_reload(tmp_path):
         assert response.status_code == 200
 
 
-def test_db_schema_reload(tmp_path):
+def test_db_schema_reload(tmp_path, defaultenv):
     "DB schema should be reloaded when PostgREST is sent SIGUSR2."
     config = (CONFIGSDIR / "sigusr2-settings.config").read_text()
     configfile = tmp_path / "test.config"
     configfile.write_text(config)
 
     headers = {"Accept-Profile": "v1"}
-    unset = {"PGRST_DB_SCHEMAS": None}
+    env = {key: value for key, value in defaultenv.items() if key != "PGRST_DB_SCHEMAS"}
 
-    with run(configfile, moreenv=unset) as postgrest:
+    with run(configfile, env=env) as postgrest:
         response = postgrest.session.get("/parents", headers=headers)
         assert response.status_code == 404
 

test/memory-tests.sh

@@ -5,10 +5,12 @@
 
 set -eu
 
+pgrPort=49421
+
 # PGRST_DB_URI, PGRST_DB_ANON_ROLE and PGRST_DB_SCHEMAS are expected to be set by with_tmp_db
 export PGRST_DB_POOL="1"
 export PGRST_SERVER_HOST="127.0.0.1"
-export PGRST_SERVER_PORT="49421"
+export PGRST_SERVER_PORT="$pgrPort"
 export PGRST_JWT_SECRET="reallyreallyreallyreallyverysafe"
 
 trap "kill 0" int term exit
@@ -19,8 +21,6 @@ result(){ echo "$1 $currentTest $2"; currentTest=$(( $currentTest + 1 )); }
 ok(){ result 'ok' "- $1"; }
 ko(){ result 'not ok' "- $1"; failedTests=$(( $failedTests + 1 )); }
 
-pgrPort=49421
-
 pgrStart(){ postgrest +RTS -p -h > /dev/null & pgrPID="$!"; }
 pgrStop(){ kill "$pgrPID" 2>/dev/null; }
 

test/with_tmp_db

@@ -56,7 +56,7 @@ export PGDATA="$tmpdir/db"
 export PGHOST="$tmpdir/socket"
 export PGUSER=postgrest_test_authenticator
 export PGDATABASE=postgres
-export DB_URI="postgresql://$PGDATABASE?host=$PGHOST&user=$PGUSER"
+export DB_URI="postgresql:///$PGDATABASE?host=$PGHOST&user=$PGUSER"
 export PGRST_DB_URI="$DB_URI"
 export PGRST_DB_SCHEMAS="test"
 export PGRST_DB_ANON_ROLE="postgrest_test_anonymous"