search

subfission / cas

Simple CAS Authentication for Laravel 5 - 10.
MIT License
151 stars 70 forks source link

Enable tests #117

Closed dstepe closed 11 months ago

dstepe commented 1 year ago

@subfission In a recent comment, you stated "If you have a testing resource for CAS auth that doesn't require certificates, that will help a ton with automating CI and eventual CD." I don't have any suggestions for enabling an actual CAS service for automated testing, but I do think you can achieve your testing goals without it. The apereo/phpcas package is responsible for the interaction with CAS and therefore is also responsible for testing that interaction. This package is responsible for enabling the use of apereo/phpcas package in Laravel, so tests in this package should be focused on that.

The need to call static methods on the phpCAS class, and the use of built in PHP functions, makes testing a challenge. My approach in those cases is to isolate the function/method calls in their own minimal class which can then be mocked in classes that do the actual work of the package.

This PR is a demonstration of this approach. I created two "proxy" classes and update CasManager to use them. There are enough tests written to show that this approach works and, in my opinion, has value. I tried to minimize the changes to CasManager, but some updates were unavoidable (env() is not available without fully bootstrapping Laravel, the SAML_VERSION_1_1 constant is not defined if the phpCAS class is not loaded), but the overall functionality is unchanged.

I took the liberty of also adding PHP CS Fixer and GitHub workflows for linting and running the tests (tests are run under PHP 7.3 through 8.1).

I would also encourage the use of orchestral/testbench to test more of the Laravel specific features, though the most important subject for testing is the CasManager class.

I've done all of this with other packages and am happy to keep working on this if it looks like a direction you are interested in going.

dstepe commented 1 year ago

@subfission I lost track of this for a while, but would like to get back to it. Are you interested in moving in the direction for testing I've proposed in this PR? It basically moves the phpCAS method calls into an isolated class so tests in this package only assert the correct use of phpCAS, not that phpCAS itself functions correctly. I've also added GitHub actions to automate the tests. There is still work to be done, but I'm willing to contribute.

dstepe commented 1 year ago

@subfission pinging on this again. Any interest in pursuing it?

subfission commented 11 months ago

@dstepe Would you have interest in sharing ownership of this project? My time is too limited to continue to manage this codebase.

dstepe commented 11 months ago

@subfission my time is also limited, but I can help with basic maintenance of the package. This has been very helpful to my organization. My ability to contribute may decrease over time, so it would be good to continue looking for other interested folks to help as well.

I'll work on fixing up this PR and getting it merged.