Skip to content

Add (initial) support for PHP 8.4 #1631

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
merged 2 commits into from
Oct 30, 2024
Merged

Add (initial) support for PHP 8.4 #1631

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b14fce8
Run tests on PHP 8.4
jnoordsij Jul 9, 2024
e0e449a
Update integration test to actually test clear command functionality
jnoordsij Sep 26, 2024
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
10 changes: 6 additions & 4 deletions .github/workflows/run-integration-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:
COMPOSER_NO_INTERACTION: 1
strategy:
matrix:
php: [8.3, 8.2, 8.1, 8.0]
php: [8.4, 8.3, 8.2, 8.1, 8.0]
lumen: [10.*, 9.*]
exclude:
- lumen: 10.*
Expand Down Expand Up @@ -54,10 +54,11 @@ jobs:
- name: Execute clear run
run: |
cd sample
mkdir -p "storage/debugbar/" && touch "storage/debugbar/foo.json"
php artisan debugbar:clear
- name: Check file count in logs
run: |
if [ `ls -1q "sample/storage/logs/" | wc -l` -gt 0 ];then exit 1;fi
Copy link
Owner

Choose a reason for hiding this comment

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

What is this change exactly?

Copy link
Contributor Author

@jnoordsij jnoordsij Oct 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not exactly sure on what the intended check was doing. This check was introduced with #1118 and seems to aim to check for 'succesful completion' of the php artisan debugbar:clear command on a fresh Laravel installation. Here 'succesful' seems to be measured by "not logging any messages" (and not exiting with non-zero code).

However, this metric seems slightly off to me. In this case it was erring, because (due to php-debugbar/php-debugbar#687) deprecations are being logged while running the command. There are ways to work around this, e.g. suppressing deprecations with some Laravel flag.

I've here opted to rather than trying to patch the old definition of 'succes', come up with a new one that actually feature-tests the actual command. Given that the purpose of the command is cleaning json files from the storage/debugbar directory, I've now added a check to ensure the command does just this. Thus the new definition of 'succesful' is now the combination of php artisan debugbar:clear exits with code 0 and does feature-wise what it's expected to do.

if [ `ls -1q "sample/storage/debugbar/" | wc -l` -gt 0 ];then exit 1;fi
php-laravel-integration-tests:
runs-on: ubuntu-latest
timeout-minutes: 15
Expand All @@ -66,7 +67,7 @@ jobs:
strategy:
fail-fast: false
matrix:
php: [8.3, 8.2, 8.1, 8.0]
php: [8.4, 8.3, 8.2, 8.1, 8.0]
laravel: [11.*, 10.*, 9.*]
exclude:
- laravel: 10.*
Expand Down Expand Up @@ -103,7 +104,8 @@ jobs:
- name: Execute generate run
run: |
cd sample
mkdir -p "storage/debugbar/" && touch "storage/debugbar/foo.json"
php artisan debugbar:clear
- name: Check file count in logs
run: |
if [ `ls -1q "sample/storage/logs/" | wc -l` -gt 0 ];then exit 1;fi
if [ `ls -1q "sample/storage/debugbar/" | wc -l` -gt 0 ];then exit 1;fi
2 changes: 1 addition & 1 deletion .github/workflows/run-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
strategy:
fail-fast: false
matrix:
php: [8.3, 8.2, 8.1, 8.0]
php: [8.4, 8.3, 8.2, 8.1, 8.0]
laravel: [^11, ^10, ^9]
dependency-version: [prefer-stable]
exclude:
Expand Down
Loading