From 0168cab2e5334f9c576ad58884798696a6d6fe84 Mon Sep 17 00:00:00 2001
From: Jochen Topf <jochen@topf.org>
Date: Fri, 22 Aug 2025 09:35:09 +0200
Subject: [PATCH] A grab-bag of fixes for clang-tidy issues in test headers

---
 tests/common-cleanup.hpp |  9 ++++++---
 tests/common-options.hpp |  2 ++
 tests/common-pg.hpp      | 37 ++++++++++++++++++++-----------------
 3 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/tests/common-cleanup.hpp b/tests/common-cleanup.hpp
index 46a1dbe5d..2fc870714 100644
--- a/tests/common-cleanup.hpp
+++ b/tests/common-cleanup.hpp
@@ -26,9 +26,8 @@ namespace testing::cleanup {
 class file_t
 {
 public:
-    explicit file_t(std::string const &filename,
-                    bool remove_on_construct = true)
-    : m_filename(filename)
+    explicit file_t(std::string filename, bool remove_on_construct = true)
+    : m_filename(std::move(filename))
     {
         if (remove_on_construct) {
             delete_file(false);
@@ -38,6 +37,10 @@ class file_t
     ~file_t() noexcept { delete_file(true); }
 
 private:
+    // This function is run from a destructor so must be noexcept. If an
+    // exception does occur the program is terminated and we are fine with
+    // that, it is test code after all.
+    // NOLINTNEXTLINE(bugprone-exception-escape)
     void delete_file(bool warn) const noexcept
     {
         if (m_filename.empty()) {
diff --git a/tests/common-options.hpp b/tests/common-options.hpp
index 58adb8ea2..bbd45c66b 100644
--- a/tests/common-options.hpp
+++ b/tests/common-options.hpp
@@ -33,6 +33,8 @@ class opt_t
         m_opt.output_dbschema = "public";
     }
 
+    // Implicit conversion is intended here for ease of use in lots of tests.
+    // NOLINTNEXTLINE(google-explicit-constructor,hicpp-explicit-conversions)
     operator options_t() const { return m_opt; }
 
     opt_t &slim()
diff --git a/tests/common-pg.hpp b/tests/common-pg.hpp
index d48628499..5189494e8 100644
--- a/tests/common-pg.hpp
+++ b/tests/common-pg.hpp
@@ -122,7 +122,7 @@ class tempdb_t
                        "Test database cannot be created: {}\n"
                        "Did you mean to run 'pg_virtualenv ctest'?\n",
                        e.what());
-            std::exit(1);
+            std::exit(1); // NOLINT(concurrency-mt-unsafe)
         }
     }
 
@@ -134,22 +134,25 @@ class tempdb_t
 
     ~tempdb_t() noexcept
     {
-        if (!m_db_name.empty()) {
-            // Disable removal of the test database by setting the environment
-            // variable OSM2PGSQL_KEEP_TEST_DB to anything. This can be useful
-            // when debugging tests.
-            char const *const keep_db = std::getenv("OSM2PGSQL_KEEP_TEST_DB");
-            if (keep_db != nullptr) {
-                return;
-            }
-            try {
-                connection_params_t connection_params;
-                connection_params.set("dbname", "postgres");
-                conn_t const conn{connection_params};
-                conn.exec(R"(DROP DATABASE IF EXISTS "{}")", m_db_name);
-            } catch (...) {
-                fprintf(stderr, "DROP DATABASE failed. Ignored.\n");
-            }
+        if (m_db_name.empty()) {
+            return;
+        }
+
+        // Disable removal of the test database by setting the environment
+        // variable OSM2PGSQL_KEEP_TEST_DB to anything. This can be useful
+        // when debugging tests.
+        // NOLINTNEXTLINE(concurrency-mt-unsafe)
+        char const *const keep_db = std::getenv("OSM2PGSQL_KEEP_TEST_DB");
+        if (keep_db != nullptr) {
+            return;
+        }
+        try {
+            connection_params_t connection_params;
+            connection_params.set("dbname", "postgres");
+            conn_t const conn{connection_params};
+            conn.exec(R"(DROP DATABASE IF EXISTS "{}")", m_db_name);
+        } catch (...) {
+            fmt::print(stderr, "DROP DATABASE failed. Ignored.\n");
         }
     }