Skip to content
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

Fix integration tests by fixing a PHP 8.1+ deprecation warnings in ReflectionClass #1351

Merged
merged 4 commits into from
May 21, 2022

Conversation

mfn
Copy link
Collaborator

@mfn mfn commented May 21, 2022

Summary

  • Let's debug why the action fails, e.g. https://github.com/barryvdh/laravel-ide-helper/runs/6533389349?check_suite_focus=true#step:9:1
  • The issue was that on PHP 8.1, new ReflectClass does not silently swallow null values anymore as it did before
  • This already happened but usually wasn't visible unless -v was used, example
    $ ./artisan ide-helper:meta -v
    Cannot make 'Faker\Generator': Class 'Faker\Provider\en_US\Barcode' not found.
    Cannot make 'Illuminate\Contracts\Auth\Authenticatable': Class  does not exist
    Cannot make 'cache.psr6': Class 'Symfony\Component\Cache\Adapter\Psr16Adapter' not found.
    Cannot make 'csp-nonce': Class 'Wza3Mf4CXIvCkcp9K3boMUGJoK6S9maO' not found.
    Cannot make 'env': Class 'local' not found.
    Cannot make 'filesystem.cloud': Disk [s3] does not have a configured driver.
    Cannot make 'redis.connection': Redis connection [default] not configured.
    A new meta file was written to .phpstorm.meta.php
    
  • PHP 8.1+ this triggers deprecation warnings
  • because neither the deprecation logger was set up but also because that deprecation warning was generated, the integration test aborted when something was written to the logs
  • This PR now fixes this by handling null values and throwing a RuntimeException
  • This replaces Fix deprecation warning #1279 which already found out the problem. But because the fix was

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Misc. change (internal, infrastructure, maintenance, etc.)

Checklist

  • Existing tests have been adapted and/or new tests have been added
  • Add a CHANGELOG.md entry
  • Update the README.md
  • Code style has been fixed via composer fix-style

@mfn mfn self-assigned this May 21, 2022
@mfn mfn changed the title gha: get more insights when/if logs exist gha: integration-tests: get more insights when/if logs exist May 21, 2022
@mfn mfn force-pushed the mfn-integration-tests branch 3 times, most recently from 5bc2929 to 90e175c Compare May 21, 2022 19:32
mfn added 2 commits May 21, 2022 21:33
The problem with `null` being passed to `ReflectionClass` was always
there but in PHP 8.1+ it triggers a deprecation warnings.

Since having `null` in `$concrete` doesn't make sense to reflect
anything anyway, we just throw a custom exception (which gets caught a
couple lines below) and just carry on.

When using `-v` this can be seen, example:
```
$ ./artisan ide-helper:meta -v
Cannot make 'Faker\Generator': Class 'Faker\Provider\en_US\Barcode' not found.
Cannot make 'Illuminate\Contracts\Auth\Authenticatable': Class  does not exist
Cannot make 'cache.psr6': Class 'Symfony\Component\Cache\Adapter\Psr16Adapter' not found.
Cannot make 'csp-nonce': Class 'Wza3Mf4CXIvCkcp9K3boMUGJoK6S9maO' not found.
Cannot make 'env': Class 'local' not found.
Cannot make 'filesystem.cloud': Disk [s3] does not have a configured driver.
Cannot make 'redis.connection': Redis connection [default] not configured.
A new meta file was written to .phpstorm.meta.php
```
@mfn mfn force-pushed the mfn-integration-tests branch from 90e175c to 1656c84 Compare May 21, 2022 19:33
@mfn mfn marked this pull request as ready for review May 21, 2022 19:37
@mfn mfn changed the title gha: integration-tests: get more insights when/if logs exist Fix integration tests by fixing a PHP 8.1+ deprecation warnings in ReflectionClass May 21, 2022
@mfn mfn mentioned this pull request May 21, 2022
1 task
@mfn mfn requested a review from barryvdh May 21, 2022 19:39
@barryvdh barryvdh merged commit 6a461a7 into barryvdh:master May 21, 2022
@mfn mfn deleted the mfn-integration-tests branch May 22, 2022 17:55
d3v2a pushed a commit to d3v2a/laravel-ide-helper that referenced this pull request Feb 16, 2024
…flectionClass (barryvdh#1351)

* gha: get more insights when/if logs exist

* Better handle cases when we can receive a concrete instance

The problem with `null` being passed to `ReflectionClass` was always
there but in PHP 8.1+ it triggers a deprecation warnings.

Since having `null` in `$concrete` doesn't make sense to reflect
anything anyway, we just throw a custom exception (which gets caught a
couple lines below) and just carry on.

When using `-v` this can be seen, example:
```
$ ./artisan ide-helper:meta -v
Cannot make 'Faker\Generator': Class 'Faker\Provider\en_US\Barcode' not found.
Cannot make 'Illuminate\Contracts\Auth\Authenticatable': Class  does not exist
Cannot make 'cache.psr6': Class 'Symfony\Component\Cache\Adapter\Psr16Adapter' not found.
Cannot make 'csp-nonce': Class 'Wza3Mf4CXIvCkcp9K3boMUGJoK6S9maO' not found.
Cannot make 'env': Class 'local' not found.
Cannot make 'filesystem.cloud': Disk [s3] does not have a configured driver.
Cannot make 'redis.connection': Redis connection [default] not configured.
A new meta file was written to .phpstorm.meta.php
```

* gha: make sure to run meta with -v to see all output

Helps when debugging things

* Add CHANGELOG.md entry
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants