From af52eed0dda746f2a495cd6e33e777c43c04d0b3 Mon Sep 17 00:00:00 2001
From: Sarah Hoffmann <lonvia@denofr.de>
Date: Wed, 19 May 2021 16:42:35 +0200
Subject: [PATCH] test: avoid use of tempfile module

Use the tmp_path fixture instead which provides automatic
cleanup.
---
 test/python/conftest.py                       |  13 +-
 test/python/test_tools_exec_utils.py          | 123 +++++++++---------
 .../test_tools_import_special_phrases.py      |  15 +--
 3 files changed, 68 insertions(+), 83 deletions(-)

diff --git a/test/python/conftest.py b/test/python/conftest.py
index 77093c4e..c2544948 100644
--- a/test/python/conftest.py
+++ b/test/python/conftest.py
@@ -1,6 +1,5 @@
 import itertools
 import sys
-import tempfile
 from pathlib import Path
 
 import psycopg2
@@ -88,6 +87,9 @@ def temp_db_cursor(temp_db):
 
 @pytest.fixture
 def table_factory(temp_db_cursor):
+    """ A fixture that creates new SQL tables, potentially filled with
+        content.
+    """
     def mk_table(name, definition='id INT', content=None):
         temp_db_cursor.execute('CREATE TABLE {} ({})'.format(name, definition))
         if content is not None:
@@ -127,18 +129,11 @@ def cli_call():
     return _call_nominatim
 
 
-@pytest.fixture
-def tmp_phplib_dir():
-    with tempfile.TemporaryDirectory() as phpdir:
-        (Path(phpdir) / 'admin').mkdir()
-
-        yield Path(phpdir)
-
-
 @pytest.fixture
 def property_table(table_factory):
     table_factory('nominatim_properties', 'property TEXT, value TEXT')
 
+
 @pytest.fixture
 def status_table(temp_db_conn):
     """ Create an empty version of the status table and
diff --git a/test/python/test_tools_exec_utils.py b/test/python/test_tools_exec_utils.py
index 3abe9818..e713300e 100644
--- a/test/python/test_tools_exec_utils.py
+++ b/test/python/test_tools_exec_utils.py
@@ -3,97 +3,90 @@ Tests for tools.exec_utils module.
 """
 from pathlib import Path
 import subprocess
-import tempfile
 
 import pytest
 
 import nominatim.tools.exec_utils as exec_utils
 
-@pytest.fixture
-def nominatim_env(tmp_phplib_dir, def_config):
-    class _NominatimEnv:
-        config = def_config
-        phplib_dir = tmp_phplib_dir
-        data_dir = Path('data')
-        project_dir = Path('.')
-        sqllib_dir = Path('lib-sql')
-        config_dir = Path('settings')
-        module_dir = 'module'
-        osm2pgsql_path = 'osm2pgsql'
-
-    return _NominatimEnv
-
-@pytest.fixture
-def test_script(nominatim_env):
-    def _create_file(code):
-        with (nominatim_env.phplib_dir / 'admin' / 't.php').open(mode='w') as fd:
-            fd.write('<?php\n')
-            fd.write(code + '\n')
+class TestRunLegacyScript:
 
-        return 't.php'
+    @pytest.fixture(autouse=True)
+    def setup_nominatim_env(self, tmp_path, def_config):
+        tmp_phplib_dir = tmp_path / 'phplib'
+        tmp_phplib_dir.mkdir()
+        (tmp_phplib_dir / 'admin').mkdir()
+
+        class _NominatimEnv:
+            config = def_config
+            phplib_dir = tmp_phplib_dir
+            data_dir = Path('data')
+            project_dir = Path('.')
+            sqllib_dir = Path('lib-sql')
+            config_dir = Path('settings')
+            module_dir = 'module'
+            osm2pgsql_path = 'osm2pgsql'
 
-    return _create_file
+        self.testenv = _NominatimEnv
 
-@pytest.fixture(params=[0, 1, 15, 255])
-def return_code(request):
-    return request.param
 
-### run_legacy_script
+    def mk_script(self, code):
+        codefile = self.testenv.phplib_dir / 'admin' / 't.php'
+        codefile.write_text('<?php\n' + code + '\n')
 
-def test_run_legacy_return_exit_code(nominatim_env, test_script, return_code):
-    fname = test_script('exit({});'.format(return_code))
-    assert return_code == exec_utils.run_legacy_script(fname,
-                                                       nominatim_env=nominatim_env)
+        return 't.php'
 
 
-def test_run_legacy_return_throw_on_fail(nominatim_env, test_script):
-    fname = test_script('exit(11);')
-    with pytest.raises(subprocess.CalledProcessError):
-        exec_utils.run_legacy_script(fname, nominatim_env=nominatim_env,
-                                     throw_on_fail=True)
+    @pytest.mark.parametrize("return_code", (0, 1, 15, 255))
+    def test_run_legacy_return_exit_code(self, return_code):
+        fname = self.mk_script('exit({});'.format(return_code))
+        assert return_code == \
+                 exec_utils.run_legacy_script(fname, nominatim_env=self.testenv)
 
 
-def test_run_legacy_return_dont_throw_on_success(nominatim_env, test_script):
-    fname = test_script('exit(0);')
-    assert 0 == exec_utils.run_legacy_script(fname, nominatim_env=nominatim_env,
-                                             throw_on_fail=True)
+    def test_run_legacy_return_throw_on_fail(self):
+        fname = self.mk_script('exit(11);')
+        with pytest.raises(subprocess.CalledProcessError):
+            exec_utils.run_legacy_script(fname, nominatim_env=self.testenv,
+                                         throw_on_fail=True)
 
-def test_run_legacy_use_given_module_path(nominatim_env, test_script):
-    fname = test_script("exit($_SERVER['NOMINATIM_DATABASE_MODULE_PATH'] == '' ? 0 : 23);")
 
-    assert 0 == exec_utils.run_legacy_script(fname, nominatim_env=nominatim_env)
+    def test_run_legacy_return_dont_throw_on_success(self):
+        fname = self.mk_script('exit(0);')
+        assert 0 == exec_utils.run_legacy_script(fname, nominatim_env=self.testenv,
+                                                 throw_on_fail=True)
 
+    def test_run_legacy_use_given_module_path(self):
+        fname = self.mk_script("exit($_SERVER['NOMINATIM_DATABASE_MODULE_PATH'] == '' ? 0 : 23);")
 
-def test_run_legacy_do_not_overwrite_module_path(nominatim_env, test_script, monkeypatch):
-    monkeypatch.setenv('NOMINATIM_DATABASE_MODULE_PATH', 'other')
-    fname = test_script("exit($_SERVER['NOMINATIM_DATABASE_MODULE_PATH'] == 'other' ? 0 : 1);")
+        assert 0 == exec_utils.run_legacy_script(fname, nominatim_env=self.testenv)
 
-    assert 0 == exec_utils.run_legacy_script(fname, nominatim_env=nominatim_env)
 
-### run_api_script
+    def test_run_legacy_do_not_overwrite_module_path(self, monkeypatch):
+        monkeypatch.setenv('NOMINATIM_DATABASE_MODULE_PATH', 'other')
+        fname = self.mk_script("exit($_SERVER['NOMINATIM_DATABASE_MODULE_PATH'] == 'other' ? 0 : 1);")
 
-@pytest.fixture
-def tmp_project_dir():
-    with tempfile.TemporaryDirectory() as tempd:
-        project_dir = Path(tempd)
-        webdir = project_dir / 'website'
-        webdir.mkdir()
+        assert 0 == exec_utils.run_legacy_script(fname, nominatim_env=self.testenv)
 
-        with (webdir / 'test.php').open(mode='w') as fd:
-            fd.write("<?php\necho 'OK\n';")
 
-        yield project_dir
+class TestRunApiScript:
+
+    @pytest.fixture(autouse=True)
+    def setup_project_dir(self, tmp_path):
+        webdir = tmp_path / 'website'
+        webdir.mkdir()
+        (webdir / 'test.php').write_text("<?php\necho 'OK\n';")
+
 
-def test_run_api(tmp_project_dir):
-    assert 0 == exec_utils.run_api_script('test', tmp_project_dir)
+    def test_run_api(self, tmp_path):
+        assert 0 == exec_utils.run_api_script('test', tmp_path)
 
-def test_run_api_execution_error(tmp_project_dir):
-    assert 0 != exec_utils.run_api_script('badname', tmp_project_dir)
+    def test_run_api_execution_error(self, tmp_path):
+        assert 0 != exec_utils.run_api_script('badname', tmp_path)
 
-def test_run_api_with_extra_env(tmp_project_dir):
-    extra_env = dict(SCRIPT_FILENAME=str(tmp_project_dir / 'website' / 'test.php'))
-    assert 0 == exec_utils.run_api_script('badname', tmp_project_dir,
-                                          extra_env=extra_env)
+    def test_run_api_with_extra_env(self, tmp_path):
+        extra_env = dict(SCRIPT_FILENAME=str(tmp_path / 'website' / 'test.php'))
+        assert 0 == exec_utils.run_api_script('badname', tmp_path,
+                                              extra_env=extra_env)
 
 
 ### run_osm2pgsql
diff --git a/test/python/test_tools_import_special_phrases.py b/test/python/test_tools_import_special_phrases.py
index b3c60029..07c72a7c 100644
--- a/test/python/test_tools_import_special_phrases.py
+++ b/test/python/test_tools_import_special_phrases.py
@@ -2,15 +2,13 @@
     Tests for import special phrases methods
     of the class SPImporter.
 """
-from nominatim.errors import UsageError
-from pathlib import Path
-import tempfile
 from shutil import copyfile
 import pytest
 from nominatim.tools.special_phrases.sp_importer import SPImporter
 from nominatim.tools.special_phrases.sp_wiki_loader import SPWikiLoader
 from nominatim.tools.special_phrases.sp_csv_loader import SPCsvLoader
 from nominatim.tools.special_phrases.special_phrase import SpecialPhrase
+from nominatim.errors import UsageError
 
 from cursor import CursorForTesting
 
@@ -93,19 +91,18 @@ def test_load_white_and_black_lists(sp_importer):
 
     assert isinstance(black_list, dict) and isinstance(white_list, dict)
 
-def test_convert_php_settings(sp_importer, testfile_dir):
+def test_convert_php_settings(sp_importer, testfile_dir, tmp_path):
     """
         Test that _convert_php_settings_if_needed() convert the given
         php file to a json file.
     """
     php_file = (testfile_dir / 'phrase_settings.php').resolve()
 
-    with tempfile.TemporaryDirectory() as temp_dir:
-        temp_settings = (Path(temp_dir) / 'phrase_settings.php').resolve()
-        copyfile(php_file, temp_settings)
-        sp_importer._convert_php_settings_if_needed(temp_settings)
+    temp_settings = (tmp_path / 'phrase_settings.php').resolve()
+    copyfile(php_file, temp_settings)
+    sp_importer._convert_php_settings_if_needed(temp_settings)
 
-        assert (Path(temp_dir) / 'phrase_settings.json').is_file()
+    assert (tmp_path / 'phrase_settings.json').is_file()
 
 def test_convert_settings_wrong_file(sp_importer):
     """
-- 
2.39.5