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

tests: Added tests for auth handler plugin #32

Merged
merged 3 commits into from
Oct 4, 2019
Merged

Conversation

abhinavraj23
Copy link

What does this do / why do we need it?

Added tests for handlers of auth and completed the leftover methods in middleware tests

What should your reviewer look out for in this PR?

Added tests for methods in handlers.go of auth folder in handler_test.go

Do you need help or clarification on anything?

Yes, want to know if I have used the right methodology for testing the publicKey

Which issue(s) does this PR fix?

#16 #17

@abhinavraj23 abhinavraj23 self-assigned this Sep 18, 2019
@abhinavraj23 abhinavraj23 changed the title tests: Added tests for auth handler plugin and completed methods for middleware_test tests: Added tests for auth handler plugin Sep 18, 2019
Copy link

@jeet-parekh jeet-parekh left a comment

Choose a reason for hiding this comment

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

I don't know what the stub client does exactly. But when saving the public keys, you're using test index. And when fetching the public keys you're using .publickey index. Is that mismatch intentional?

@abhinavraj23
Copy link
Author

@jeet-parekh Actually I was doing it, it should be saved in the .publickey index, I have made the change.

Copy link

@jeet-parekh jeet-parekh left a comment

Choose a reason for hiding this comment

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

LGTM

@jeet-parekh jeet-parekh merged commit 8e55c07 into feat/es7 Oct 4, 2019
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.

2 participants