tests: remove string templating in SQL statements (#6631)

* Closes #6630
* Didn't find any functional breakages with newer versions of sqlite3
  (tested with 3.49.1).
* Fixed failing tests by moving away from templating variables into
  SQL statements.

Author: oliver-sanders

tests/integration/test_workflow_db_mgr.py

@@ -71,11 +71,11 @@ def db_remove_column(schd: 'Scheduler', table: str, column: str) -> None:
             [fields[1] for fields in desc if fields[1] != column]
         )
         # Copy table data to a temporary table, and rename it back.
-        conn.execute(rf'CREATE TABLE "tmp"({c_names})')
+        conn.execute(rf"CREATE TABLE 'tmp'({c_names})")
         conn.execute(
-            rf'INSERT INTO "tmp"({c_names}) SELECT {c_names} FROM {table}')
-        conn.execute(rf'DROP TABLE "{table}"')
-        conn.execute(rf'ALTER TABLE "tmp" RENAME TO "{table}"')
+            rf"INSERT INTO 'tmp'({c_names}) SELECT {c_names} FROM {table}")
+        conn.execute(rf"DROP TABLE '{table}'")
+        conn.execute(rf"ALTER TABLE 'tmp' RENAME TO '{table}'")
         conn.commit()
 
 

tests/unit/test_db_compat.py

@@ -33,7 +33,7 @@
 @pytest.fixture
 def _setup_db(tmp_path):
     """Fixture to create old DB."""
-    def _inner(values, db_file_name='sql.db'):
+    def _inner(stmts, db_file_name='sql.db'):
         db_file = tmp_path / db_file_name
         db_file.parent.mkdir(parents=True, exist_ok=True)
         # Note: cannot use CylcWorkflowDAO here as creating outdated DB
@@ -53,8 +53,8 @@ def _inner(values, db_file_name='sql.db'):
             r' job_runner_name TEXT, job_id TEXT,'
             r' PRIMARY KEY(cycle, name, submit_num));'
         ))
-        for value in values:
-            conn.execute(value)
+        for stmt, values in stmts:
+            conn.execute(stmt, values)
         conn.execute((
             r"INSERT INTO task_jobs VALUES"
             r"    ('10090101T0000Z', 'foo', 1, 0, 1, '2022-12-05T14:46:06Z',"
@@ -69,13 +69,21 @@ def _inner(values, db_file_name='sql.db'):
 
 
 def test_upgrade_pre_810_fails_on_multiple_flows(_setup_db):
-    values = [(
-        r'INSERT INTO task_states VALUES'
-        r"    ('foo', '10050101T0000Z', '[1, 3]',"
-        r" '2022-12-05T14:46:33Z',"
-        r" '2022-12-05T14:46:40Z', 1, 'succeeded', 0, 0)"
+    stmts = [(
+        r'INSERT INTO task_states VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)',
+        (
+            'foo',
+            '10050101T0000Z',
+            '[1, 3]',
+            '2022-12-05T14:46:33Z',
+            '2022-12-05T14:46:40Z',
+            1,
+            'succeeded',
+            0,
+            0,
+        ),
     )]
-    db_file_name = _setup_db(values)
+    db_file_name = _setup_db(stmts)
     with CylcWorkflowDAO(db_file_name) as dao, pytest.raises(
         CylcError,
         match='^Cannot .* 8.0.x to 8.1.0 .* used.$'
@@ -84,13 +92,21 @@ def test_upgrade_pre_810_fails_on_multiple_flows(_setup_db):
 
 
 def test_upgrade_pre_810_pass_on_single_flow(_setup_db):
-    values = [(
-        r'INSERT INTO task_states VALUES'
-        r"    ('foo', '10050101T0000Z', '[1]',"
-        r" '2022-12-05T14:46:33Z',"
-        r" '2022-12-05T14:46:40Z', 1, 'succeeded', 0, 0)"
+    stmts = [(
+        r'INSERT INTO task_states VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)',
+        (
+            'foo',
+            '10050101T0000Z',
+            '[1]',
+            '2022-12-05T14:46:33Z',
+            '2022-12-05T14:46:40Z',
+            1,
+            'succeeded',
+            0,
+            0,
+        ),
     )]
-    db_file_name = _setup_db(values)
+    db_file_name = _setup_db(stmts)
     with CylcWorkflowDAO(db_file_name) as dao:
         WorkflowDatabaseManager.upgrade_pre_810(dao)
         result = dao.connect().execute(
@@ -104,14 +120,21 @@ def test_check_workflow_db_compat(_setup_db, capsys):
     """
     # Create public and private databases with different cylc versions:
     create = r'CREATE TABLE workflow_params(key TEXT, value TEXT)'
-    insert = (
-        r'INSERT INTO workflow_params VALUES'
-        r'("cylc_version", "{}")'
-    )
+    insert = r'INSERT INTO workflow_params VALUES (?, ?)'
     pri_path = _setup_db(
-        [create, insert.format('7.99.99')], db_file_name='private/db')
+        [
+            (create, tuple()),
+            (insert, ('cylc_version', '7.99.99')),
+        ],
+        db_file_name='private/db',
+    )
     pub_path = _setup_db(
-        [create, insert.format('7.99.98')], db_file_name='public/db')
+        [
+            (create, tuple()),
+            (insert, ('cylc_version', '7.99.98')),
+        ],
+        db_file_name='public/db'
+    )
 
     with pytest.raises(ServiceFileError, match='99.98'):
         WorkflowDatabaseManager.check_db_compatibility(pub_path)
@@ -124,11 +147,11 @@ def test_cylc_7_db_wflow_params_table(_setup_db):
     """Test back-compat needed by workflow state xtrigger for Cylc 7 DBs."""
     ptformat = "CCYY"
     create = r'CREATE TABLE suite_params(key TEXT, value TEXT)'
-    insert = (
-        r'INSERT INTO suite_params VALUES'
-        rf'("cycle_point_format", "{ptformat}")'
-    )
-    db_file_name = _setup_db([create, insert])
+    insert = r'INSERT INTO suite_params VALUES (?, ?)'
+    db_file_name = _setup_db([
+        (create, tuple()),
+        (insert, ('cycle_point_format', ptformat)),
+    ])
     with CylcWorkflowDBChecker('foo', 'bar', db_path=db_file_name) as checker:
         with pytest.raises(
             sqlite3.OperationalError, match="no such table: workflow_params"
@@ -145,29 +168,48 @@ def test_pre_830_task_action_timers(_setup_db):
     index 1. TaskPool.load_db_task_action_timers() should be able to
     discard this field.
     """
-    values = [
-        r'''
-            CREATE TABLE task_action_timers(
-                cycle TEXT, name TEXT, ctx_key TEXT, ctx TEXT, delays TEXT,
-                num INTEGER, delay TEXT, timeout TEXT,
-                PRIMARY KEY(cycle, name, ctx_key)
-            );
-        ''',
-        r'''
-            INSERT INTO task_action_timers VALUES(
-                '1','foo','[["event-mail", "failed"], 9]',
-                '["TaskEventMailContext", ["event-mail", "event-mail", "[email protected]", "jfaden"]]',
-                '[0.0]',1,'0.0','1709229449.61275'
-            );
-        ''',
-        r'''
-            INSERT INTO task_action_timers VALUES(
-                '1','foo','["try_timers", "execution-retry"]', null,
-                '[94608000.0]',1,NULL,NULL
-            );
-        ''',
+    stmts = [
+        (
+            r'''
+                CREATE TABLE task_action_timers(
+                    cycle TEXT, name TEXT, ctx_key TEXT, ctx TEXT, delays TEXT,
+                    num INTEGER, delay TEXT, timeout TEXT,
+                    PRIMARY KEY(cycle, name, ctx_key)
+                );
+            ''',
+            tuple(),
+        ),
+        (
+            r'INSERT INTO task_action_timers VALUES (?, ?, ?, ?, ?, ?, ?, ?)',
+            (
+                '1',
+                'foo',
+                '[["event-mail", "failed"], 9]',
+                (
+                    '["TaskEventMailContext", ["event-mail", "event-mail",'
+                    ' "[email protected]", "jfaden"]]'
+                ),
+                '[0.0]',
+                1,
+                '0.0',
+                '1709229449.61275',
+            )
+        ),
+        (
+            r'INSERT INTO task_action_timers VALUES (?, ?, ?, ?, ?, ?, ?, ?)',
+            (
+                '1',
+                'foo',
+                '["try_timers", "execution-retry"]',
+                None,
+                '[94608000.0]',
+                1,
+                None,
+                None,
+            ),
+        ),
     ]
-    db_file = _setup_db(values)
+    db_file = _setup_db(stmts)
     mock_pool = Mock()
     load_db_task_action_timers = partial(
         TaskPool.load_db_task_action_timers, mock_pool

tests/unit/test_rundb.py

@@ -15,7 +15,6 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import contextlib
-import json
 from pathlib import Path
 import sqlite3
 from types import SimpleNamespace
@@ -224,9 +223,8 @@ def test_select_latest_flow_nums(
         )
         for (fnums, timestamp) in values:
             conn.execute(
-                "INSERT INTO task_states VALUES ("
-                f"{json.dumps(serialise_set(fnums))}, {json.dumps(timestamp)}"
-                ")"
+                'INSERT INTO task_states VALUES (?, ?)',
+                (serialise_set(fnums), timestamp)
             )
         conn.commit()
 

tests/unit/test_templatevars.py

@@ -118,21 +118,25 @@ def _setup_db(tmp_path_factory):
     db_path = logfolder / WorkflowFiles.LogDir.DB
     with CylcWorkflowDAO(db_path, create_tables=True) as dao:
         dao.connect().execute(
-            rf'''
+            r'''
                 INSERT INTO workflow_params
                 VALUES
-                    ("cylc_version", "{cylc_version}")
-            '''
+                    (?, ?)
+            ''',
+            ("cylc_version", cylc_version),
         )
-        dao.connect().execute(
+        dao.connect().executemany(
             r'''
                 INSERT INTO workflow_template_vars
                 VALUES
-                    ("FOO", "42"),
-                    ("BAR", "'hello world'"),
-                    ("BAZ", "'foo', 'bar', 48"),
-                    ("QUX", "['foo', 'bar', 21]")
-            '''
+                    (?, ?)
+            ''',
+            [
+                ("FOO", "42"),
+                ("BAR", "'hello world'"),
+                ("BAZ", "'foo', 'bar', 48"),
+                ("QUX", "['foo', 'bar', 21]"),
+            ],
         )
         dao.connect().commit()
     yield get_template_vars_from_db(tmp_path)