Fix #377 - undefined variables

apereo / phpCAS

Apereo PHP CAS Client

https://apereo.github.io/phpCAS/

Apache License 2.0

796 stars 398 forks source link

Fix #377 - undefined variables #417

Closed DeyV closed 2 years ago

jfritschi commented 2 years ago

Thanks @DeyV

Can you please check why some of the integration tests fail?

jfritschi commented 2 years ago

Some of the tests are still failing :-(

DeyV commented 2 years ago

Its hard to fix because this error is not connected with my fix

Can you run retest on previous commit? (It's look like composer or cache problem on php 7.1 version)

jfritschi commented 2 years ago

Looking at the logs:

PHP 7.1 fails due to: phpstan/phpstan 1.5.4 requires php ^7.2|^8.0 -> your PHP version (7.1.33) does not satisfy that requirement
You introduced PHP 8.1 into the composer testing cycle and something "fails".. Not exactly sure what the issue is...

DeyV commented 2 years ago

At this moment this test don't work on my fork So after previous problem it was my try to fix both - 7.1 problem (that looks different - something about broken interface in composer code) And missing 8.1

Other question: is test on php 7.1 still needed?

jfritschi commented 2 years ago

Well there is always the question... The issue is that older php versions are often part of the base Linux operating system (e.g. Enterprise Linux) and therefor remain supported even longer than announced by php.net

We had the discussion already last in #380 year if we should drop 7.1. and 7.2 to allow for newer phpunit...

jfritschi commented 2 years ago

I have no trouble dropping 7.1. and 7.2 from testing ... It just complicates progress. This will probably also make @francoisfreitag happy :-)

jfritschi commented 2 years ago

For php 8.1 i have no idea what is going wrong... Seem to be some compiling issue during php setup. Not really related to phpcas :-(

jfritschi commented 2 years ago

Please just remove php 7.1. from testing and also remove the composer workarounds your added to make 7.1 work.

A wild guess is that this may create the issues on php 8.1. If this does not work we can then also add the flag: "experimental: true" to the test.yml in the 8.1 definition. Then we can figure out over time what the issue is with the actions under 8.1 ... Some strange permission error that i tried tracking down but all my attempts to fix it failed so far.

jfritschi commented 2 years ago

Thanks for your contribution...

The php 8.1 issues went away after merging. Seems to be some weird permission issue when it's a pull request :-(