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

[8.x] Add tests for Passport::actingAs and Passport::actingAsClient #1155

Merged
merged 6 commits into from
Jan 6, 2020

Conversation

X-Coder264
Copy link
Contributor

Currently Passport is being only unit tested which is great except that each method where Laravel global helpers are being used cannot be tested at all.

This is the case with the Passport::actingAsClient method which uses the app helper to resolve and bind stuff with the container. Because of that the method wasn't tested which resulted in it being broken from July (#1040) till November (#1119). The fix was made in November but as there are still no tests for it we won't know if it breaks again in the future.

This PR adds Orchestra Testbench so that we can write full functional tests with the whole framework and I've added tests for the Passport::actingAs and Passport::actingAsClient methods.

In doing so I had to fix the storage_path function conflict because now the foundation storage_path function exists.

I've also created an abstract PassportTestCase class, it does a few things so that we don't need to do them in almost every test (as they are gonna be needed):

  1. It calls the passport:keys command which generates the oauth keys (if they don't exist already) - without the keys the ResourceServer class throws an exception on instantiation and the ResourceServer is probably gonna be instantiated at some point in 95% of the functional tests.
  2. It sets the passport driver for the auth API guard.
  3. It registers the PassportServiceProvider without which a bunch of bindings would be missing from the container.

This paves the way for further covering Passport functionality with feature/functional tests.

@X-Coder264 X-Coder264 changed the title Add tests for Passport::actingAs and Passport::actingAsClient [8.x] Add tests for Passport::actingAs and Passport::actingAsClient Jan 5, 2020
@taylorotwell
Copy link
Member

If you're requiring testbench why do you need to require laravel/framework?

@X-Coder264
Copy link
Contributor Author

@taylorotwell I required orchestra/testbench-core instead of orchestra/testbench the first time so I had to require laravel/framework manually. This is now fixed.

@taylorotwell taylorotwell merged commit 3cbf798 into laravel:8.x Jan 6, 2020
@X-Coder264 X-Coder264 deleted the feature-tests branch January 6, 2020 14:37
@driesvints
Copy link
Member

@X-Coder264 thanks!

@X-Coder264
Copy link
Contributor Author

@driesvints No problem :)

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.

3 participants