Problem/Motivation
The drupal/cas_mock_server:dev-0.x branch has been created only because the main CAS module has supported, at that time, PHP 5.6. But, as now, the the lowest supported Drupal version in 8.9.x and a Drupal 8.9.x requires PHP ^7.0.8, there's no reason to keep the dependency on drupal/cas_mock_server:dev-0.x. It seems that this dependency also prevents this module to be tested on Drupal 8.9, according to #3083230: Branch 8.x-0.x should support also Drupal 8.8.x
Proposed resolution
Remove the dependency.
Remaining tasks
None.
User interface changes
None.
API changes
None.
Data model changes
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | 3207386-15.patch | 11.18 KB | claudiu.cristea |
Issue fork cas-3207386
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
claudiu.cristeaHm, it seems that the tests are not enabled for 2.x branch (?)
Comment #4
claudiu.cristeaChange to branch to try to trigger the tests
Comment #5
claudiu.cristeaLet's try as a patch.
Comment #6
claudiu.cristeaFix IS
Comment #7
claudiu.cristeaThe failure withs PHP 7.4 & 8, from #5 shows that the module is not yet prepared for PHPUnit 8 & 9
Comment #8
claudiu.cristeaI can't select 2.x-dev to test. So, maybe I was wrong in #7
Comment #9
bkosborneThought I fixed the deprecations for phpunit in the 2.x branch. You were right though about tests not enabled for 2.x branch. I just enabled them. I'll take a look at this. Thank you
Comment #10
claudiu.cristea@bkosborne,
The problem is that, when the deprecation has been dropped, in https://git.drupalcode.org/project/cas/commit/f75e1f7, also the method parameters should have been swapped, to follow the the externalauth method signature and also the need to avoid an optional param followed by a non optional. Now, I see that 2.0.0 is in beta. According to Drupal core standards, API changes are only allowed in ALPHA but it's up to you if you accept this change now, given that BETA is fresh and most of the sites are still on ^1.0. If this change is not acceptable, we have to deprecate the order of parameters and add message telling the they will be swapped in 3.0.0.
IMO, event is BETA, is not yet a stable release, so I would make the changes now.
Comment #11
bkosborneGood catch. Yes I will make the changes in 2.x. I'll review this tonight.
Comment #13
claudiu.cristeaI left outside one occurrence. Fixed also coding standards.
Comment #14
claudiu.cristeaTests from #13 shows that everything is OK, except with Drupal 9.2.x but that is only a development branch now, not the official stable. I'll try to look into that failure.
Comment #15
claudiu.cristeaThey've changed the password reset message in 9.2
Fixed also some coding standards. BTW, you may want to enforce coding standards by adding a drupalci.yml file. It would keep the codebase sane.
EDIT: Also, to ensure against regressions, it would be good to have weekly tests for the lowest setup (e.g. PHP7.0+SQLite+D8.9) and highest (e.g. PHP8+MySQL8+D9.2)
Comment #17
bkosborneCommitted. Thank you again. I'll update the testing schedule as you suggest. That makes sense.