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 for warning behavior on oauth scope change #810

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jdevries3133
Copy link

When Oauth scope changes, oauthlib raises a Warning with special attributes (
token, old_scope, and new_scope). This behavior was documented in the
original commit message from ca4811b, but
there are no tests to check for this behavior. I started working on #809 and
while reading the code, I thought the extra attributes tacked onto the warning
were strange. I thought it was even more strange that this behavior wasn't
documented anywhere or tested for.

I decided not to add any documentation because there is no existing
documentation about scope changes already, and this is a very specific
situation, however I think that having the test case, and the
TokenScopeChangedWarning class makes it much clearer to anyone taking a
glance at the source code what the behavior is.

Plus, I didn't want to break anything while working on #809, and I became a
little suspicious when I deleted those lines (w.token = params; w.old_scope = ...),
and all the unit tests still passed!

* create ``TokenScopeChangedWarning``, a sublcass of ``Warning`` to make
  the special properties more explicit.
* add test for this special behavior
Copy link
Contributor

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

please check the CI failures

@jdevries3133
Copy link
Author

@auvipy oops; I think it's fixed

@auvipy
Copy link
Contributor

auvipy commented Apr 5, 2022

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

2 participants