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 xfailing integration tests against proxy.py #6002

Merged
merged 2 commits into from
Oct 3, 2021

Conversation

bmbouter
Copy link
Contributor

@bmbouter bmbouter commented Sep 14, 2021

I was running this with pytest 'tests/test_proxy_functional.py::test_secure_proxy_http_absolute_path'

I tried using the TestCase from proxy.py but I want these as pytest fixtures and I couldn't find a way to call the trustme fixture and set the class attributes I need to here. So instead I'm "doing it like their TestCase" only as a fixture. This probably should go into proxy.py eventually, but here it is for now.

setup.cfg Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
aiohttp/client_reqrep.py Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
@bmbouter bmbouter force-pushed the add-xfail-https-proxy-tests branch 3 times, most recently from e631f76 to a59d42e Compare September 21, 2021 18:47
@bmbouter
Copy link
Contributor Author

I was running these with:

pytest --log-level=INFO 'tests/test_proxy_functional.py::test_secure_proxy_http_absolute_path'
pytest --log-level=INFO 'tests/test_proxy_functional.py::test_secure_proxy_https_absolute_path'

@bmbouter
Copy link
Contributor Author

@webknjaz this is as far as I could take these tests. I spent a long time messing with it. I'm hoping you can take it from here. Also FYI @dkliban.

@asvetlov asvetlov added this to the 3.8 milestone Oct 1, 2021
@webknjaz webknjaz changed the base branch from 3.8 to master October 3, 2021 00:19
@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Oct 3, 2021
@webknjaz webknjaz marked this pull request as ready for review October 3, 2021 00:21
@webknjaz webknjaz closed this Oct 3, 2021
@webknjaz webknjaz reopened this Oct 3, 2021
@webknjaz webknjaz changed the title Adds basic xfail tests for secure proxy support Add xfailing integration tests against proxy.py Oct 3, 2021
@webknjaz webknjaz enabled auto-merge (squash) October 3, 2021 00:39
@webknjaz webknjaz added client infra python Pull requests that update Python code labels Oct 3, 2021
@webknjaz webknjaz force-pushed the add-xfail-https-proxy-tests branch 2 times, most recently from aa3a120 to d405eee Compare October 3, 2021 01:09
webknjaz and others added 2 commits October 3, 2021 17:21
This patch adds full end-to-end tests for sending requests to HTTP and
HTTPS endpoints through an HTTPS proxy. The first case is currently
supported and the second one is not. This is why the latter test is
marked as expected to fail. The support for TLS-in-TLS in the upstream
stdlib asyncio is currently disabled but is available in Python 3.9
via monkey-patching which is demonstrated in the added tests.

Refs:
* https://bugs.python.org/issue37179
* python/cpython#28073
* aio-libs#5992

Co-Authored-By: Sviatoslav Sydorenko <webknjaz@redhat.com>
@codecov
Copy link

codecov bot commented Oct 3, 2021

Codecov Report

Merging #6002 (1edb61a) into master (e445011) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #6002   +/-   ##
=======================================
  Coverage   96.75%   96.75%           
=======================================
  Files          44       44           
  Lines        9857     9857           
  Branches     1592     1592           
=======================================
  Hits         9537     9537           
  Misses        182      182           
  Partials      138      138           
Flag Coverage Δ
unit 96.65% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e445011...1edb61a. Read the comment docs.

@webknjaz webknjaz merged commit d66e07c into aio-libs:master Oct 3, 2021
@patchback
Copy link
Contributor

patchback bot commented Oct 3, 2021

Backport to 3.8: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply d66e07c on top of patchback/backports/3.8/d66e07c652322d280740106ebb9946a3dd7daf5b/pr-6002

Backporting merged PR #6002 into master

  1. Ensure you have a local repo clone of your fork. Unless you cloned it
    from the upstream, this would be your origin remote.
  2. Make sure you have an upstream repo added as a remote too. In these
    instructions you'll refer to it by the name upstream. If you don't
    have it, here's how you can add it:
    $ git remote add upstream https://github.com/aio-libs/aiohttp.git
  3. Ensure you have the latest copy of upstream and prepare a branch
    that will hold the backported code:
    $ git fetch upstream
    $ git checkout -b patchback/backports/3.8/d66e07c652322d280740106ebb9946a3dd7daf5b/pr-6002 upstream/3.8
  4. Now, cherry-pick PR Add xfailing integration tests against proxy.py #6002 contents into that branch:
    $ git cherry-pick -x d66e07c652322d280740106ebb9946a3dd7daf5b
    If it'll yell at you with something like fatal: Commit d66e07c652322d280740106ebb9946a3dd7daf5b is a merge but no -m option was given., add -m 1 as follows intead:
    $ git cherry-pick -m1 -x d66e07c652322d280740106ebb9946a3dd7daf5b
  5. At this point, you'll probably encounter some merge conflicts. You must
    resolve them in to preserve the patch from PR Add xfailing integration tests against proxy.py #6002 as close to the
    original as possible.
  6. Push this branch to your fork on GitHub:
    $ git push origin patchback/backports/3.8/d66e07c652322d280740106ebb9946a3dd7daf5b/pr-6002
  7. Create a PR, ensure that the CI is green. If it's not — update it so that
    the tests and any other checks pass. This is it!
    Now relax and wait for the maintainers to process your pull request
    when they have some cycles to do reviews. Don't worry — they'll tell you if
    any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

webknjaz pushed a commit to webknjaz/aiohttp that referenced this pull request Oct 3, 2021
This patch adds full end-to-end tests for sending requests to HTTP and
HTTPS endpoints through an HTTPS proxy. The first case is currently
supported and the second one is not. This is why the latter test is
marked as expected to fail. The support for TLS-in-TLS in the upstream
stdlib asyncio is currently disabled but is available in Python 3.9
via monkey-patching which is demonstrated in the added tests.

Refs:
* https://bugs.python.org/issue37179
* python/cpython#28073
* aio-libs#5992

Co-authored-by: Sviatoslav Sydorenko <webknjaz@redhat.com>

PR aio-libs#6002

(cherry picked from commit d66e07c)
webknjaz added a commit that referenced this pull request Oct 3, 2021
…nst ``proxy.py`` (#6033)

This patch adds full end-to-end tests for sending requests to HTTP and
HTTPS endpoints through an HTTPS proxy. The first case is currently
supported and the second one is not. This is why the latter test is
marked as expected to fail. The support for TLS-in-TLS in the upstream
stdlib asyncio is currently disabled but is available in Python 3.9
via monkey-patching which is demonstrated in the added tests.

Refs:
* https://bugs.python.org/issue37179
* python/cpython#28073
* #5992

Co-authored-by: bmbouter <bmbouter@gmail.com>
Co-authored-by: Sviatoslav Sydorenko <webknjaz@redhat.com>

PR #6002

(cherry picked from commit d66e07c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot:chronographer:provided There is a change note present in this PR client infra python Pull requests that update Python code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants