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

Add testActionPassAuth to facilitate testing with trivial auth mocks #251

Merged
merged 1 commit into from
May 19, 2018

Conversation

amory-coursera
Copy link
Contributor

Years ago, it was common practice to write resource-level unit tests for Coursera APIs - there are 555 usages of the existing testAction method in our codebase. As authorization logic became more complicated, teams increasingly moved away from resource testing because it was cumbersome to mock or fake auths.

The irony here is that testAction itself requires the caller define a RestContext with authorization data, and tests only execute the authorizer's test-only check method. Needing to define a fully functional authorizer for testing is silly, because the bulk of its machinery won't even be exercised within a testAction execution.

This revision adds a new test method that skips the check step of testAction. This makes it possible to construct test resources with trivial authorizers - e.g. mock[<AuthorizerType]. While we don't add very many new resource-level tests, the option to switch to testActionPassAuth when refactoring tests for legacy code will make it much easier to migrate authorizer definitions for legacy code.

@@ -88,7 +88,8 @@ class MacroImpls(val c: blackbox.Context) {
* @tparam Resource The resource type that we are specializing.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes to this file from pre-compile scalafmt application. Please ignore.

@amory-coursera amory-coursera merged commit ff9ff4f into master May 19, 2018
@amory-coursera amory-coursera deleted the as-auth-check-test-bypass branch May 22, 2018 01:41
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.

None yet

1 participant