lonvia PR feedback

diff --git a/.travis.yml b/.travis.yml
index dd973aecbc2649f7ae1e86b29a41be32f121e59b..f53447422c303ed9740a02edbd18a41acbd768e6 100644 (file)
--- a/.travis.yml
+++ b/.travis.yml
@@ -25,7 +25,7 @@ script:
   - cd $TRAVIS_BUILD_DIR/
   - if [[ $TEST_SUITE == "tests" ]]; then phpcs --report-width=120 . ; fi
   - cd $TRAVIS_BUILD_DIR/test/php
   - cd $TRAVIS_BUILD_DIR/
   - if [[ $TEST_SUITE == "tests" ]]; then phpcs --report-width=120 . ; fi
   - cd $TRAVIS_BUILD_DIR/test/php

-  - if [[ $TEST_SUITE == "tests" ]]; then UNIT_TEST_DSN='pgsql:dbname=nominatim_unit_tests' /usr/bin/phpunit ./ ; fi
+  - if [[ $TEST_SUITE == "tests" ]]; then /usr/bin/phpunit ./ ; fi

   - cd $TRAVIS_BUILD_DIR/test/bdd
   - # behave --format=progress3 api
   - if [[ $TEST_SUITE == "tests" ]]; then behave -DREMOVE_TEMPLATE=1 --format=progress3 db ; fi
   - cd $TRAVIS_BUILD_DIR/test/bdd
   - # behave --format=progress3 api
   - if [[ $TEST_SUITE == "tests" ]]; then behave -DREMOVE_TEMPLATE=1 --format=progress3 db ; fi

diff --git a/lib/DB.php b/lib/DB.php
index 5915b2e81242a727c82b9118c7ba0892114fb399..38b3e27ec8102f5ce8daca912a263ee62a311a3d 100644 (file)
--- a/lib/DB.php
+++ b/lib/DB.php
@@ -251,7 +251,7 @@ class DB
     }
 
     /**
     }
 
     /**

-     * Deletes a table. Returns true on success. Returns true if the table didn't exist.
+     * Deletes a table. Returns true if deleted or didn't exist.

      *
      * @param string  $sTableName
      *
      *
      * @param string  $sTableName
      *


@@ -405,29 +405,16 @@ END;
      */
     public static function generateDSN($aInfo)
     {
      */
     public static function generateDSN($aInfo)
     {

-        $sDSN = 'pgsql:';
-        if (isset($aInfo['host'])) {
-            $sDSN .= 'host=' . $aInfo['host'] . ';';
-        } elseif (isset($aInfo['hostspec'])) {
-            $sDSN .= 'host=' . $aInfo['hostspec'] . ';';
-        }
-        if (isset($aInfo['port'])) {
-            $sDSN .= 'port=' . $aInfo['port'] . ';';
-        }
-        if (isset($aInfo['dbname'])) {
-            $sDSN .= 'dbname=' . $aInfo['dbname'] . ';';
-        } elseif (isset($aInfo['database'])) {
-            $sDSN .= 'dbname=' . $aInfo['database'] . ';';
-        }
-        if (isset($aInfo['user'])) {
-            $sDSN .= 'user=' . $aInfo['user'] . ';';
-        } elseif (isset($aInfo['username'])) {
-            $sDSN .= 'user=' . $aInfo['username'] . ';';
-        }
-        if (isset($aInfo['password'])) {
-            $sDSN .= 'password=' . $aInfo['password'] . ';';
-        }
-        $sDSN = preg_replace('/;$/', '', $sDSN);
+        $sDSN = sprintf(
+            'pgsql:host=%s;port=%s;dbname=%s;user=%s;password=%s;',
+            $aInfo['host'] ?? $aInfo['hostspec'] ?? '',
+            $aInfo['port'] ?? '',
+            $aInfo['dbname'] ?? $aInfo['database'] ?? '',
+            $aInfo['user'] ?? '',
+            $aInfo['password'] ?? ''
+        );
+        $sDSN = preg_replace('/\b\w+=;/', '', $sDSN);
+        $sDSN = preg_replace('/;\Z/', '', $sDSN);

 
         return $sDSN;
     }
 
         return $sDSN;
     }

diff --git a/test/README.md b/test/README.md
index 5de08759532fb962cefb5d1afc87331d8576f777..1f7a33ca47f5b1c5d4ca541a4113ec860733da82 100644 (file)
--- a/test/README.md
+++ b/test/README.md
@@ -51,8 +51,8 @@ To execute the test suite run
 It will read phpunit.xml which points to the library, test path, bootstrap
 strip and set other parameters.
 
 It will read phpunit.xml which points to the library, test path, bootstrap
 strip and set other parameters.
 

-The database set by `UNIT_TEST_DSN` will be deleted and recreated. Not setting
-it will skip some tests as pending, but not fail the tests.
+It will use (and destroy) a local database 'nominatim_unit_tests'. You can set
+a different connection string with e.g. UNIT_TEST_DSN='pgsql:dbname=foo_unit_tests'.

 
 BDD Functional Tests
 ====================
 
 BDD Functional Tests
 ====================

diff --git a/test/php/Nominatim/DBTest.php b/test/php/Nominatim/DBTest.php
index e47919758a449175683fa5721d3ab75d1ec7fa38..1991f6f1e7655d69da15c4096ea237e801ca5b08 100644 (file)
--- a/test/php/Nominatim/DBTest.php
+++ b/test/php/Nominatim/DBTest.php
@@ -128,11 +128,19 @@ class DBTest extends \PHPUnit\Framework\TestCase
 
     public function testAgainstDatabase()
     {
 
     public function testAgainstDatabase()
     {

-        if (getenv('UNIT_TEST_DSN') == false) $this->markTestSkipped('UNIT_TEST_DSN not set');
+        $unit_test_dsn = getenv('UNIT_TEST_DSN') != false ?
+                            getenv('UNIT_TEST_DSN') :
+                            'pgsql:dbname=nominatim_unit_tests';
+
+        $this->assertRegExp(
+            '/unit_test/',
+            $unit_test_dsn,
+            'Test database will get destroyed, thus should have a name like unit_test to be safe'
+        );

 
         ## Create the database.
         {
 
         ## Create the database.
         {

-            $aDSNParsed = \Nominatim\DB::parseDSN(getenv('UNIT_TEST_DSN'));
+            $aDSNParsed = \Nominatim\DB::parseDSN($unit_test_dsn);

             $sDbname = $aDSNParsed['database'];
             $aDSNParsed['database'] = 'postgres';
 
             $sDbname = $aDSNParsed['database'];
             $aDSNParsed['database'] = 'postgres';
 


@@ -142,7 +150,7 @@ class DBTest extends \PHPUnit\Framework\TestCase
             $oDB->exec('CREATE DATABASE ' . $sDbname);
         }
 
             $oDB->exec('CREATE DATABASE ' . $sDbname);
         }
 

-        $oDB = new \Nominatim\DB(getenv('UNIT_TEST_DSN'));
+        $oDB = new \Nominatim\DB($unit_test_dsn);

         $oDB->connect();
 
         $this->assertTrue(
         $oDB->connect();
 
         $this->assertTrue(