From 044cc641e6be0c20b5c70dc214731fd36ec31b66 Mon Sep 17 00:00:00 2001 From: Davide Bellettini <325358+dbellettini@users.noreply.github.com.> Date: Sat, 2 Aug 2025 18:47:39 +0200 Subject: [PATCH 1/6] Copy infrastructure from precious --- .github/workflows/phpunit.yml | 38 ++++++++++++++++++++++++++++++++ Dockerfile | 34 +++++++++++++++++++++++++++++ Makefile | 41 +++++++++++++++++++++++++++++++++++ compose.yaml | 8 +++++++ 4 files changed, 121 insertions(+) create mode 100644 .github/workflows/phpunit.yml create mode 100644 Dockerfile create mode 100644 Makefile create mode 100644 compose.yaml diff --git a/.github/workflows/phpunit.yml b/.github/workflows/phpunit.yml new file mode 100644 index 0000000..fe1ae53 --- /dev/null +++ b/.github/workflows/phpunit.yml @@ -0,0 +1,38 @@ +name: PHPUnit Tests + +on: + push: + branches: [ "master" ] + pull_request: + branches: [ "master" ] + +permissions: + contents: read + +jobs: + build: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v4 + + - name: Validate composer.json and composer.lock + run: composer validate --strict + + - name: Cache Composer packages + id: composer-cache + uses: actions/cache@v3 + with: + path: vendor + key: ${{ runner.os }}-php-${{ hashFiles('**/composer.lock') }} + restore-keys: | + ${{ runner.os }}-php- + + - name: Build the Docker image + run: make build + + - name: Install dependencies + run: make install + + - name: Run test suite + run: make test \ No newline at end of file diff --git a/Dockerfile b/Dockerfile new file mode 100644 index 0000000..da99830 --- /dev/null +++ b/Dockerfile @@ -0,0 +1,34 @@ +FROM php:8.4-cli + +# Install system dependencies +RUN apt-get update && apt-get install -y \ + git \ + unzip \ + libssl-dev \ + libcurl4-openssl-dev \ + pkg-config \ + && rm -rf /var/lib/apt/lists/* + +# Install MongoDB extension +RUN pecl install mongodb \ + && docker-php-ext-enable mongodb + +# Copy Composer from official image +COPY --from=composer:latest /usr/bin/composer /usr/bin/composer + +# Set working directory +WORKDIR /app + +# Copy composer files +COPY composer.json composer.lock* ./ + +# Install dependencies including dev dependencies for testing +RUN composer install --optimize-autoloader + +# Copy application code +COPY . . + +# Set environment variable for Composer +ENV COMPOSER_ALLOW_SUPERUSER=1 + +CMD ["tail", "-f", "/dev/null"] diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..81cba80 --- /dev/null +++ b/Makefile @@ -0,0 +1,41 @@ +.PHONY: build up down test test-unit fix-cs install shell logs clean + +# Build the Docker image +build: + docker compose build + +# Start the services +up: + docker compose up -d + +# Stop the services +down: + docker compose down + +# Install dependencies +install: + docker compose run --rm php composer install + +# Run all tests +test: up + docker compose exec php vendor/bin/phpunit + +# Run unit tests specifically +test-unit: up + docker compose exec php vendor/bin/phpunit tests/unit + +fix-cs: up + docker compose exec php vendor/bin/php-cs-fixer fix -v + +# Open a shell in the PHP container +shell: + docker compose exec php bash + +# View logs +logs: + docker compose logs -f php + +# Clean up containers and volumes +clean: + docker compose down -v + docker compose rm -f diff --git a/compose.yaml b/compose.yaml new file mode 100644 index 0000000..5509e13 --- /dev/null +++ b/compose.yaml @@ -0,0 +1,8 @@ +services: + php: + build: . + working_dir: /app + volumes: + - .:/app + environment: + - COMPOSER_ALLOW_SUPERUSER=1 From 68630a140088e54b7a877d0c80058b979d94672b Mon Sep 17 00:00:00 2001 From: Davide Bellettini <325358+dbellettini@users.noreply.github.com.> Date: Sat, 2 Aug 2025 19:11:41 +0200 Subject: [PATCH 2/6] Add JRE --- Dockerfile | 1 + 1 file changed, 1 insertion(+) diff --git a/Dockerfile b/Dockerfile index da99830..5e745ec 100644 --- a/Dockerfile +++ b/Dockerfile @@ -7,6 +7,7 @@ RUN apt-get update && apt-get install -y \ libssl-dev \ libcurl4-openssl-dev \ pkg-config \ + openjdk-17-jre-headless \ && rm -rf /var/lib/apt/lists/* # Install MongoDB extension From e3ae60f6158ddfd11311d1cf6157365c742bae58 Mon Sep 17 00:00:00 2001 From: Davide Bellettini <325358+dbellettini@users.noreply.github.com> Date: Sat, 2 Aug 2025 20:14:46 +0200 Subject: [PATCH 3/6] Upgrade deps and fix errors --- .gitignore | 2 +- compose.yaml | 15 +++++ composer.json | 29 +++++---- phpunit.xml | 14 ++++- src/Recruiter/Concurrency/PeriodicalCheck.php | 23 ++++--- .../Concurrency/InProcessRetryTest.php | 43 ++++++++++--- tests/Recruiter/Concurrency/MongoLockTest.php | 61 +++++++++---------- .../Concurrency/PeriodicalCheckTest.php | 2 +- tests/Recruiter/Concurrency/mongolock.php | 8 ++- 9 files changed, 128 insertions(+), 69 deletions(-) diff --git a/.gitignore b/.gitignore index 0cd4874..9f91b68 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,4 @@ vendor/ composer.lock .idea/ -.php_cs.cache +.*.cache diff --git a/compose.yaml b/compose.yaml index 5509e13..d49c078 100644 --- a/compose.yaml +++ b/compose.yaml @@ -6,3 +6,18 @@ services: - .:/app environment: - COMPOSER_ALLOW_SUPERUSER=1 + - MONGODB_URI=mongodb://mongodb:27017/concurrency + depends_on: + - mongodb + + mongodb: + image: mongo:7 + container_name: concurrency_mongodb + restart: unless-stopped + ports: + - "27017:27017" + volumes: + - mongodb_data:/data/db + +volumes: + mongodb_data: diff --git a/composer.json b/composer.json index fa8675d..7908034 100644 --- a/composer.json +++ b/composer.json @@ -3,24 +3,31 @@ "description": "MongoDB-based locking system", "license": "MIT", "require": { - "php": "^7.2", - "ext-mongodb": ">=1.2", - "recruiterphp/clock": "^3", - "mongodb/mongodb": "^1.4" + "php": "^8.4", + "ext-mongodb": "*", + "mongodb/mongodb": "^2.1", + "recruiterphp/clock": "^4.1" }, "require-dev": { - "phpunit/phpunit": "@stable", - "phake/phake": "~2.1", - "giorgiosironi/eris": "dev-master", - "symfony/process": "~4.0", - "friendsofphp/php-cs-fixer": "^2.13" + "ergebnis/composer-normalize": "^2.47", + "friendsofphp/php-cs-fixer": "^3.85", + "giorgiosironi/eris": "^1.0", + "phake/phake": "^4.6", + "phpunit/phpunit": "^12.3", + "symfony/process": "^7.3" + }, + "replace": { + "easywelfare/onebip-concurrency": "self.version", + "onebip/onebip-concurrency": "self.version" }, "autoload": { "psr-0": { "Recruiter\\": "src/" } }, - "replace": { - "recruiterphp/concurrency": "self.version" + "config": { + "allow-plugins": { + "ergebnis/composer-normalize": true + } } } diff --git a/phpunit.xml b/phpunit.xml index cc287a4..296ff04 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -1,7 +1,19 @@ - + + tests + + + src + + diff --git a/src/Recruiter/Concurrency/PeriodicalCheck.php b/src/Recruiter/Concurrency/PeriodicalCheck.php index ef33bf1..ecc16da 100644 --- a/src/Recruiter/Concurrency/PeriodicalCheck.php +++ b/src/Recruiter/Concurrency/PeriodicalCheck.php @@ -7,12 +7,10 @@ class PeriodicalCheck { - private $check; - private $clock; - private $lastCheck; - private $seconds; + private array|\Closure $check; + private int $lastCheck; - public static function every($seconds, Clock $clock = null) + public static function every(int $seconds, ?Clock $clock = null): self { if (null === $clock) { $clock = new SystemClock(); @@ -21,26 +19,27 @@ public static function every($seconds, Clock $clock = null) return new self($seconds, $clock); } - private function __construct($seconds, $clock) + private function __construct(private readonly int $seconds, private readonly Clock $clock) { - $this->seconds = $seconds; - $this->clock = $clock; $this->lastCheck = 0; } - public function onFire(callable $check) + /** + * @return $this + */ + public function onFire(callable $check): self { $this->check = $check; return $this; } - public function __invoke() + public function __invoke(): void { - return $this->execute(); + $this->execute(); } - public function execute() + public function execute(): void { $now = $this->clock->current()->getTimestamp(); if ($now - $this->lastCheck >= $this->seconds) { diff --git a/tests/Recruiter/Concurrency/InProcessRetryTest.php b/tests/Recruiter/Concurrency/InProcessRetryTest.php index 87ac722..4c8dc59 100644 --- a/tests/Recruiter/Concurrency/InProcessRetryTest.php +++ b/tests/Recruiter/Concurrency/InProcessRetryTest.php @@ -4,11 +4,16 @@ use Exception; use InvalidArgumentException; +use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; +#[CoversClass(InProcessRetry::class)] class InProcessRetryTest extends TestCase { - public function setUp() + private int $count; + private \Closure $counter; + + protected function setUp(): void { $this->count = 0; $this->counter = function () { @@ -18,7 +23,7 @@ public function setUp() }; } - private function exceptionalCounterFactory($exceptionClass) + private function exceptionalCounterFactory(string $exceptionClass): \Closure { return function () use ($exceptionClass) { ++$this->count; @@ -26,7 +31,7 @@ private function exceptionalCounterFactory($exceptionClass) }; } - private function limitedExceptionalCounterFactory($exceptionClass, $limit) + private function limitedExceptionalCounterFactory(string $exceptionClass, int $limit): \Closure { return function () use ($exceptionClass, &$limit) { ++$this->count; @@ -39,20 +44,29 @@ private function limitedExceptionalCounterFactory($exceptionClass, $limit) }; } - public function testPerformsOnceATaskIfItIsSuccessful() + /** + * @throws Exception + */ + public function testPerformsOnceATaskIfItIsSuccessful(): void { $retry = InProcessRetry::of($this->counter, 'InvalidArgumentException'); $retry->__invoke(); $this->assertEquals(1, $this->count); } - public function testReturnsTheValueReturnedByTheTask() + /** + * @throws Exception + */ + public function testReturnsTheValueReturnedByTheTask(): void { $retry = InProcessRetry::of($this->counter, 'InvalidArgumentException'); $this->assertEquals(1, $retry->__invoke()); } - public function testInCaseOfSpecifiedExceptionRetriesOnceByDefault() + /** + * @throws Exception + */ + public function testInCaseOfSpecifiedExceptionRetriesOnceByDefault(): void { $retry = InProcessRetry::of($this->exceptionalCounterFactory('InvalidArgumentException'), 'InvalidArgumentException'); try { @@ -63,13 +77,16 @@ public function testInCaseOfSpecifiedExceptionRetriesOnceByDefault() $this->assertEquals(2, $this->count); } - public function testInCaseOfSpecifiedExceptionRetriesReturnTheOriginalValueReturnedByTheTask() + /** + * @throws Exception + */ + public function testInCaseOfSpecifiedExceptionRetriesReturnTheOriginalValueReturnedByTheTask(): void { $retry = InProcessRetry::of($this->limitedExceptionalCounterFactory('InvalidArgumentException', 1), 'InvalidArgumentException'); $this->assertEquals(2, $retry->__invoke()); } - public function testInCaseOfGenericExceptionDoesNotRetry() + public function testInCaseOfGenericExceptionDoesNotRetry(): void { $retry = InProcessRetry::of($this->exceptionalCounterFactory('Exception'), 'InvalidArgumentException'); try { @@ -80,7 +97,10 @@ public function testInCaseOfGenericExceptionDoesNotRetry() $this->assertEquals(1, $this->count); } - public function testCanPerformMultipleRetriesUntilAnHappyReturn() + /** + * @throws Exception + */ + public function testCanPerformMultipleRetriesUntilAnHappyReturn(): void { $failures = 4; $retry = InProcessRetry::of($this->limitedExceptionalCounterFactory('InvalidArgumentException', $failures), 'InvalidArgumentException') @@ -88,7 +108,10 @@ public function testCanPerformMultipleRetriesUntilAnHappyReturn() $this->assertEquals($totalCalls = $failures + 1, $retry->__invoke()); } - public function testCanPerformMultipleRetriesUntilTheyAreFinished() + /** + * @throws Exception + */ + public function testCanPerformMultipleRetriesUntilTheyAreFinished(): void { $failures = 4; $retry = InProcessRetry::of($this->exceptionalCounterFactory('InvalidArgumentException', $failures), 'InvalidArgumentException') diff --git a/tests/Recruiter/Concurrency/MongoLockTest.php b/tests/Recruiter/Concurrency/MongoLockTest.php index fbb7d4e..7475dfb 100644 --- a/tests/Recruiter/Concurrency/MongoLockTest.php +++ b/tests/Recruiter/Concurrency/MongoLockTest.php @@ -7,22 +7,26 @@ use Eris\Generator; use MongoDB; use Phake; +use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\TestCase; +use Recruiter\Clock; use Symfony\Component\Process\Process; class MongoLockTest extends TestCase { use Eris\TestTrait; - private $lockCollection; - private $clock; - private $slept; - private $sleep; + private MongoDB\Collection $lockCollection; + private Clock&Phake\IMock $clock; + private array $slept; + private \Closure $sleep; + private int $iteration; - public function setUp() + protected function setUp(): void { - $this->lockCollection = (new MongoDB\Client())->test->lock; - $this->clock = Phake::mock('Recruiter\Clock'); + $uri = getenv('MONGODB_URI') ?: null; + $this->lockCollection = new MongoDB\Client($uri)->selectCollection('concurrency-test', 'lock'); + $this->clock = Phake::mock(Clock::class); $this->slept = []; $this->sleep = function ($amount) { @@ -30,7 +34,7 @@ public function setUp() }; } - public function tearDown() + protected function tearDown(): void { $this->lockCollection->drop(); } @@ -43,10 +47,6 @@ public function testALockCanBeAcquired() $this->assertTrue(true, 'make PHPUnit happy'); } - /** - * @expectedException \Recruiter\Concurrency\LockNotAvailableException - * @expectedExceptionMessage ws-a-30:23 cannot acquire a lock for the program windows_defrag - */ public function testAnAlreadyAcquiredLockCannotBeAcquiredAgain() { $this->givenTimeIsFixed(); @@ -54,14 +54,13 @@ public function testAnAlreadyAcquiredLockCannotBeAcquiredAgain() $first->acquire(); $second = new MongoLock($this->lockCollection, 'windows_defrag', 'ws-a-30:23', $this->clock); + + $this->expectExceptionMessage("ws-a-30:23 cannot acquire a lock for the program windows_defrag"); + $this->expectException(LockNotAvailableException::class); + $second->acquire(); - // $this->assertTrue(true, 'make PHPUnit happy'); } - /** - * @expectedException \Recruiter\Concurrency\LockNotAvailableException - * @expectedExceptionMessage ws-a-30:23 cannot acquire a lock for the program windows_defrag - */ public function testAnAlreadyAcquiredLockCannotBeAcquiredAgainEvenWithRefreshMethod() { $this->givenTimeIsFixed(); @@ -69,6 +68,10 @@ public function testAnAlreadyAcquiredLockCannotBeAcquiredAgainEvenWithRefreshMet $first->acquire(); $second = new MongoLock($this->lockCollection, 'windows_defrag', 'ws-a-30:23', $this->clock); + + $this->expectExceptionMessage("ws-a-30:23 cannot acquire a lock for the program windows_defrag"); + $this->expectException(LockNotAvailableException::class); + $second->refresh(); // $this->assertTrue(true, 'make PHPUnit happy'); } @@ -110,16 +113,15 @@ public function testLocksCanBeReleasedToMakeThemAvailableAgain() $this->assertTrue(true, 'make PHPUnit happy'); } - /** - * @expectedException \Recruiter\Concurrency\LockNotAvailableException - * @expectedExceptionMessage ws-a-30:23 does not have a lock for windows_defrag to release - */ public function testALockCannotBeReleasedBySomeoneElseThanTheProcessAcquiringIt() { $this->givenTimeIsFixed(); $first = new MongoLock($this->lockCollection, 'windows_defrag', 'ws-a-25:42', $this->clock); $first->acquire(); + $this->expectExceptionMessage("ws-a-30:23 does not have a lock for windows_defrag to release"); + $this->expectException(LockNotAvailableException::class); + $second = new MongoLock($this->lockCollection, 'windows_defrag', 'ws-a-30:23', $this->clock); $second->release(); } @@ -237,10 +239,6 @@ public function testAnAlreadyAcquiredLockCanBeRefreshed() ); } - /** - * @expectedException \Recruiter\Concurrency\LockNotAvailableException - * @expectedExceptionMessage ws-a-25:42 cannot acquire a lock for the program windows_defrag - */ public function testAnExpiredLockCannotBeRefreshed() { Phake::when($this->clock)->current() @@ -250,6 +248,9 @@ public function testAnExpiredLockCannotBeRefreshed() $first = new MongoLock($this->lockCollection, 'windows_defrag', 'ws-a-25:42', $this->clock); $first->acquire(); + $this->expectExceptionMessage("ws-a-25:42 cannot acquire a lock for the program windows_defrag"); + $this->expectException(LockNotAvailableException::class); + $second = new MongoLock($this->lockCollection, 'windows_defrag', 'ws-a-25:42', $this->clock); $second->refresh(); } @@ -259,9 +260,7 @@ private function givenTimeIsFixed() Phake::when($this->clock)->current()->thenReturn(new DateTime('2014-01-01')); } - /** - * @group long - */ + #[Group('long')] public function testPropertyBased() { $this->iteration = 0; @@ -295,7 +294,7 @@ public function testPropertyBased() foreach ($sequencesOfSteps as $i => $sequence) { $processName = "p{$i}"; $steps = implode(',', $sequence); - $process = new Process('exec php ' . __DIR__ . "/mongolock.php $processName $steps >> $log"); + $process = Process::fromShellCommandline('exec php ' . __DIR__ . "/mongolock.php $processName $steps >> $log"); $process->start(); $processes[] = $process; } @@ -303,14 +302,14 @@ public function testPropertyBased() $process->wait(); $this->assertExitedCorrectly($process, 'Error in MongoLock run'); } - $process = new Process('exec java -jar ' . __DIR__ . "/knossos-recruiterphp.jar mongo-lock $log"); + $process = Process::fromShellCommandline('exec java -jar ' . __DIR__ . "/knossos-recruiterphp.jar mongo-lock $log"); $process->run(); $this->assertExitedCorrectly($process, "Non-linearizable history in $log"); ++$this->iteration; }); } - private function assertExitedCorrectly($process, $errorMessage) + private function assertExitedCorrectly($process, $errorMessage): void { $this->assertEquals( 0, diff --git a/tests/Recruiter/Concurrency/PeriodicalCheckTest.php b/tests/Recruiter/Concurrency/PeriodicalCheckTest.php index a658e58..fbbb1fd 100644 --- a/tests/Recruiter/Concurrency/PeriodicalCheckTest.php +++ b/tests/Recruiter/Concurrency/PeriodicalCheckTest.php @@ -12,7 +12,7 @@ class PeriodicalCheckTest extends TestCase { use Eris\TestTrait; - private $counter; + private int $counter; public function testDoesNotPerformTheCheckTooManyTimes() { diff --git a/tests/Recruiter/Concurrency/mongolock.php b/tests/Recruiter/Concurrency/mongolock.php index b590b04..ea3a24d 100644 --- a/tests/Recruiter/Concurrency/mongolock.php +++ b/tests/Recruiter/Concurrency/mongolock.php @@ -17,7 +17,8 @@ } $operations = explode(',', $argv[2]); -$lockCollection = (new MongoDB\Client())->test->lock; +$uri = getenv('MONGODB_URI') ?: null; +$lockCollection = new MongoDB\Client($uri)->selectCollection('concurrency-test', 'lock'); $lock = new MongoLock($lockCollection, 'ilium_gate', $name); $log = function ($data) { fputcsv( @@ -27,7 +28,10 @@ 'time' => (int) (microtime(true) * 1000000), ], $data - ) + ), + ',', + '"', + '', ); }; foreach ($operations as $operation) { From e718cbfa901d0d7d2f3e931b917baf7f83da9d55 Mon Sep 17 00:00:00 2001 From: Davide Bellettini <325358+dbellettini@users.noreply.github.com> Date: Sat, 2 Aug 2025 20:21:21 +0200 Subject: [PATCH 4/6] Fix CS --- .php-cs-fixer.dist.php | 25 +++++ src/Recruiter/Concurrency/Idle.php | 2 + src/Recruiter/Concurrency/InProcessRetry.php | 9 +- src/Recruiter/Concurrency/Leadership.php | 2 + src/Recruiter/Concurrency/Lock.php | 13 ++- .../Concurrency/LockNotAvailableException.php | 6 +- src/Recruiter/Concurrency/MongoLock.php | 36 +++---- .../Concurrency/MongoLockRepository.php | 2 + src/Recruiter/Concurrency/NoPatience.php | 2 + src/Recruiter/Concurrency/NullLock.php | 10 +- src/Recruiter/Concurrency/Patience.php | 2 + src/Recruiter/Concurrency/PeriodicalCheck.php | 2 + src/Recruiter/Concurrency/Poison.php | 6 +- .../Concurrency/ProcessRequestedStatus.php | 2 + src/Recruiter/Concurrency/Timeout.php | 2 + .../Concurrency/TimeoutException.php | 2 + src/Recruiter/Concurrency/TimeoutPatience.php | 2 + .../Concurrency/InProcessRetryTest.php | 28 ++--- tests/Recruiter/Concurrency/MongoLockTest.php | 101 ++++++++++-------- .../Concurrency/PeriodicalCheckTest.php | 11 +- tests/Recruiter/Concurrency/mongolock.php | 4 +- 21 files changed, 161 insertions(+), 108 deletions(-) create mode 100644 .php-cs-fixer.dist.php diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php new file mode 100644 index 0000000..b33e48c --- /dev/null +++ b/.php-cs-fixer.dist.php @@ -0,0 +1,25 @@ +setRiskyAllowed(true) + ->setRules([ + '@PSR12' => true, + '@Symfony' => true, + 'array_indentation' => true, + 'array_syntax' => ['syntax' => 'short'], + 'concat_space' => ['spacing' => 'one'], + 'declare_strict_types' => true, + 'string_implicit_backslashes' => true, + 'list_syntax' => ['syntax' => 'short'], + 'multiline_whitespace_before_semicolons' => ['strategy' => 'new_line_for_chained_calls'], + 'ordered_imports' => true, + 'phpdoc_to_comment' => false, + 'trailing_comma_in_multiline' => ['elements' => ['arrays', 'arguments', 'parameters']], + 'visibility_required' => ['elements' => ['property', 'method', 'const']], + ]) + ->setFinder( + PhpCsFixer\Finder::create() + ->in(__DIR__ . '/src') + ->in(__DIR__ . '/tests') + ) +; diff --git a/src/Recruiter/Concurrency/Idle.php b/src/Recruiter/Concurrency/Idle.php index 1bcc2eb..3d77aab 100644 --- a/src/Recruiter/Concurrency/Idle.php +++ b/src/Recruiter/Concurrency/Idle.php @@ -1,5 +1,7 @@ what); - } catch (Exception $e) { + } catch (\Exception $e) { if (!($e instanceof $this->exceptionClass)) { throw $e; } } } - throw $e ?? new InvalidArgumentException("Invalid number of retries: $possibleRetries"); + throw $e ?? new \InvalidArgumentException("Invalid number of retries: $possibleRetries"); } } diff --git a/src/Recruiter/Concurrency/Leadership.php b/src/Recruiter/Concurrency/Leadership.php index fd23ed3..8f5d24c 100644 --- a/src/Recruiter/Concurrency/Leadership.php +++ b/src/Recruiter/Concurrency/Leadership.php @@ -1,5 +1,7 @@ removeExpiredLocks($now); $expiration = clone $now; - $expiration->add(new DateInterval("PT{$duration}S")); + $expiration->add(new \DateInterval("PT{$duration}S")); try { $document = [ @@ -56,9 +56,7 @@ public function acquire($duration = 3600): void $this->collection->insertOne($document); } catch (BulkWriteException $e) { if (self::DUPLICATE_KEY == $e->getCode()) { - throw new LockNotAvailableException( - "{$this->processName} cannot acquire a lock for the program {$this->programName}" - ); + throw new LockNotAvailableException("{$this->processName} cannot acquire a lock for the program {$this->programName}"); } throw $e; } @@ -71,17 +69,15 @@ public function refresh($duration = 3600): void $this->removeExpiredLocks($now); $expiration = clone $now; - $expiration->add(new DateInterval("PT{$duration}S")); + $expiration->add(new \DateInterval("PT{$duration}S")); $result = $this->collection->updateOne( ['program' => $this->programName, 'process' => $this->processName], - ['$set' => ['expires_at' => new UTCDateTime($expiration)]] + ['$set' => ['expires_at' => new UTCDateTime($expiration)]], ); if (!$this->lockRefreshed($result)) { - throw new LockNotAvailableException( - "{$this->processName} cannot acquire a lock for the program {$this->programName}, result is: " . var_export($result, true) - ); + throw new LockNotAvailableException("{$this->processName} cannot acquire a lock for the program {$this->programName}, result is: " . var_export($result, true)); } } @@ -89,7 +85,7 @@ public function show(): ?array { $document = $this->collection->findOne( ['program' => $this->programName, 'process' => $this->processName], - ['typeMap' => ['root' => 'array']] + ['typeMap' => ['root' => 'array']], ); if (!is_null($document)) { @@ -109,9 +105,7 @@ public function release($force = false): void } $operationResult = $this->collection->deleteMany($query); if (1 !== $operationResult->getDeletedCount()) { - throw new LockNotAvailableException( - "{$this->processName} does not have a lock for {$this->programName} to release" - ); + throw new LockNotAvailableException("{$this->processName} does not have a lock for {$this->programName} to release"); } } @@ -121,7 +115,7 @@ public function release($force = false): void */ public function wait($polling = 30, $maximumWaitingTime = 3600): void { - $timeLimit = $this->clock->current()->add(new DateInterval("PT{$maximumWaitingTime}S")); + $timeLimit = $this->clock->current()->add(new \DateInterval("PT{$maximumWaitingTime}S")); while (true) { $now = $this->clock->current(); $result = $this->collection->count($query = [ @@ -131,9 +125,7 @@ public function wait($polling = 30, $maximumWaitingTime = 3600): void if ($result) { if ($now > $timeLimit) { - throw new LockNotAvailableException( - "I have been waiting up until {$timeLimit->format(DateTime::ATOM)} for the lock $this->programName ($maximumWaitingTime seconds polling every $polling seconds), but it is still not available (now is {$now->format(DateTime::ATOM)})." - ); + throw new LockNotAvailableException("I have been waiting up until {$timeLimit->format(\DateTime::ATOM)} for the lock $this->programName ($maximumWaitingTime seconds polling every $polling seconds), but it is still not available (now is {$now->format(\DateTime::ATOM)})."); } call_user_func($this->sleep, $polling); } else { @@ -147,7 +139,7 @@ public function __toString(): string return var_export($this->show(), true); } - private function removeExpiredLocks(DateTime $now): void + private function removeExpiredLocks(\DateTime $now): void { $this->collection->deleteMany($query = [ 'program' => $this->programName, @@ -161,7 +153,7 @@ private function convertToIso8601String(UTCDateTime $mongoDateTime): string { $datetime = $mongoDateTime->toDateTime(); - return $datetime->format(DateTime::ATOM); + return $datetime->format(\DateTime::ATOM); } private function lockRefreshed($result): bool diff --git a/src/Recruiter/Concurrency/MongoLockRepository.php b/src/Recruiter/Concurrency/MongoLockRepository.php index a8aa4d8..2e96b11 100644 --- a/src/Recruiter/Concurrency/MongoLockRepository.php +++ b/src/Recruiter/Concurrency/MongoLockRepository.php @@ -1,5 +1,7 @@ __invoke(); $this->fail('Should let the 2nd InvalidArgumentException bubble up'); - } catch (InvalidArgumentException $e) { + } catch (\InvalidArgumentException $e) { } $this->assertEquals(2, $this->count); } /** - * @throws Exception + * @throws \Exception */ public function testInCaseOfSpecifiedExceptionRetriesReturnTheOriginalValueReturnedByTheTask(): void { @@ -92,34 +92,36 @@ public function testInCaseOfGenericExceptionDoesNotRetry(): void try { $retry->__invoke(); $this->fail('Should let the 1st Exception bubble up'); - } catch (Exception $e) { + } catch (\Exception $e) { } $this->assertEquals(1, $this->count); } /** - * @throws Exception + * @throws \Exception */ public function testCanPerformMultipleRetriesUntilAnHappyReturn(): void { $failures = 4; $retry = InProcessRetry::of($this->limitedExceptionalCounterFactory('InvalidArgumentException', $failures), 'InvalidArgumentException') - ->forTimes($failures); + ->forTimes($failures) + ; $this->assertEquals($totalCalls = $failures + 1, $retry->__invoke()); } /** - * @throws Exception + * @throws \Exception */ public function testCanPerformMultipleRetriesUntilTheyAreFinished(): void { $failures = 4; $retry = InProcessRetry::of($this->exceptionalCounterFactory('InvalidArgumentException', $failures), 'InvalidArgumentException') - ->forTimes($failures); + ->forTimes($failures) + ; try { $retry->__invoke(); $this->fail('Even multiple invocations should always fail.'); - } catch (InvalidArgumentException $e) { + } catch (\InvalidArgumentException $e) { $this->assertEquals($totalCalls = $failures + 1, $this->count); } } diff --git a/tests/Recruiter/Concurrency/MongoLockTest.php b/tests/Recruiter/Concurrency/MongoLockTest.php index 7475dfb..bf2ac4c 100644 --- a/tests/Recruiter/Concurrency/MongoLockTest.php +++ b/tests/Recruiter/Concurrency/MongoLockTest.php @@ -1,8 +1,9 @@ lockCollection = new MongoDB\Client($uri)->selectCollection('concurrency-test', 'lock'); - $this->clock = Phake::mock(Clock::class); + $this->clock = \Phake::mock(Clock::class); $this->slept = []; $this->sleep = function ($amount) { @@ -55,7 +56,7 @@ public function testAnAlreadyAcquiredLockCannotBeAcquiredAgain() $second = new MongoLock($this->lockCollection, 'windows_defrag', 'ws-a-30:23', $this->clock); - $this->expectExceptionMessage("ws-a-30:23 cannot acquire a lock for the program windows_defrag"); + $this->expectExceptionMessage('ws-a-30:23 cannot acquire a lock for the program windows_defrag'); $this->expectException(LockNotAvailableException::class); $second->acquire(); @@ -69,7 +70,7 @@ public function testAnAlreadyAcquiredLockCannotBeAcquiredAgainEvenWithRefreshMet $second = new MongoLock($this->lockCollection, 'windows_defrag', 'ws-a-30:23', $this->clock); - $this->expectExceptionMessage("ws-a-30:23 cannot acquire a lock for the program windows_defrag"); + $this->expectExceptionMessage('ws-a-30:23 cannot acquire a lock for the program windows_defrag'); $this->expectException(LockNotAvailableException::class); $second->refresh(); @@ -78,9 +79,9 @@ public function testAnAlreadyAcquiredLockCannotBeAcquiredAgainEvenWithRefreshMet public function testAnAlreadyAcquiredLockCanExpireSoThatItCanBeAcquiredAgain() { - Phake::when($this->clock)->current() - ->thenReturn(new DateTime('2014-01-01T10:00:00Z')) - ->thenReturn(new DateTime('2014-01-01T11:00:01Z')) + \Phake::when($this->clock)->current() + ->thenReturn(new \DateTime('2014-01-01T10:00:00Z')) + ->thenReturn(new \DateTime('2014-01-01T11:00:01Z')) ; $first = new MongoLock($this->lockCollection, 'windows_defrag', 'ws-a-25:42', $this->clock); $first->acquire(3600); @@ -119,7 +120,7 @@ public function testALockCannotBeReleasedBySomeoneElseThanTheProcessAcquiringIt( $first = new MongoLock($this->lockCollection, 'windows_defrag', 'ws-a-25:42', $this->clock); $first->acquire(); - $this->expectExceptionMessage("ws-a-30:23 does not have a lock for windows_defrag to release"); + $this->expectExceptionMessage('ws-a-30:23 does not have a lock for windows_defrag to release'); $this->expectException(LockNotAvailableException::class); $second = new MongoLock($this->lockCollection, 'windows_defrag', 'ws-a-30:23', $this->clock); @@ -139,8 +140,9 @@ public function testALockCanBeForcedToBeReleasedIfYouReallyKnowWhatYouReDoing() public function testALockCanBeShownEvenByOtherProcessesWorkingOnTheSameProgram() { - Phake::when($this->clock)->current() - ->thenReturn(new DateTime('2014-01-01T00:00:00Z')); + \Phake::when($this->clock)->current() + ->thenReturn(new \DateTime('2014-01-01T00:00:00Z')) + ; $first = new MongoLock($this->lockCollection, 'windows_defrag', 'ws-a-25:42', $this->clock); $first->acquire(3600); @@ -152,19 +154,20 @@ public function testALockCanBeShownEvenByOtherProcessesWorkingOnTheSameProgram() 'acquired_at' => '2014-01-01T00:00:00+00:00', 'expires_at' => '2014-01-01T01:00:00+00:00', ], - $second->show() + $second->show(), ); $this->assertTrue(true, 'make PHPUnit happy'); } public function testALockCanBeWaitedOnUntilItsDisappearance() { - $allCalls = Phake::when($this->clock)->current() - ->thenReturn(new DateTime('2014-01-01T00:00:00Z')) - ->thenReturn(new DateTime('2014-01-01T00:00:00Z')) - ->thenReturn(new DateTime('2014-01-01T00:00:00Z')) - ->thenReturn(new DateTime('2014-01-01T00:00:30Z')) - ->thenReturn(new DateTime('2014-01-01T00:01:00Z')); + $allCalls = \Phake::when($this->clock)->current() + ->thenReturn(new \DateTime('2014-01-01T00:00:00Z')) + ->thenReturn(new \DateTime('2014-01-01T00:00:00Z')) + ->thenReturn(new \DateTime('2014-01-01T00:00:00Z')) + ->thenReturn(new \DateTime('2014-01-01T00:00:30Z')) + ->thenReturn(new \DateTime('2014-01-01T00:01:00Z')) + ; $first = new MongoLock($this->lockCollection, 'windows_defrag', 'ws-a-25:42', $this->clock); $first->acquire(45); @@ -175,13 +178,14 @@ public function testALockCanBeWaitedOnUntilItsDisappearance() public function testALockShouldNotBeWaitedUponForever() { - $allCalls = Phake::when($this->clock)->current() - ->thenReturn(new DateTime('2014-01-01T00:00:00Z')) - ->thenReturn(new DateTime('2014-01-01T00:00:00Z')) - ->thenReturn(new DateTime('2014-01-01T00:00:30Z')) - ->thenReturn(new DateTime('2014-01-01T00:00:50Z')) - ->thenReturn(new DateTime('2014-01-01T00:01:01Z')) - ->thenThrow(new \LogicException('Should not call anymore')); + $allCalls = \Phake::when($this->clock)->current() + ->thenReturn(new \DateTime('2014-01-01T00:00:00Z')) + ->thenReturn(new \DateTime('2014-01-01T00:00:00Z')) + ->thenReturn(new \DateTime('2014-01-01T00:00:30Z')) + ->thenReturn(new \DateTime('2014-01-01T00:00:50Z')) + ->thenReturn(new \DateTime('2014-01-01T00:01:01Z')) + ->thenThrow(new \LogicException('Should not call anymore')) + ; $first = new MongoLock($this->lockCollection, 'windows_defrag', 'ws-a-25:42', $this->clock); $first->acquire(3600); @@ -192,21 +196,21 @@ public function testALockShouldNotBeWaitedUponForever() } catch (LockNotAvailableException $e) { $this->assertEquals( 'I have been waiting up until 2014-01-01T00:01:00+00:00 for the lock windows_defrag (60 seconds polling every 30 seconds), but it is still not available (now is 2014-01-01T00:01:01+00:00).', - $e->getMessage() + $e->getMessage(), ); } } public function testALockWaitedUponCanBeImmediatelyReacquired() { - $allCalls = Phake::when($this->clock)->current() - ->thenReturn(new DateTime('2014-01-01T00:00:00Z')) - ->thenReturn(new DateTime('2014-01-01T00:00:30Z')) - ->thenReturn(new DateTime('2014-01-01T00:00:30Z')) - ->thenReturn(new DateTime('2014-01-01T00:00:30Z')) - ->thenReturn(new DateTime('2014-01-01T00:00:31Z')) - ->thenReturn(new DateTime('2014-01-01T00:00:31Z')) - ; + $allCalls = \Phake::when($this->clock)->current() + ->thenReturn(new \DateTime('2014-01-01T00:00:00Z')) + ->thenReturn(new \DateTime('2014-01-01T00:00:30Z')) + ->thenReturn(new \DateTime('2014-01-01T00:00:30Z')) + ->thenReturn(new \DateTime('2014-01-01T00:00:30Z')) + ->thenReturn(new \DateTime('2014-01-01T00:00:31Z')) + ->thenReturn(new \DateTime('2014-01-01T00:00:31Z')) + ; $first = new MongoLock($this->lockCollection, 'windows_defrag', 'ws-a-25:42', $this->clock); $first->acquire(30); @@ -218,9 +222,10 @@ public function testALockWaitedUponCanBeImmediatelyReacquired() public function testAnAlreadyAcquiredLockCanBeRefreshed() { - Phake::when($this->clock)->current() - ->thenReturn(new DateTime('2014-01-01T00:00:00Z')) - ->thenReturn(new DateTime('2014-01-01T00:10:00Z')); + \Phake::when($this->clock)->current() + ->thenReturn(new \DateTime('2014-01-01T00:00:00Z')) + ->thenReturn(new \DateTime('2014-01-01T00:10:00Z')) + ; $first = new MongoLock($this->lockCollection, 'windows_defrag', 'ws-a-25:42', $this->clock); $first->acquire(); @@ -235,20 +240,21 @@ public function testAnAlreadyAcquiredLockCanBeRefreshed() 'acquired_at' => '2014-01-01T00:00:00+00:00', 'expires_at' => '2014-01-01T01:10:00+00:00', ], - $second->show() + $second->show(), ); } public function testAnExpiredLockCannotBeRefreshed() { - Phake::when($this->clock)->current() - ->thenReturn(new DateTime('2014-01-01T00:00:00Z')) - ->thenReturn(new DateTime('2014-01-01T02:00:00Z')); + \Phake::when($this->clock)->current() + ->thenReturn(new \DateTime('2014-01-01T00:00:00Z')) + ->thenReturn(new \DateTime('2014-01-01T02:00:00Z')) + ; $first = new MongoLock($this->lockCollection, 'windows_defrag', 'ws-a-25:42', $this->clock); $first->acquire(); - $this->expectExceptionMessage("ws-a-25:42 cannot acquire a lock for the program windows_defrag"); + $this->expectExceptionMessage('ws-a-25:42 cannot acquire a lock for the program windows_defrag'); $this->expectException(LockNotAvailableException::class); $second = new MongoLock($this->lockCollection, 'windows_defrag', 'ws-a-25:42', $this->clock); @@ -257,7 +263,7 @@ public function testAnExpiredLockCannotBeRefreshed() private function givenTimeIsFixed() { - Phake::when($this->clock)->current()->thenReturn(new DateTime('2014-01-01')); + \Phake::when($this->clock)->current()->thenReturn(new \DateTime('2014-01-01')); } #[Group('long')] @@ -269,10 +275,10 @@ public function testPropertyBased() Generator\vector( 2, Generator\seq( - Generator\elements(['acquire', 'release']) + Generator\elements(['acquire', 'release']), // TODO: add 'sleep' - ) - ) + ), + ), ) ->when(function ($sequencesOfSteps) { foreach ($sequencesOfSteps as $sequence) { @@ -306,7 +312,8 @@ public function testPropertyBased() $process->run(); $this->assertExitedCorrectly($process, "Non-linearizable history in $log"); ++$this->iteration; - }); + }) + ; } private function assertExitedCorrectly($process, $errorMessage): void @@ -316,7 +323,7 @@ private function assertExitedCorrectly($process, $errorMessage): void $process->getExitCode(), $errorMessage . PHP_EOL . $process->getErrorOutput() . PHP_EOL . - $process->getOutput() + $process->getOutput(), ); } } diff --git a/tests/Recruiter/Concurrency/PeriodicalCheckTest.php b/tests/Recruiter/Concurrency/PeriodicalCheckTest.php index fbbb1fd..d2b5a1a 100644 --- a/tests/Recruiter/Concurrency/PeriodicalCheckTest.php +++ b/tests/Recruiter/Concurrency/PeriodicalCheckTest.php @@ -1,5 +1,7 @@ hook(Listener\collectFrequencies()) + // ->hook(Listener\collectFrequencies()) ->then(function ($startingDate, $period, $deltas) { $clock = new SettableClock($startingDate); $check = PeriodicalCheck::every($period, $clock); @@ -41,6 +43,7 @@ public function testDoesNotPerformTheCheckTooManyTimes() $maximumNumberOfCalls = ceil($totalInterval / $period); $actualNumberOfCallsExcludingTheFirst = $this->counter - 1; $this->assertLessThanOrEqual($maximumNumberOfCalls, $actualNumberOfCallsExcludingTheFirst); - }); + }) + ; } } diff --git a/tests/Recruiter/Concurrency/mongolock.php b/tests/Recruiter/Concurrency/mongolock.php index ea3a24d..3ba60e0 100644 --- a/tests/Recruiter/Concurrency/mongolock.php +++ b/tests/Recruiter/Concurrency/mongolock.php @@ -1,5 +1,7 @@ (int) (microtime(true) * 1000000), ], - $data + $data, ), ',', '"', From f4a118815112d1f3569f080775e5107a8a793d5c Mon Sep 17 00:00:00 2001 From: Davide Bellettini <325358+dbellettini@users.noreply.github.com> Date: Sat, 2 Aug 2025 20:31:42 +0200 Subject: [PATCH 5/6] Split test suites --- .github/workflows/phpunit.yml | 7 +++++-- Makefile | 12 ++++++------ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/.github/workflows/phpunit.yml b/.github/workflows/phpunit.yml index fe1ae53..9619b69 100644 --- a/.github/workflows/phpunit.yml +++ b/.github/workflows/phpunit.yml @@ -34,5 +34,8 @@ jobs: - name: Install dependencies run: make install - - name: Run test suite - run: make test \ No newline at end of file + - name: Run test suite (except long ones) + run: make test + + - name: Run test suite (only long ones) + run: make test-long diff --git a/Makefile b/Makefile index 81cba80..809ecb0 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: build up down test test-unit fix-cs install shell logs clean +.PHONY: build up down test test-long fix-cs install shell logs clean # Build the Docker image build: @@ -16,13 +16,13 @@ down: install: docker compose run --rm php composer install -# Run all tests +# Run all tests except the long ones test: up - docker compose exec php vendor/bin/phpunit + docker compose exec php vendor/bin/phpunit --exclude-group=long -# Run unit tests specifically -test-unit: up - docker compose exec php vendor/bin/phpunit tests/unit +# Run long tests specifically +test-long: up + docker compose exec php vendor/bin/phpunit --group=long fix-cs: up docker compose exec php vendor/bin/php-cs-fixer fix -v From 60af41ad5ad2829b9bfd18eeac0676c36ddf10c3 Mon Sep 17 00:00:00 2001 From: Davide Bellettini <325358+dbellettini@users.noreply.github.com> Date: Sat, 2 Aug 2025 20:42:21 +0200 Subject: [PATCH 6/6] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/Recruiter/Concurrency/Lock.php | 2 +- src/Recruiter/Concurrency/NullLock.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Recruiter/Concurrency/Lock.php b/src/Recruiter/Concurrency/Lock.php index e6c453b..f8b2115 100644 --- a/src/Recruiter/Concurrency/Lock.php +++ b/src/Recruiter/Concurrency/Lock.php @@ -21,7 +21,7 @@ public function release($force = false): void; /** * @param int $duration (in seconds) * - * @throws LockNotavailableexception + * @throws LockNotAvailableException */ public function refresh($duration = 3600): void; diff --git a/src/Recruiter/Concurrency/NullLock.php b/src/Recruiter/Concurrency/NullLock.php index f7be2bb..cde0920 100644 --- a/src/Recruiter/Concurrency/NullLock.php +++ b/src/Recruiter/Concurrency/NullLock.php @@ -25,7 +25,7 @@ public function release($force = false): void /** * @param int $duration (in seconds) * - * @throws LockNotavailableexception + * @throws LockNotAvailableException */ public function refresh($duration = 3600): void {