Use more reliable database locking strategy; reorganize tests

PostgreSQL and MySQL both have better locking mechanisms than what was previously implemented, as well

lib/Db/AbstractDriver.php

@@ -4,10 +4,13 @@ namespace JKingWeb\Arsse\Db;
 use JKingWeb\DrUUID\UUID as UUID;
 
 abstract class AbstractDriver implements Driver {
+    protected $locked = false;
     protected $transDepth = 0;
     protected $transStatus = [];
 
     public abstract function prepareArray(string $query, array $paramTypes): Statement;
+    protected abstract function lock(): bool;
+    protected abstract function unlock(bool $rollback = false) : bool;
 
     public function schemaVersion(): int {
         try {
@@ -17,11 +20,15 @@ abstract class AbstractDriver implements Driver {
         }
     }
 
-    public function begin(): Transaction {
-        return new Transaction($this);
+    public function begin(bool $lock = false): Transaction {
+        return new Transaction($this, $lock);
     }
     
-    public function savepointCreate(): int {
+    public function savepointCreate(bool $lock = false): int {
+        if($lock && !$this->transDepth) {
+            $this->lock();
+            $this->locked = true;
+        }
         $this->exec("SAVEPOINT arsse_".(++$this->transDepth));
         $this->transStatus[$this->transDepth] = self::TR_PEND;
         return $this->transDepth;
@@ -60,6 +67,10 @@ abstract class AbstractDriver implements Driver {
                     $this->transDepth--;
                 }
             }
+            if(!$this->transDepth && $this->locked) {
+                $this->unlock();
+                $this->locked = false;
+            }
             return $out;
         } else {
             throw new ExceptionSavepoint("invalid", ['action' => "commit", 'index' => $index]);
@@ -100,36 +111,16 @@ abstract class AbstractDriver implements Driver {
                     $this->transDepth--;
                 }
             }
+            if(!$this->transDepth && $this->locked) {
+                $this->unlock(true);
+                $this->locked = false;
+            }
             return $out;
         } else {
             throw new ExceptionSavepoint("invalid", ['action' => "rollback", 'index' => $index]);
         }
     }
 
-    public function lock(): bool {
-        if($this->schemaVersion() < 1) return true;
-        if($this->isLocked()) return false;
-        $uuid = UUID::mintStr();
-        try {
-            $this->prepare("INSERT INTO arsse_meta(key,value) values(?,?)", "str", "str")->run("lock", $uuid);
-        } catch(ExceptionInput $e) {
-            return false;
-        }
-        sleep(1);
-        return ($this->query("SELECT value from arsse_meta where key is 'lock'")->getValue() == $uuid);
-    }
-
-    public function unlock(): bool {
-        if($this->schemaVersion() < 1) return true;
-        $this->exec("DELETE from arsse_meta where key is 'lock'");
-        return true;
-    }
-
-    public function isLocked(): bool {
-        if($this->schemaVersion() < 1) return false;
-        return ($this->query("SELECT count(*) from arsse_meta where key is 'lock'")->getValue() > 0);
-    }
-
     public function prepare(string $query, ...$paramType): Statement {
         return $this->prepareArray($query, $paramType);
     }

lib/Db/AbstractStatement.php

@@ -59,25 +59,17 @@ abstract class AbstractStatement implements Statement {
             case "string":
             case "boolean":
                 if($t=="binary") $t = "string";
-                $value = $v;
-                try{
-                    settype($value, $t);
-                } catch(\Throwable $e) {
-                    // handle objects
-                    $value = $v;
-                    if($value instanceof \DateTimeInterface) {
-                        if($t=="string") {
-                            $value = $this->dateTransform($value, "sql");
-                        } else {
-                            $value = $value->getTimestamp();
-                            settype($value, $t);    
-                        }
+                if($v instanceof \DateTimeInterface) {
+                    if($t=="string") {
+                        return $this->dateTransform($v, "sql");
                     } else {
-                        $value = null;
-                        settype($value, $t);
+                        $v = $v->getTimestamp();
+                        settype($v, $t);    
                     }
+                } else {
+                    settype($v, $t);
                 }
-                return $value;
+                return $v;
             default:
                 throw new Exception("paramTypeUnknown", $type);
         }

lib/Db/Driver.php

@@ -15,17 +15,13 @@ interface Driver {
     // returns the version of the scheme of the opened database; if uninitialized should return 0
     function schemaVersion(): int;
     // return a Transaction object
-    function begin(): Transaction;
+    function begin(bool $lock = false): Transaction;
     // manually begin a real or synthetic transactions, with real or synthetic nesting
     function savepointCreate(): int;
     // manually commit either the latest or all pending nested transactions
     function savepointRelease(int $index = null): bool;
     // manually rollback either the latest or all pending nested transactions
     function savepointUndo(int $index = null): bool;
-    // attempt to advise other processes that they should not attempt to access the database; used during live upgrades
-    function lock(): bool;
-    function unlock(): bool;
-    function isLocked(): bool;
     // attempt to perform an in-place upgrade of the database schema; this may be a no-op which always throws an exception
     function schemaUpdate(int $to): bool;
     // execute one or more unsanitized SQL queries and return an indication of success

lib/Db/SQLite3/Driver.php

@@ -22,7 +22,7 @@ class Driver extends \JKingWeb\Arsse\Db\AbstractDriver {
         $file = Data::$conf->dbSQLite3File;
         // if the file exists (or we're initializing the database), try to open it
         try {
-            $this->db = new \SQLite3($file, ($install) ? \SQLITE3_OPEN_READWRITE | \SQLITE3_OPEN_CREATE : \SQLITE3_OPEN_READWRITE, Data::$conf->dbSQLite3Key);
+            $this->db = $this->makeConnection($file, ($install) ? \SQLITE3_OPEN_READWRITE | \SQLITE3_OPEN_CREATE : \SQLITE3_OPEN_READWRITE, Data::$conf->dbSQLite3Key);
         } catch(\Throwable $e) {
             // if opening the database doesn't work, check various pre-conditions to find out what the problem might be
             if(!file_exists($file)) {
@@ -46,6 +46,10 @@ class Driver extends \JKingWeb\Arsse\Db\AbstractDriver {
         }
     }
 
+    protected function makeConnection(string $file, int $opts, string $key): \SQLite3 {
+        return new \SQLite3($file, $opts, $key);
+    }
+
     public function __destruct() {
         try{$this->db->close();} catch(\Exception $e) {}
         unset($this->db);
@@ -66,9 +70,9 @@ class Driver extends \JKingWeb\Arsse\Db\AbstractDriver {
         if($ver >= $to) throw new Exception("updateTooNew", ['difference' => ($ver - $to), 'current' => $ver, 'target' => $to, 'driver_name' => $this->driverName()]);
         $sep = \DIRECTORY_SEPARATOR;
         $path = Data::$conf->dbSchemaBase.$sep."SQLite3".$sep;
-        $this->lock();
-        $tr = $this->savepointCreate();
-        for($a = $ver; $a < $to; $a++) {
+        // lock the database
+        $this->savepointCreate(true);
+        for($a = $this->schemaVersion(); $a < $to; $a++) {
             $this->savepointCreate();
             try {
                 $file = $path.$a.".sql";
@@ -78,7 +82,7 @@ class Driver extends \JKingWeb\Arsse\Db\AbstractDriver {
                 if($sql===false) throw new Exception("updateFileUnusable", ['file' => $file, 'driver_name' => $this->driverName(), 'current' => $a]);
                 try {
                     $this->exec($sql);
-                } catch(\Exception $e) {
+                } catch(\Throwable $e) {
                     throw new Exception("updateFileError", ['file' => $file, 'driver_name' => $this->driverName(), 'current' => $a, 'message' => $this->getError()]);
                 }
                 if($this->schemaVersion() != $a+1) throw new Exception("updateFileIncomplete", ['file' => $file, 'driver_name' => $this->driverName(), 'current' => $a]);
@@ -86,14 +90,12 @@ class Driver extends \JKingWeb\Arsse\Db\AbstractDriver {
                 // undo any partial changes from the failed update
                 $this->savepointUndo();
                 // commit any successful updates if updating by more than one version
-                $this->unlock();
                 $this->savepointRelease();
                 // throw the error received
                 throw $e;
             }
             $this->savepointRelease();
         }
-        $this->unlock();
         $this->savepointRelease();
         return true;
     }
@@ -132,4 +134,14 @@ class Driver extends \JKingWeb\Arsse\Db\AbstractDriver {
         }
         return new Statement($this->db, $s, $paramTypes);
     }
+
+    protected function lock(): bool {
+        $this->exec("BEGIN EXCLUSIVE TRANSACTION");
+        return true;
+    }
+
+    protected function unlock(bool $rollback = false): bool {
+        $this->exec((!$rollback) ? "COMMIT" : "ROLLBACK");
+        return true;
+    }
 }

lib/Db/Transaction.php

@@ -7,8 +7,8 @@ class Transaction {
     protected $pending = false;
     protected $drv;
 
-    function __construct(Driver $drv) {
-        $this->index = $drv->savepointCreate();
+    function __construct(Driver $drv, bool $lock = false) {
+        $this->index = $drv->savepointCreate($lock);
         $this->drv = $drv;
         $this->pending = true;
     }

lib/Misc/Context.php

@@ -43,13 +43,8 @@ class Context {
                 continue;
             }
             if(is_string($id)) {
-                try {
-                    $ch1 = strval(intval($id));
-                    $ch2 = strval($id);
-                } catch(\Throwable $e) {
-                    $ch1 = true;
-                    $ch2 = false;
-                }
+                $ch1 = strval(@intval($id));
+                $ch2 = strval($id);
                 if($ch1 !== $ch2 || $id < 1) $id = 0;
             } else {
                 $id = 0;

lib/REST/AbstractHandler.php

@@ -32,12 +32,8 @@ abstract class AbstractHandler implements Handler {
     }
 
     protected function validateInt($id): bool {
-        try {
-            $ch1 = strval(intval($id));
-            $ch2 = strval($id);
-        } catch(\Throwable $e) {
-            return false;
-        }
+        $ch1 = strval(@intval($id));
+        $ch2 = strval($id);
         return ($ch1 === $ch2);
     }
 

lib/REST/NextCloudNews/V1_2.php

@@ -46,9 +46,8 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler {
         if($req->body) {
             // if the entity body is not JSON according to content type, return "415 Unsupported Media Type"
             if(!preg_match("<^application/json\b|^$>", $req->type)) return new Response(415, "", "", ['Accept: application/json']);
-            try {
-                $data = json_decode($req->body, true);
-            } catch(\Throwable $e) {
+            $data = @json_decode($req->body, true);
+            if(json_last_error() != \JSON_ERROR_NONE) {
                 // if the body could not be parsed as JSON, return "400 Bad Request"
                 return new Response(400);
             }

tests/Conf/TestConf.php

@@ -4,9 +4,7 @@ namespace JKingWeb\Arsse;
 use org\bovigo\vfs\vfsStream;
 
 
-class TestConf extends \PHPUnit\Framework\TestCase {
-    use Test\Tools;
-
+class TestConf extends Test\AbstractTest {
     static $vfs;
     static $path;
 

tests/Db/SQLite3/Database/TestDatabaseArticleSQLite3.php

@@ -2,8 +2,8 @@
 declare(strict_types=1);
 namespace JKingWeb\Arsse;
 
-class TestDatabaseArticleSQLite3 extends \PHPUnit\Framework\TestCase {
-    use Test\Tools, Test\Database\Setup;
+class TestDatabaseArticleSQLite3 extends Test\AbstractTest {
+    use Test\Database\Setup;
     use Test\Database\DriverSQLite3;
     use Test\Database\SeriesArticle;
 }

tests/Db/SQLite3/Database/TestDatabaseFeedSQLite3.php

@@ -2,8 +2,8 @@
 declare(strict_types=1);
 namespace JKingWeb\Arsse;
 
-class TestDatabaseFeedSQLite3 extends \PHPUnit\Framework\TestCase {
-    use Test\Tools, Test\Database\Setup;
+class TestDatabaseFeedSQLite3 extends Test\AbstractTest {
+    use Test\Database\Setup;
     use Test\Database\DriverSQLite3;
     use Test\Database\SeriesFeed;
 }

tests/Db/SQLite3/Database/TestDatabaseFolderSQLite3.php

@@ -2,8 +2,8 @@
 declare(strict_types=1);
 namespace JKingWeb\Arsse;
 
-class TestDatabaseFolderSQLite3 extends \PHPUnit\Framework\TestCase {
-    use Test\Tools, Test\Database\Setup;
+class TestDatabaseFolderSQLite3 extends Test\AbstractTest {
+    use Test\Database\Setup;
     use Test\Database\DriverSQLite3;
     use Test\Database\SeriesFolder;
 }

tests/Db/SQLite3/Database/TestDatabaseSubscriptionSQLite3.php

@@ -2,8 +2,8 @@
 declare(strict_types=1);
 namespace JKingWeb\Arsse;
 
-class TestDatabaseSubscriptionSQLite3 extends \PHPUnit\Framework\TestCase {
-    use Test\Tools, Test\Database\Setup;
+class TestDatabaseSubscriptionSQLite3 extends Test\AbstractTest {
+    use Test\Database\Setup;
     use Test\Database\DriverSQLite3;
     use Test\Database\SeriesSubscription;
 }

tests/Db/SQLite3/Database/TestDatabaseUserSQLite3.php

@@ -2,8 +2,8 @@
 declare(strict_types=1);
 namespace JKingWeb\Arsse;
 
-class TestDatabaseUserSQLite3 extends \PHPUnit\Framework\TestCase {
-    use Test\Tools, Test\Database\Setup;
+class TestDatabaseUserSQLite3 extends Test\AbstractTest {
+    use Test\Database\Setup;
     use Test\Database\DriverSQLite3;
     use Test\Database\SeriesUser;
 }

tests/Db/SQLite3/TestDbDriverSQLite3.php

@@ -3,9 +3,7 @@ declare(strict_types=1);
 namespace JKingWeb\Arsse;
 
 
-class TestDbDriverSQLite3 extends \PHPUnit\Framework\TestCase {
-    use Test\Tools;
-
+class TestDbDriverSQLite3 extends Test\AbstractTest {
     protected $data;
     protected $drv;
     protected $ch;
@@ -21,6 +19,7 @@ class TestDbDriverSQLite3 extends \PHPUnit\Framework\TestCase {
         Data::$conf = $conf;
         $this->drv = new Db\SQLite3\Driver(true);
         $this->ch = new \SQLite3(Data::$conf->dbSQLite3File);
+        $this->ch->enableExceptions(true);
     }
 
     function tearDown() {
@@ -68,7 +67,7 @@ class TestDbDriverSQLite3 extends \PHPUnit\Framework\TestCase {
     }
 
     function testMakeAValidQuery() {
-        $this->assertInstanceOf(Db\SQLite3\Result::class, $this->drv->query("SELECT 1"));
+        $this->assertInstanceOf(Db\Result::class, $this->drv->query("SELECT 1"));
     }
 
     function testMakeAnInvalidQuery() {
@@ -96,7 +95,7 @@ class TestDbDriverSQLite3 extends \PHPUnit\Framework\TestCase {
 
     function testPrepareAValidQuery() {
         $s = $this->drv->prepare("SELECT ?, ?", "int", "int");
-        $this->assertInstanceOf(Db\SQLite3\Statement::class, $s);
+        $this->assertInstanceOf(Db\Statement::class, $s);
     }
 
     function testPrepareAnInvalidQuery() {
@@ -295,25 +294,17 @@ class TestDbDriverSQLite3 extends \PHPUnit\Framework\TestCase {
         $this->assertSame(1, $this->drv->schemaVersion());
         $this->drv->exec("PRAGMA user_version=2");
         $this->assertSame(2, $this->drv->schemaVersion());
+    }
 
+    function testLockTheDatabase() {
+        $this->drv->savepointCreate(true);
+        $this->assertException();
+        $this->ch->exec("CREATE TABLE test(id integer primary key)");
     }
 
-    function testManipulateAdvisoryLock() {
-        $this->assertTrue($this->drv->unlock());
-        $this->assertFalse($this->drv->isLocked());
-        $this->assertTrue($this->drv->lock());
-        $this->assertFalse($this->drv->isLocked());
-        $this->drv->exec("CREATE TABLE arsse_meta(key text primary key, value text); PRAGMA user_version=1");
-        $this->assertTrue($this->drv->lock());
-        $this->assertTrue($this->drv->isLocked());
-        $this->assertFalse($this->drv->lock());
-        $this->drv->exec("PRAGMA user_version=0");
-        $this->assertFalse($this->drv->isLocked());
-        $this->assertTrue($this->drv->lock());
-        $this->assertFalse($this->drv->isLocked());
-        $this->drv->exec("PRAGMA user_version=1");
-        $this->assertTrue($this->drv->isLocked());
-        $this->assertTrue($this->drv->unlock());
-        $this->assertFalse($this->drv->isLocked());
+    function testUnlockTheDatabase() {
+        $this->drv->savepointCreate(true);
+        $this->drv->savepointRelease();
+        $this->assertSame(true, $this->ch->exec("CREATE TABLE test(id integer primary key)"));
     }
 }

tests/Db/SQLite3/TestDbResultSQLite3.php

@@ -3,8 +3,7 @@ declare(strict_types=1);
 namespace JKingWeb\Arsse;
 
 
-class TestDbResultSQLite3 extends \PHPUnit\Framework\TestCase {
-    use Test\Tools;
+class TestDbResultSQLite3 extends Test\AbstractTest {
 
     protected $c;
 

tests/Db/SQLite3/TestDbStatementSQLite3.php

@@ -4,8 +4,8 @@ namespace JKingWeb\Arsse;
 use JKingWeb\Arsse\Db\Statement;
 
 
-class TestDbStatementSQLite3 extends \PHPUnit\Framework\TestCase {
-    use Test\Tools, Test\Db\BindingTests;
+class TestDbStatementSQLite3 extends Test\AbstractTest {
+    use Test\Db\BindingTests;
 
     protected $c;
     static protected $imp = Db\SQLite3\Statement::class;
@@ -20,7 +20,6 @@ class TestDbStatementSQLite3 extends \PHPUnit\Framework\TestCase {
     }
 
     function tearDown() {
-        try {$this->s->close();} catch(\Exception $e) {}
         $this->c->close();
         unset($this->c);
     }
@@ -32,7 +31,7 @@ class TestDbStatementSQLite3 extends \PHPUnit\Framework\TestCase {
         foreach($types as $type) {
             $s->rebindArray([$strict ? "strict $type" : $type]);
             $val = $s->runArray([$input])->getRow()['value'];
-            $this->assertSame($expectations[$type], $val, "Type $type failed comparison.");
+            $this->assertSame($expectations[$type], $val, "Binding from type $type failed comparison.");
         }
     }
 

tests/Db/SQLite3/TestDbUpdateSQLite3.php

@@ -4,9 +4,7 @@ namespace JKingWeb\Arsse;
 use org\bovigo\vfs\vfsStream;
 
 
-class TestDbUpdateSQLite3 extends \PHPUnit\Framework\TestCase {
-    use Test\Tools;
-
+class TestDbUpdateSQLite3 extends Test\AbstractTest {
     protected $data;
     protected $drv;
     protected $vfs;

tests/Exception/TestException.php

@@ -4,9 +4,7 @@ namespace JKingWeb\Arsse;
 Use Phake;
 
 
-class TestException extends \PHPUnit\Framework\TestCase {
-    use Test\Tools;
-
+class TestException extends Test\AbstractTest {
     function setUp() {
         $this->clearData(false);
         // create a mock Lang object so as not to create a dependency loop

tests/Feed/TestFeed.php

@@ -4,9 +4,7 @@ namespace JKingWeb\Arsse;
 Use Phake;
 
 
-class TestFeed extends \PHPUnit\Framework\TestCase {
-    use Test\Tools;
-
+class TestFeed extends Test\AbstractTest {
     protected static $host = "http://localhost:8000/";
     protected $base = "";
     protected $latest = [

tests/Feed/TestFeedFetching.php

@@ -4,9 +4,7 @@ namespace JKingWeb\Arsse;
 Use Phake;
 
 
-class TestFeedFetching extends \PHPUnit\Framework\TestCase {
-    use Test\Tools;
-
+class TestFeedFetching extends Test\AbstractTest {
     protected static $host = "http://localhost:8000/";
     protected $base = "";
 

tests/Lang/TestLang.php

@@ -4,8 +4,8 @@ namespace JKingWeb\Arsse;
 use org\bovigo\vfs\vfsStream;
 
 
-class TestLang extends \PHPUnit\Framework\TestCase {
-    use Test\Tools, Test\Lang\Setup;
+class TestLang extends Test\AbstractTest {
+    use Test\Lang\Setup;
 
     public $files;
     public $path;

tests/Lang/TestLangErrors.php

@@ -4,8 +4,8 @@ namespace JKingWeb\Arsse;
 use org\bovigo\vfs\vfsStream;
 
 
-class TestLangErrors extends \PHPUnit\Framework\TestCase {
-    use Test\Tools, Test\Lang\Setup;
+class TestLangErrors extends Test\AbstractTest {
+    use Test\Lang\Setup;
 
     public $files;
     public $path;

tests/Lang/testLangComplex.php

@@ -4,8 +4,8 @@ namespace JKingWeb\Arsse;
 use org\bovigo\vfs\vfsStream;
 
 
-class TestLangComplex extends \PHPUnit\Framework\TestCase {
-    use Test\Tools, Test\Lang\Setup;
+class TestLangComplex extends Test\AbstractTest {
+    use Test\Lang\Setup;
 
     public $files;
     public $path;

tests/Misc/TestContext.php

@@ -4,9 +4,7 @@ namespace JKingWeb\Arsse;
 use JKingWeb\Arsse\Misc\Context;
 
 
-class TestContext extends \PHPUnit\Framework\TestCase {
-    use Test\Tools;
-
+class TestContext extends Test\AbstractTest {
     function testVerifyInitialState() {
         $c = new Context;
         foreach((new \ReflectionObject($c))->getMethods(\ReflectionMethod::IS_PUBLIC) as $m) {

tests/REST/NextCloudNews/TestNCNV1_2.php

@@ -8,9 +8,7 @@ use JKingWeb\Arsse\Misc\Context;
 use Phake;
 
 
-class TestNCNV1_2 extends \PHPUnit\Framework\TestCase {
-    use Test\Tools;
-
+class TestNCNV1_2 extends Test\AbstractTest {
     protected $h;
     protected $feeds = [ // expected sample output of a feed list from the database, and the resultant expected transformation by the REST handler
         'db' => [
@@ -25,7 +23,7 @@ class TestNCNV1_2 extends \PHPUnit\Framework\TestCase {
                 'err_count' => 0,
                 'err_msg' => '',
                 'order_type' => 0,
-                'added' => 1495287354,
+                'added' => '2017-05-20 13:35:54',
                 'title' => 'First example feed',
                 'unread' => 50048,
             ],
@@ -40,7 +38,7 @@ class TestNCNV1_2 extends \PHPUnit\Framework\TestCase {
                 'err_count' => 0,
                 'err_msg' => '',
                 'order_type' => 2,
-                'added' => 1495287354,
+                'added' => '2017-05-20 13:35:54',
                 'title' => 'Second example feed',
                 'unread' => 23,
             ],

tests/REST/NextCloudNews/TestNCNVersionDiscovery.php

@@ -5,9 +5,7 @@ use JKingWeb\Arsse\REST\Request;
 use JKingWeb\Arsse\REST\Response;
 
 
-class TestNCNVersionDiscovery extends \PHPUnit\Framework\TestCase {
-    use Test\Tools;
-
+class TestNCNVersionDiscovery extends Test\AbstractTest {
     function setUp() {
         $this->clearData();
     }

tests/User/TestAuthorization.php

@@ -4,9 +4,7 @@ namespace JKingWeb\Arsse;
 use Phake;
 
 
-class TestAuthorization extends \PHPUnit\Framework\TestCase {
-    use Test\Tools;
-
+class TestAuthorization extends Test\AbstractTest {
     const USERS = [
         'user@example.com' => User\Driver::RIGHTS_NONE,
         'user@example.org' => User\Driver::RIGHTS_NONE,

tests/User/TestUserInternalDriver.php

@@ -3,8 +3,8 @@ declare(strict_types=1);
 namespace JKingWeb\Arsse;
 
 
-class TestUserInternalDriver extends \PHPUnit\Framework\TestCase {
-    use Test\Tools, Test\User\CommonTests;
+class TestUserInternalDriver extends Test\AbstractTest {
+    use Test\User\CommonTests;
 
     const USER1 = "john.doe@example.com";
     const USER2 = "jane.doe@example.com";

tests/User/TestUserMockExternal.php

@@ -3,8 +3,8 @@ declare(strict_types=1);
 namespace JKingWeb\Arsse;
 
 
-class TestUserMockExternal extends \PHPUnit\Framework\TestCase {
-    use Test\Tools, Test\User\CommonTests;
+class TestUserMockExternal extends Test\AbstractTest {
+    use Test\User\CommonTests;
 
     const USER1 = "john.doe@example.com";
     const USER2 = "jane.doe@example.com";

tests/User/TestUserMockInternal.php

@@ -3,8 +3,8 @@ declare(strict_types=1);
 namespace JKingWeb\Arsse;
 
 
-class TestUserMockInternal extends \PHPUnit\Framework\TestCase {
-    use Test\Tools, Test\User\CommonTests;
+class TestUserMockInternal extends Test\AbstractTest {
+    use Test\User\CommonTests;
 
     const USER1 = "john.doe@example.com";
     const USER2 = "jane.doe@example.com";

tests/lib/AbstractTest.php

@@ -0,0 +1,44 @@
+<?php
+declare(strict_types=1);
+namespace JKingWeb\Arsse\Test;
+use JKingWeb\Arsse\Exception;
+use JKingWeb\Arsse\Data;
+
+abstract class AbstractTest extends \PHPUnit\Framework\TestCase {
+    use \JKingWeb\Arsse\Misc\DateFormatter;
+
+    function assertException(string $msg = "", string $prefix = "", string $type = "Exception") {
+        if(func_num_args()) {
+            $class = \JKingWeb\Arsse\NS_BASE . ($prefix !== "" ? str_replace("/", "\\", $prefix) . "\\" : "") . $type;
+            $msgID = ($prefix !== "" ? $prefix . "/" : "") . $type. ".$msg";
+            if(array_key_exists($msgID, Exception::CODES)) {
+                $code = Exception::CODES[$msgID];
+            } else {
+                $code = 0;
+            }
+            $this->expectException($class);
+            $this->expectExceptionCode($code);
+        } else {
+            // expecting a standard PHP exception
+            $this->expectException(\Exception::class);
+        }
+    }
+
+    function assertTime($exp, $test) {
+        $exp  = $this->dateTransform($exp, "unix");
+        $test = $this->dateTransform($test, "unix");
+        $this->assertSame($exp, $test);
+    }
+
+    function clearData(bool $loadLang = true): bool {
+        $r = new \ReflectionClass(\JKingWeb\Arsse\Data::class);
+        $props = array_keys($r->getStaticProperties());
+        foreach($props as $prop) {
+            Data::$$prop = null;
+        }
+        if($loadLang) {
+            Data::$l = new \JKingWeb\Arsse\Lang();
+        }
+        return true;
+    }
+}

tests/lib/Tools.php

@@ -1,39 +0,0 @@
-<?php
-declare(strict_types=1);
-namespace JKingWeb\Arsse\Test;
-use JKingWeb\Arsse\Exception;
-use JKingWeb\Arsse\Data;
-
-trait Tools {
-    use \JKingWeb\Arsse\Misc\DateFormatter;
-
-    function assertException(string $msg, string $prefix = "", string $type = "Exception") {
-        $class = \JKingWeb\Arsse\NS_BASE . ($prefix !== "" ? str_replace("/", "\\", $prefix) . "\\" : "") . $type;
-        $msgID = ($prefix !== "" ? $prefix . "/" : "") . $type. ".$msg";
-        if(array_key_exists($msgID, Exception::CODES)) {
-            $code = Exception::CODES[$msgID];
-        } else {
-            $code = 0;
-        }
-        $this->expectException($class);
-        $this->expectExceptionCode($code);
-    }
-
-    function assertTime($exp, $test) {
-        $exp  = $this->dateTransform($exp, "unix");
-        $test = $this->dateTransform($test, "unix");
-        $this->assertSame($exp, $test);
-    }
-
-    function clearData(bool $loadLang = true): bool {
-        $r = new \ReflectionClass(\JKingWeb\Arsse\Data::class);
-        $props = array_keys($r->getStaticProperties());
-        foreach($props as $prop) {
-            Data::$$prop = null;
-        }
-        if($loadLang) {
-            Data::$l = new \JKingWeb\Arsse\Lang();
-        }
-        return true;
-    }
-}

tests/phpunit.xml

@@ -2,9 +2,9 @@
 <phpunit
     colors="true"
     bootstrap="../bootstrap.php"
-    convertErrorsToExceptions="true"
-    convertNoticesToExceptions="true"
-    convertWarningsToExceptions="true"
+    convertErrorsToExceptions="false"
+    convertNoticesToExceptions="false"
+    convertWarningsToExceptions="false"
     beStrictAboutTestsThatDoNotTestAnything="true"
     beStrictAboutOutputDuringTests="true"
     beStrictAboutTestSize="true"