-
-
Notifications
You must be signed in to change notification settings - Fork 224
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.
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
Add php 8.3 matrix #460
Add php 8.3 matrix #460
Conversation
The tests seem to fail, could you take a look at that? |
@@ -36,4 +36,7 @@ | |||
<logging> | |||
<junit outputFile="build/report.junit.xml"/> | |||
</logging> | |||
<php> | |||
<env name="APP_DEBUG" value="true"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was an unintended issue with the responsecache.add_cache_time_header
test code because APP_DEBUG
was not enabled in all the test code.
However, I'm not sure what differences from before caused this.
@@ -211,7 +211,7 @@ | |||
|
|||
$this->assertFalse($firstResponse->headers->has('X-Cached-At')); | |||
assertTrue($secondResponse->headers->has('X-Cached-At')); | |||
$this->assertInstanceOf(DateTime::class, $secondResponse->headers->getDate('X-Cached-At')); | |||
$this->assertInstanceOf(DateTimeInterface::class, $secondResponse->headers->getDate('X-Cached-At')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depending on the version, it may create a DateTime::class
or a DateTimeImmutable::class
. I'm not sure why this happens either.
First, please understand that I am new to open source involvement. By modifying that code, your test code runs successfully, but you realize that the modified code is not the correct answer. For more information, see: https://github.com/binbyz/laravel-responsecache/actions/runs/7141203514 Just for the record, I hope you find this helpful. You can probably just close my PR. |
This is perfect, thanks! |
Add php 8.3 to run-tests.yml