lstrojny / phpunit-clever-and-smart

Smarter test runner for PHPUnit
169 stars 13 forks source link

phpunit fails if sqlite database is locked #36

Closed mpscholten closed 8 years ago

mpscholten commented 9 years ago

When running multiple tests in parallel the database of phpunit-clever-and-smart might locked for a short moment. Sometimes this leads to phpunit aborting execution.

Here's the error message with stack trace:

Warning:  SQLite3::query(): Unable to prepare statement: 5, database is locked in /myapp/vendor/lstrojny/phpunit-clever-and-smart/src/PHPUnit/Runner/CleverAndSmart/Storage/Sqlite3Storage.php on line 185
Stack trace:
   1. {main}() /myapp/vendor/phpunit/phpunit/phpunit:0
   2. PHPUnit_TextUI_Command::main() /myapp/vendor/phpunit/phpunit/phpunit:36
   3. PHPUnit_TextUI_Command->run() /myapp/vendor/phpunit/phpunit/src/TextUI/Command.php:105
   4. PHPUnit_TextUI_TestRunner->doRun() /myapp/vendor/phpunit/phpunit/src/TextUI/Command.php:153
   5. PHPUnit_TextUI_TestRunner->handleConfiguration() /myapp/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:153
   6. PHPUnit_Util_Configuration->getListenerConfiguration() /myapp/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:714
   7. PHPUnit_Util_XML::xmlToVariable() /myapp/vendor/phpunit/phpunit/src/Util/Configuration.php:342
   8. PHPUnit\Runner\CleverAndSmart\Storage\Sqlite3Storage->__construct() /myapp/vendor/phpunit/phpunit/src/Util/XML.php:245
   9. PHPUnit\Runner\CleverAndSmart\Storage\Sqlite3Storage->query() /myapp/vendor/lstrojny/phpunit-clever-and-smart/src/PHPUnit/Runner/CleverAndSmart/Storage/Sqlite3Storage.php:34
  10. PHPUnit\Runner\CleverAndSmart\Storage\Sqlite3Storage->doQuery() /myapp/vendor/lstrojny/phpunit-clever-and-smart/src/PHPUnit/Runner/CleverAndSmart/Storage/Sqlite3Storage.php:159
  11. SQLite3->query() /myapp/vendor/lstrojny/phpunit-clever-and-smart/src/PHPUnit/Runner/CleverAndSmart/Storage/Sqlite3Storage.php:185
 Fatal error:  Uncaught exception 'PHPUnit\Runner\CleverAndSmart\Exception\StorageException' with message 'database is locked (error code 5)' in /myapp/vendor/lstrojny/phpunit-clever-and-smart/src/PHPUnit/Runner/CleverAndSmart/Exception/StorageException.php:8

Maybe we can change this behaviour to "wait a second and try again" or at least to ignore this exception (maybe print a warning to stderr).

lstrojny commented 9 years ago

When running multiple tests in parallel I would recommend to have different sqlite files per run. Is that an option?

mpscholten commented 9 years ago

Not really, I guess. By using multiple sqlite files per run we'd loose the advantages of phpunit-clever-and-smart.

Some more context: The issue happens when running the tests inside a docker container. Let's say we have a docker container running the phpunit test suite (with a pseudo tty attached, so that you can see the output from phpunit). When you now press CTRL+C, docker will detach the tty from the container but the tests will keep running in the background. If you now change some code and then re-run the tests in another docker container (while the other container has still not finished running the tests) you might get the error from above. It's not happening very often, but when it happens it's very annoying.

staabm commented 9 years ago

@mpscholten did you consider using a different Storage in your setup?

maybe you could also use a different locking-mode[1]? I dont have much experience with SQLite so this might not be a viable option after all

[1] https://github.com/lstrojny/phpunit-clever-and-smart/blob/master/src/PHPUnit/Runner/CleverAndSmart/Storage/Sqlite3Storage.php#L35

mpscholten commented 9 years ago

Good idea, haven't thought about that yet.

I guess setting the locking-mode to PENDING (A PENDING lock means that the process holding the lock wants to write to the database as soon as possible and is just waiting on all current SHARED locks to clear so that it can get an EXCLUSIVE lock. No new SHARED locks are permitted against the database if a PENDING lock is active, though existing SHARED locks are allowed to continue.) should do the job. If I understood this right, the pending lock waits until the database is unlocked and then applies an exclusive lock.

This looks like a great solution. So, should we change the default lock to PENDING? This should not change behavior for single-test-run environments, but will fix this issue for multi-test-run environments. If everyone is happy with this I'd prepare a pull request :)

lstrojny commented 9 years ago

I don't know enough about SQLite to judge but if that helps I am certainly fine with it.

mpscholten commented 9 years ago

Just tested it. PENDING doesn't work with EXCLUSIVE. But I just found the real issue :) As long as the Sqlite3Storage object is living the database is locked (see https://github.com/lstrojny/phpunit-clever-and-smart/blob/master/src/PHPUnit/Runner/CleverAndSmart/Storage/Sqlite3Storage.php#L35). So the second process cannot read/write from/to the database while the first process is still running. But sqlite is automatically getting a lock when firing a query. So we can safely remove line 35 in Sqlite3Storage and then the second process can access the database as long as the first process is not writing to it at the moment.

Just removed the line in my local copy and tried it out. Seems to be working.

staabm commented 8 years ago

@lstrojny we can close this as https://github.com/lstrojny/phpunit-clever-and-smart/pull/37 was merged

lstrojny commented 8 years ago

Thanks!