From 12b0e76defb15eebcc1fb2d79200842739e81fe6 Mon Sep 17 00:00:00 2001
From: "Antoine R. Dumont (@ardumont)" <ardumont@softwareheritage.org>
Date: Thu, 29 Oct 2020 14:57:24 +0100
Subject: [PATCH] core.db.cli: Add coverage and ensure `swh db *` works as
 expected

This explicitely checks the different scenario possible with the current
subcommands (create, init-admin, init).

Using a subset of sql schema representation we have in our existing datasets.

Closes T2741
---
 .../db/tests/data/cli/0-superuser-init.sql    |   1 +
 swh/core/db/tests/data/cli/1-schema.sql       |  13 ++
 swh/core/db/tests/data/cli/3-func.sql         |   6 +
 swh/core/db/tests/data/cli/4-data.sql         |   5 +
 swh/core/db/tests/test_cli.py                 | 212 +++++++++++++++++-
 5 files changed, 230 insertions(+), 7 deletions(-)
 create mode 100644 swh/core/db/tests/data/cli/0-superuser-init.sql
 create mode 100644 swh/core/db/tests/data/cli/1-schema.sql
 create mode 100644 swh/core/db/tests/data/cli/3-func.sql
 create mode 100644 swh/core/db/tests/data/cli/4-data.sql

diff --git a/swh/core/db/tests/data/cli/0-superuser-init.sql b/swh/core/db/tests/data/cli/0-superuser-init.sql
new file mode 100644
index 00000000..480018c8
--- /dev/null
+++ b/swh/core/db/tests/data/cli/0-superuser-init.sql
@@ -0,0 +1 @@
+create extension if not exists pgcrypto;
diff --git a/swh/core/db/tests/data/cli/1-schema.sql b/swh/core/db/tests/data/cli/1-schema.sql
new file mode 100644
index 00000000..a5f6d2c3
--- /dev/null
+++ b/swh/core/db/tests/data/cli/1-schema.sql
@@ -0,0 +1,13 @@
+-- schema version table which won't get truncated
+create table if not exists dbversion (
+  version     int primary key,
+  release     timestamptz,
+  description text
+);
+
+-- origin table
+create table if not exists origin (
+  id       bigserial not null,
+  url      text not null,
+  hash     text not null
+);
diff --git a/swh/core/db/tests/data/cli/3-func.sql b/swh/core/db/tests/data/cli/3-func.sql
new file mode 100644
index 00000000..d4dd410b
--- /dev/null
+++ b/swh/core/db/tests/data/cli/3-func.sql
@@ -0,0 +1,6 @@
+create or replace function hash_sha1(text)
+    returns text
+    language sql strict immutable
+as $$
+    select encode(public.digest($1, 'sha1'), 'hex')
+$$;
diff --git a/swh/core/db/tests/data/cli/4-data.sql b/swh/core/db/tests/data/cli/4-data.sql
new file mode 100644
index 00000000..ed29fa1b
--- /dev/null
+++ b/swh/core/db/tests/data/cli/4-data.sql
@@ -0,0 +1,5 @@
+insert into dbversion(version, release, description)
+values (1, '2016-02-22 15:56:28.358587+00', 'Work In Progress');
+
+insert into origin(url, hash)
+values ('https://forge.softwareheritage.org', hash_sha1('https://forge.softwareheritage.org'));
diff --git a/swh/core/db/tests/test_cli.py b/swh/core/db/tests/test_cli.py
index 236d260f..067524ac 100644
--- a/swh/core/db/tests/test_cli.py
+++ b/swh/core/db/tests/test_cli.py
@@ -1,8 +1,24 @@
-#
+# Copyright (C) 2019-2020  The Software Heritage developers
+# See the AUTHORS file at the top-level directory of this distribution
+# License: GNU General Public License version 3, or any later version
+# See top-level LICENSE file for more information
+
+import copy
+import glob
+from os import path
 
 from click.testing import CliRunner
+import pytest
 
 from swh.core.cli.db import db as swhdb
+from swh.core.db import BaseDb
+from swh.core.db.pytest_plugin import postgresql_fact
+
+
+@pytest.fixture
+def cli_runner():
+    return CliRunner()
+
 
 help_msg = """Usage: swh [OPTIONS] COMMAND [ARGS]...
 
@@ -27,10 +43,9 @@ Commands:
 """
 
 
-def test_swh_help(swhmain):
+def test_cli_swh_help(swhmain, cli_runner):
     swhmain.add_command(swhdb)
-    runner = CliRunner()
-    result = runner.invoke(swhmain, ["-h"])
+    result = cli_runner.invoke(swhmain, ["-h"])
     assert result.exit_code == 0
     assert result.output == help_msg
 
@@ -51,9 +66,192 @@ Commands:
 """
 
 
-def test_swh_db_help(swhmain):
+def test_cli_swh_db_help(swhmain, cli_runner):
     swhmain.add_command(swhdb)
-    runner = CliRunner()
-    result = runner.invoke(swhmain, ["db", "-h"])
+    result = cli_runner.invoke(swhmain, ["db", "-h"])
     assert result.exit_code == 0
     assert result.output == help_db_msg
+
+
+@pytest.fixture()
+def mock_package_sql(mocker, datadir):
+    """This bypasses the module manipulation to only returns the data test files.
+
+    """
+    from swh.core.utils import numfile_sortkey as sortkey
+
+    mock_sql_files = mocker.patch("swh.core.cli.db.get_sql_for_package")
+    sql_files = sorted(glob.glob(path.join(datadir, "cli", "*.sql")), key=sortkey)
+    mock_sql_files.return_value = sql_files
+    return mock_sql_files
+
+
+# We do not want the truncate behavior for those tests
+test_db = postgresql_fact(
+    "postgresql_proc", db_name="clidb", no_truncate_tables={"dbversion", "origin"}
+)
+
+
+@pytest.fixture
+def swh_db_cli(cli_runner, monkeypatch, test_db):
+    """This initializes a cli_runner and sets the correct environment variable expected by
+       the cli to run appropriately (when not specifying the --db-name flag)
+
+    """
+    db_params = test_db.get_dsn_parameters()
+    monkeypatch.setenv("PGHOST", db_params["host"])
+    monkeypatch.setenv("PGUSER", db_params["user"])
+    monkeypatch.setenv("PGPORT", db_params["port"])
+
+    return cli_runner, db_params
+
+
+def craft_conninfo(test_db, dbname=None) -> str:
+    """Craft conninfo string out of the test_db object. This also allows to override the
+    dbname."""
+    db_params = test_db.get_dsn_parameters()
+    if dbname:
+        params = copy.deepcopy(db_params)
+        params["dbname"] = dbname
+    else:
+        params = db_params
+    return "postgresql://{user}@{host}:{port}/{dbname}".format(**params)
+
+
+def test_cli_swh_db_create_and_init_db(cli_runner, test_db, mock_package_sql):
+    """Create a db then initializing it should be ok
+
+    """
+    module_name = "something"
+
+    conninfo = craft_conninfo(test_db, "new-db")
+    # This creates the db and installs the necessary admin extensions
+    result = cli_runner.invoke(swhdb, ["create", module_name, "--db-name", conninfo])
+    assert result.exit_code == 0, f"Unexpected output: {result.output}"
+
+    # This initializes the schema and data
+    result = cli_runner.invoke(swhdb, ["init", module_name, "--db-name", conninfo])
+
+    assert result.exit_code == 0, f"Unexpected output: {result.output}"
+
+    # the origin value in the scripts uses a hash function (which implementation wise
+    # uses a function from the pgcrypt extension, installed during db creation step)
+    with BaseDb.connect(conninfo).cursor() as cur:
+        cur.execute("select * from origin")
+        origins = cur.fetchall()
+        assert len(origins) == 1
+
+
+def test_cli_swh_db_initialization_fail_without_creation_first(
+    cli_runner, test_db, mock_package_sql
+):
+    """Init command on an inexisting db cannot work
+
+    """
+    module_name = "anything"  # it's mocked here
+    conninfo = craft_conninfo(test_db, "inexisting-db")
+
+    result = cli_runner.invoke(swhdb, ["init", module_name, "--db-name", conninfo])
+    # Fails because we cannot connect to an inexisting db
+    assert result.exit_code == 1, f"Unexpected output: {result.output}"
+
+
+def test_cli_swh_db_initialization_fail_without_extension(
+    cli_runner, test_db, mock_package_sql
+):
+    """Init command cannot work without privileged extension.
+
+       In this test, the schema needs privileged extension to work.
+
+    """
+    module_name = "anything"  # it's mocked here
+    conninfo = craft_conninfo(test_db)
+
+    result = cli_runner.invoke(swhdb, ["init", module_name, "--db-name", conninfo])
+    # Fails as the function `public.digest` is not installed, init-admin calls is needed
+    # first (the next tests show such behavior)
+    assert result.exit_code == 1, f"Unexpected output: {result.output}"
+
+
+def test_cli_swh_db_initialization_works_with_flags(
+    cli_runner, test_db, mock_package_sql
+):
+    """Init commands with carefully crafted libpq conninfo works
+
+    """
+    module_name = "anything"  # it's mocked here
+    conninfo = craft_conninfo(test_db)
+
+    result = cli_runner.invoke(
+        swhdb, ["init-admin", module_name, "--db-name", conninfo]
+    )
+    assert result.exit_code == 0, f"Unexpected output: {result.output}"
+
+    result = cli_runner.invoke(swhdb, ["init", module_name, "--db-name", conninfo])
+
+    assert result.exit_code == 0, f"Unexpected output: {result.output}"
+    # the origin values in the scripts uses a hash function (which implementation wise
+    # uses a function from the pgcrypt extension, init-admin calls installs it)
+    with BaseDb.connect(test_db.dsn).cursor() as cur:
+        cur.execute("select * from origin")
+        origins = cur.fetchall()
+        assert len(origins) == 1
+
+
+def test_cli_swh_db_initialization_with_env(swh_db_cli, mock_package_sql, test_db):
+    """Init commands with standard environment variables works
+
+    """
+    module_name = "anything"  # it's mocked here
+    cli_runner, db_params = swh_db_cli
+    result = cli_runner.invoke(
+        swhdb, ["init-admin", module_name, "--db-name", db_params["dbname"]]
+    )
+    assert result.exit_code == 0, f"Unexpected output: {result.output}"
+
+    result = cli_runner.invoke(
+        swhdb, ["init", module_name, "--db-name", db_params["dbname"]]
+    )
+
+    assert result.exit_code == 0, f"Unexpected output: {result.output}"
+    # the origin values in the scripts uses a hash function (which implementation wise
+    # uses a function from the pgcrypt extension, init-admin calls installs it)
+    with BaseDb.connect(test_db.dsn).cursor() as cur:
+        cur.execute("select * from origin")
+        origins = cur.fetchall()
+        assert len(origins) == 1
+
+
+def test_cli_swh_db_initialization_idempotent(swh_db_cli, mock_package_sql, test_db):
+    """Multiple runs of the init commands are idempotent
+
+    """
+    module_name = "anything"  # mocked
+    cli_runner, db_params = swh_db_cli
+
+    result = cli_runner.invoke(
+        swhdb, ["init-admin", module_name, "--db-name", db_params["dbname"]]
+    )
+    assert result.exit_code == 0, f"Unexpected output: {result.output}"
+
+    result = cli_runner.invoke(
+        swhdb, ["init", module_name, "--db-name", db_params["dbname"]]
+    )
+    assert result.exit_code == 0, f"Unexpected output: {result.output}"
+
+    result = cli_runner.invoke(
+        swhdb, ["init-admin", module_name, "--db-name", db_params["dbname"]]
+    )
+    assert result.exit_code == 0, f"Unexpected output: {result.output}"
+
+    result = cli_runner.invoke(
+        swhdb, ["init", module_name, "--db-name", db_params["dbname"]]
+    )
+    assert result.exit_code == 0, f"Unexpected output: {result.output}"
+
+    # the origin values in the scripts uses a hash function (which implementation wise
+    # uses a function from the pgcrypt extension, init-admin calls installs it)
+    with BaseDb.connect(test_db.dsn).cursor() as cur:
+        cur.execute("select * from origin")
+        origins = cur.fetchall()
+        assert len(origins) == 1
-- 
GitLab