Refactor redirect tests #5841
Merged
Refactor redirect tests #5841
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is most like a little hack to make the tests of the new feature in .com pass. Why tests are passing after this changes? First, we don't need to patch anything, by default those files don't exist. Now, here is the interesting part. When a version exists (in this case `0.8.1`) the serve_docs function (from .com) tries to verify that the current user has permissions over that version first, this gives a 401 response. Accessing to a unexisting version, it gives 404, which is needed to trigger the redirect
|
oh, and the user really has permissions over that version, but it was logged in outside the new backend |
agjohnson
reviewed
Jun 24, 2019
There was a problem hiding this comment.
Going through history, I don't see a good reason why these testss were patched to begin with. Though patching the method is much more explicit for unit tests, I think the implementation here is probably enough, it should test the same paths in theory.
It would be good to get another opinion on this though.
ericholscher
approved these changes
Jun 25, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is most like a little hack to make
the tests of the new feature in .com pass.
Why tests are passing after this changes?
First, we don't need to patch anything, by default
those files don't exist.
Now, here is the interesting part.
When a version exists (in this case
0.8.1)the serve_docs function (from .com)
tries to verify that the current user has permissions over that
version first, this gives a 401 response.
Accessing to a unexisting version, it gives 404,
which is needed to trigger the redirect