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.

Sign up for GitHub

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

Jump to bottom

Enable auth test in test_base_scraper.py #597

Merged
merged 4 commits into from
Mar 31, 2020
Merged

Enable auth test in test_base_scraper.py #597

merged 4 commits into from
Mar 31, 2020

Conversation

kaushiksk
Copy link
Contributor

@kaushiksk kaushiksk commented Mar 7, 2020 •
edited
Loading

Fixes #492

  • Adds a custom auth handler and register a route
  • Overrides the decode_basic method of request.Authentication
    • uses the right base64 decode function and fixes the bytes/str error
  • enables the auth tests

The decode_basic call in request.Authentication would fail in py3 as it uses base64.decodestring which expects a byte-like array. Hence overrode this to use base64.b64decode

NOTE:

wptserve==2.0 from PyPI has this issue. wptserve has since been moved to https://github.com/web-platform-tests/wpt where this has been fixed.

@kaushiksk
Fix issue-492
234d580 
- Adds a custom auth handler and register a route
- Overrides the `decode_basic` method of request.Authentication
  -  uses the right base64 decode function and fixes the bytes/str error
- enables the auth tests
@coveralls
Copy link

coveralls commented Mar 7, 2020 •
edited
Loading

Coverage Status

Coverage increased (+0.6%) to 92.867% when pulling 8566414 on kaushiksk:issue-492 into fe75cb6 on mozilla:master.

@kaushiksk kaushiksk changed the title Fix issue-492 Enable auth test in test_base_scraper.py Mar 7, 2020
@kaushiksk kaushiksk marked this pull request as ready for review March 7, 2020 10:07
@kaushiksk kaushiksk mentioned this pull request Mar 7, 2020
Merged
@whimboo
Copy link
Contributor

whimboo commented Mar 20, 2020

@kaushiksk thank you for the fix! There were test failures on MacOS for the Python3 job, but I just fixed those. I will have a look at the PR in a moment.

whimboo
whimboo requested changes Mar 20, 2020
View reviewed changes
tests/base_scraper/test_base_scraper.py Outdated Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
@whimboo
Copy link
Contributor

whimboo commented Mar 20, 2020

wptserve==2.0 from PyPI has this issue. wptserve has since been moved to https://github.com/web-platform-tests/wpt where this has been fixed.

@jgraham, could you please release a new version of wptserve on PyPI, so that we could depend on the fixed code? Thanks.

@whimboo
Copy link
Contributor

whimboo commented Mar 24, 2020

The new version of wptserve will be released via web-platform-tests/wpt#22412.

@whimboo
Copy link
Contributor

whimboo commented Mar 25, 2020

@kaushiksk the wptserve 3.0 package has been released to PyPI:
https://pypi.org/project/wptserve/3.0/

Would you mind updating your PR? Thanks!

@kaushiksk
Copy link
Contributor Author

@whimboo I'll update my PR with the changes!

@kaushiksk
Merge branch 'master' of github.com:kaushiksk/mozdownload into issue-492
c0807d2
@kaushiksk
Bump up wptserve to version 3.0
b94d5af
@kaushiksk
Remove custom Auth handler
8566414 
- Use request.Authentication since now wptserve==3.0
- Hardcode username and password
- Rename `http_auth_handler` to `basic_auth_handler`
- Rename `test_url` to `basic_auth_url`
@kaushiksk kaushiksk requested a review from whimboo March 25, 2020 18:46
@whimboo
Copy link
Contributor

whimboo commented Mar 31, 2020

Thanks a lot for this addition. It's great to finally have authentication tests working again! If you are eager to pick-up some more issues I'm happy to assist in whatever you need.

@whimboo whimboo merged commit 66d439b into mozilla:master Mar 31, 2020
@kaushiksk kaushiksk deleted the issue-492 branch April 1, 2020 05:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@whimboo whimboo whimboo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

tests/base_scraper/test_base_scraper.py::test_authentication failed
3 participants
@kaushiksk @coveralls @whimboo