Skip to content

Modernize codebase with PHP 8.1+ features and expand test coverage #9

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aarondfrancis merged 6 commits into from
Nov 27, 2025
Merged

Modernize codebase with PHP 8.1+ features and expand test coverage #9

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7e92a37
Modernize codebase with PHP 8.1+ features and expand test coverage
aarondfrancis Nov 27, 2025
e174b26
Ignore
aarondfrancis Nov 27, 2025
211ea57
Fix testbench minimum version for prefer-lowest compatibility
aarondfrancis Nov 27, 2025
a4aaf55
Fix testbench minimum version for prefer-lowest compatibility
aarondfrancis Nov 27, 2025
96288b4
Merge and fix testbench version to ^9.5
aarondfrancis Nov 27, 2025
46f1463
Remove nullable type from exceptionIsFlaky method
aarondfrancis Nov 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/style.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:
- name: Setup PHP
uses: shivammathur/setup-php@v2
with:
php-version: "8.3"
php-version: "8.4"
extensions: json, dom, curl, libxml, mbstring
coverage: none

Expand Down
15 changes: 1 addition & 14 deletions .github/workflows/tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
strategy:
fail-fast: false
matrix:
php: [ 8.1, 8.2, 8.3 ]
php: [ 8.1, 8.2, 8.3, 8.4 ]
laravel: [ '10.*', '11.*', '12.*' ]
dependency-version: [ prefer-lowest, prefer-stable ]

Expand All @@ -29,19 +29,6 @@ jobs:

name: P${{ matrix.php }} / L${{ matrix.laravel }} / ${{ matrix.dependency-version }}

services:
mysql:
image: mysql:8.0
env:
MYSQL_DATABASE: fast_paginate
MYSQL_HOST: 127.0.0.1
MYSQL_USER: test
MYSQL_PASSWORD: root
MYSQL_ROOT_PASSWORD: root
ports:
- 3306:3306
options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=3

steps:
- name: Checkout code
uses: actions/checkout@v4
Expand Down
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,4 @@ node_modules
docs/*.blade.php
docs/**/*.blade.php
.phpunit.cache/test-results
.claude/settings.local.json
12 changes: 6 additions & 6 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@
}
],
"require": {
"php": "^8.0",
"illuminate/support": "^10|^11.0|^12.0",
"illuminate/cache": "^10|^11.0|^12.0",
"illuminate/console": "^10|^11.|^12.00"
"php": "^8.1",
"illuminate/support": "^10.0|^11.0|^12.0",
"illuminate/cache": "^10.0|^11.0|^12.0",
"illuminate/console": "^10.0|^11.0|^12.0"
},
"require-dev": {
"mockery/mockery": "^1.3.3",
"phpunit/phpunit": ">=8.5.23|^9|^10",
"mockery/mockery": "^1.6",
"phpunit/phpunit": "^10.5|^11.0",
"orchestra/testbench": "^8.0|^9.0|^10.0"
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Laravel 10 PHP minimum version requirement

<web_search_result>
Laravel 10 requires PHP 8.1 or greater. (laravel.com)

Citations:

Laravel 11 PHP minimum version requirement

<web_search_result>
Laravel 11 requires PHP 8.2 or greater. [1][2]

Sources:
[1] Laravel 11 release notes (official docs).
[2] Laravel 11 upgrade guide (official docs).
</web_search_result>

Laravel 12 PHP minimum version requirement

<web_search_result>
Laravel 12 requires PHP 8.2 or higher (supports PHP 8.2–8.4). [1][2]

Sources:
[1] Laravel 12 release notes (laravel.com).
[2] Laravel News — "Laravel 12 is Now Released".
</web_search_result>

Orchestra Testbench version compatibility Laravel

<web_search_result>
Orchestra Testbench version mapping to Laravel (short):

  • Laravel 5.x → Testbench 3.x. [1]
  • Laravel 6.x → Testbench 4.x. [1]
  • Laravel 7.x → Testbench 5.x. [1]
  • Laravel 8.x → Testbench 6.x. [1]
  • Laravel 9.x → Testbench 7.x. [1]
  • Laravel 10.x → Testbench 8.x. [1]
  • Laravel 11.x → Testbench 9.x. [1]
  • Laravel 12.x → Testbench 10.x. [1][2]

Install the testbench major that matches your Laravel major (use the ^ version constraint for the recommended minor). [1][2]

Sources: [1] orchestral/testbench-core README (GitHub). [2] Testbench compatibility table (packages/tools / Testbench docs).
</web_search_result>

PHPUnit 10.5 minimum PHP version requirement

<web_search_result>
PHPUnit 10.5 requires PHP >= 8.1. [1] [2]

Sources:
[1] PHPUnit 10.5 Installation manual.
[2] PHPUnit Supported Versions (PHP compatibility table).
</web_search_result>

PHPUnit 11.0 minimum PHP version requirement

<web_search_result>
PHPUnit 11.0 requires PHP 8.2 or later. [1][2]

Sources:
[1] PHPUnit 11 announcement — phpunit.de.
[2] Supported Versions (PHP compatibility) — phpunit.de.
</web_search_result>

Mockery 1.6 minimum PHP version requirement

<web_search_result>
Mockery 1.6 requires PHP >= 7.3. [1]

Sources:
[1] Packagist — mockery/mockery (requires php: >=7.3).
</web_search_result>

The PHP version constraint must be bumped to ^8.2 to support Laravel 11 and 12.

The original review's compatibility analysis contains a critical oversight:

  • PHP ^8.1 works for Laravel 10 (correct), but Laravel 11 and 12 both require PHP ^8.2 minimum
  • PHPUnit 11.0 also requires PHP ^8.2, while PHPUnit 10.5 supports PHP ^8.1
  • With "php": "^8.1" and "illuminate/support": "^10.0|^11.0|^12.0", users on PHP 8.1 attempting to install Laravel 11 or 12 will encounter dependency conflicts

Recommended fix: Change "php": "^8.1" to "php": "^8.2" to ensure proper alignment with Laravel 11/12 requirements and PHPUnit 11.0 compatibility.

🤖 Prompt for AI Agents 
In composer.json around lines 12 to 21, the PHP platform requirement is set to
"^8.1" which is incompatible with Laravel 11/12 and PHPUnit 11; update the "php"
constraint to "^8.2" so the package can be installed with illuminate/* versions
^11.0|^12.0 and phpunit 11.0, then run composer validate and composer update to
refresh the lockfile and confirm there are no dependency conflicts.

},
"autoload": {
Expand Down
48 changes: 26 additions & 22 deletions src/Arbiter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,102 +6,106 @@

namespace AaronFrancis\Flaky;

use Illuminate\Contracts\Cache\Repository;
use Illuminate\Support\Arr;
use Illuminate\Support\Facades\Cache;
use Illuminate\Support\Traits\Macroable;
use Throwable;

class Arbiter
{
use Macroable;

public $failuresAllowedForSeconds = 60 * 60 * 24 * 365 * 10;
public int $failuresAllowedForSeconds = 60 * 60 * 24 * 365 * 10;

public $consecutiveFailuresAllowed = INF;
public int|float $consecutiveFailuresAllowed = INF;

public $totalFailuresAllowed = INF;
public int|float $totalFailuresAllowed = INF;

/** @var callable(Throwable): void */
public $handleFailuresWith;

protected $key;
protected string $key;

protected $totalFailures;
protected int $totalFailures;

protected $consecutiveFailures;
protected int $consecutiveFailures;

protected $deadline;
protected ?int $deadline;

protected $cache;
protected Repository $cache;

public function __construct($id)
public function __construct(string $id)
{
$this->key = "flaky::$id";
$this->cache = Cache::store();

/** @var array{total?: int, consecutive?: int, deadline?: int|null} $stats */
$stats = $this->cache->get($this->key, []);

$this->totalFailures = Arr::get($stats, 'total', 0);
$this->consecutiveFailures = Arr::get($stats, 'consecutive', 0);
$this->deadline = Arr::get($stats, 'deadline');

$this->handleFailuresWith = function ($e) {
$this->handleFailuresWith = function (Throwable $e): never {
throw $e;
};
}

public function handle($exception)
public function handle(?Throwable $exception): void
{
$this->deadline = $this->deadline ?? $this->freshDeadline();

if ($exception) {
if ($exception !== null) {
$this->totalFailures++;
$this->consecutiveFailures++;
}

$this->updateCachedStats($exception);

if (!is_null($exception) && $this->outOfBounds()) {
if ($exception !== null && $this->outOfBounds()) {
$this->callHandler($exception);
}
}

public function handleFailures($callback)
public function handleFailures(callable $callback): void
{
$this->handleFailuresWith = $callback;
}

public function outOfBounds()
public function outOfBounds(): bool
{
return $this->tooManyConsecutiveFailures() || $this->tooManyTotalFailures() || $this->beyondDeadline();
}

public function tooManyConsecutiveFailures()
public function tooManyConsecutiveFailures(): bool
{
return $this->consecutiveFailures > $this->consecutiveFailuresAllowed;
}

public function tooManyTotalFailures()
public function tooManyTotalFailures(): bool
{
return $this->totalFailures > $this->totalFailuresAllowed;
}

public function beyondDeadline()
public function beyondDeadline(): bool
{
return now()->timestamp > $this->deadline;
}

protected function callHandler($exception)
protected function callHandler(Throwable $exception): void
{
call_user_func($this->handleFailuresWith, $exception);
}

protected function freshDeadline()
protected function freshDeadline(): int
{
return now()->addSeconds($this->failuresAllowedForSeconds)->timestamp;
}

protected function updateCachedStats($exception)
protected function updateCachedStats(?Throwable $exception): void
{
$failed = !is_null($exception);
$failed = $exception !== null;

$stats = $failed ? [
// Reset if we passed, otherwise just store the incremented value.
Expand Down
Loading
Loading