Repository: abhinavsingh/proxy.py
Pull request #746 (2021-11-15 22:56:05)

Move GHA e2e integration job to pytest

codecov[bot]: # [Codecov](https://codecov.io/gh/abhinavsingh/proxy.py/pull/746?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh) Report
> Merging [#746](https://codecov.io/gh/abhinavsingh/proxy.py/pull/746?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh) (2446c70) into [develop](https://codecov.io/gh/abhinavsingh/proxy.py/commit/09c9b63b5d9211f94e5763ac854cd779ec449da5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh) (09c9b63) will **decrease** coverage by `0.05%`.
> The diff coverage is `88.88%`.

[![Impacted file tree graph](https://codecov.io/gh/abhinavsingh/proxy.py/pull/746/graphs/tree.svg?width=650&height=150&src=pr&token=Zh9J7b4la2&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh)](https://codecov.io/gh/abhinavsingh/proxy.py/pull/746?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh)

```diff
@@             Coverage Diff             @@
##           develop     #746      +/-   ##
===========================================
- Coverage    87.65%   87.59%   -0.06%     
===========================================
  Files          122      124       +2     
  Lines         5363     5386      +23     
  Branches       431      385      -46     
===========================================
+ Hits          4701     4718      +17     
- Misses         571      575       +4     
- Partials        91       93       +2     
```

| Flag | Coverage Δ | |
|---|---|---|
| pytest | `87.59% <88.88%> (-0.01%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/abhinavsingh/proxy.py/pull/746?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh) | Coverage Δ | |
|---|---|---|
| [proxy/common/flag.py](https://codecov.io/gh/abhinavsingh/proxy.py/pull/746/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh#diff-cHJveHkvY29tbW9uL2ZsYWcucHk=) | `90.97% <50.00%> (+0.06%)` | :arrow_up: |
| [proxy/common/utils.py](https://codecov.io/gh/abhinavsingh/proxy.py/pull/746/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh#diff-cHJveHkvY29tbW9uL3V0aWxzLnB5) | `91.12% <62.50%> (-0.75%)` | :arrow_down: |
| [tests/test\_set\_open\_file\_limit.py](https://codecov.io/gh/abhinavsingh/proxy.py/pull/746/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh#diff-dGVzdHMvdGVzdF9zZXRfb3Blbl9maWxlX2xpbWl0LnB5) | `96.42% <75.00%> (+0.13%)` | :arrow_up: |
| [proxy/common/\_compat.py](https://codecov.io/gh/abhinavsingh/proxy.py/pull/746/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh#diff-cHJveHkvY29tbW9uL19jb21wYXQucHk=) | `100.00% <100.00%> (ø)` | |
| [proxy/common/constants.py](https://codecov.io/gh/abhinavsingh/proxy.py/pull/746/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh#diff-cHJveHkvY29tbW9uL2NvbnN0YW50cy5weQ==) | `100.00% <100.00%> (+2.24%)` | :arrow_up: |
| [tests/core/test\_listener.py](https://codecov.io/gh/abhinavsingh/proxy.py/pull/746/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh#diff-dGVzdHMvY29yZS90ZXN0X2xpc3RlbmVyLnB5) | `100.00% <100.00%> (ø)` | |
| [tests/integration/test\_integration.py](https://codecov.io/gh/abhinavsingh/proxy.py/pull/746/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh#diff-dGVzdHMvaW50ZWdyYXRpb24vdGVzdF9pbnRlZ3JhdGlvbi5weQ==) | `100.00% <100.00%> (ø)` | |
| [tests/testing/test\_embed.py](https://codecov.io/gh/abhinavsingh/proxy.py/pull/746/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh#diff-dGVzdHMvdGVzdGluZy90ZXN0X2VtYmVkLnB5) | `91.17% <100.00%> (-2.95%)` | :arrow_down: |
| [proxy/common/types.py](https://codecov.io/gh/abhinavsingh/proxy.py/pull/746/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh#diff-cHJveHkvY29tbW9uL3R5cGVzLnB5) | `86.66% <0.00%> (-13.34%)` | :arrow_down: |
| [tests/http/test\_proxy\_protocol.py](https://codecov.io/gh/abhinavsingh/proxy.py/pull/746/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh#diff-dGVzdHMvaHR0cC90ZXN0X3Byb3h5X3Byb3RvY29sLnB5) | `96.00% <0.00%> (-4.00%)` | :arrow_down: |
| ... and [6 more](https://codecov.io/gh/abhinavsingh/proxy.py/pull/746/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/abhinavsingh/proxy.py/pull/746?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/abhinavsingh/proxy.py/pull/746?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh). Last update [09c9b63...2446c70](https://codecov.io/gh/abhinavsingh/proxy.py/pull/746?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Abhinav+Singh).


Patch:
diff --git a/.flake8 b/.flake8
index cf84c82..575ba2c 100644
--- a/.flake8
+++ b/.flake8
@@ -226,6 +226,8 @@ pytest-parametrize-names-type = tuple
 # PT007:
 pytest-parametrize-values-type = tuple
 pytest-parametrize-values-row-type = tuple
+# PT023:
+pytest-mark-no-parentheses = true
 
 # flake8-rst-docstrings
 rst-roles =
diff --git a/proxy/common/_compat.py b/proxy/common/_compat.py
new file mode 100644
index 0000000..389c81a
--- /dev/null
+++ b/proxy/common/_compat.py
@@ -0,0 +1,7 @@
+"""Compatibility code for using Proxy.py across various versions of Python."""
+
+import platform
+
+
+SYS_PLATFORM = platform.system()
+IS_WINDOWS = SYS_PLATFORM == 'Windows'
diff --git a/proxy/common/constants.py b/proxy/common/constants.py
index 0451bf8..4a34ba4 100644
--- a/proxy/common/constants.py
+++ b/proxy/common/constants.py
@@ -18,15 +18,14 @@ import ipaddress
 
 from typing import Any, List
 
+from ._compat import IS_WINDOWS  # noqa: WPS436
 from .version import __version__
 
 
 def _env_threadless_compliant() -> bool:
     """Returns true for Python 3.8+ across all platforms
     except Windows."""
-    if os.name == 'nt':
-        return False
-    return sys.version_info >= (3, 8)
+    return not IS_WINDOWS and sys.version_info >= (3, 8)
 
 
 PROXY_PY_START_TIME = time.time()
diff --git a/proxy/common/flag.py b/proxy/common/flag.py
index 5c3de44..96d65d5 100644
--- a/proxy/common/flag.py
+++ b/proxy/common/flag.py
@@ -19,6 +19,7 @@ import multiprocessing
 
 from typing import Optional, List, Any, cast
 
+from ._compat import IS_WINDOWS  # noqa: WPS436
 from .plugins import Plugins
 from .types import IpAddress
 from .utils import bytes_, is_py2, set_open_file_limit
@@ -255,7 +256,7 @@ class FlagParser:
         )
         # AF_UNIX is not available on Windows
         # See https://bugs.python.org/issue33408
-        if os.name != 'nt':
+        if not IS_WINDOWS:
             args.family = socket.AF_UNIX if args.unix_socket_path else (
                 socket.AF_INET6 if args.hostname.version == 6 else socket.AF_INET
             )
diff --git a/proxy/common/utils.py b/proxy/common/utils.py
index f65cf10..52b7ac2 100644
--- a/proxy/common/utils.py
+++ b/proxy/common/utils.py
@@ -8,7 +8,6 @@
     :copyright: (c) 2013-present by Abhinav Singh and contributors.
     :license: BSD, see LICENSE for more details.
 """
-import os
 import sys
 import ssl
 import socket
@@ -20,9 +19,10 @@ import contextlib
 from types import TracebackType
 from typing import Optional, Dict, Any, List, Tuple, Type, Callable
 
+from ._compat import IS_WINDOWS  # noqa: WPS436
 from .constants import HTTP_1_1, COLON, WHITESPACE, CRLF, DEFAULT_TIMEOUT, DEFAULT_THREADLESS
 
-if os.name != 'nt':
+if not IS_WINDOWS:
     import resource
 
 logger = logging.getLogger(__name__)
@@ -272,14 +272,16 @@ def get_available_port() -> int:
 
 def set_open_file_limit(soft_limit: int) -> None:
     """Configure open file description soft limit on supported OS."""
-    if os.name != 'nt':  # resource module not available on Windows OS
-        curr_soft_limit, curr_hard_limit = resource.getrlimit(
-            resource.RLIMIT_NOFILE,
+    if IS_WINDOWS:  # resource module not available on Windows OS
+        return
+
+    curr_soft_limit, curr_hard_limit = resource.getrlimit(
+        resource.RLIMIT_NOFILE,
+    )
+    if curr_soft_limit < soft_limit < curr_hard_limit:
+        resource.setrlimit(
+            resource.RLIMIT_NOFILE, (soft_limit, curr_hard_limit),
+        )
+        logger.debug(
+            'Open file soft limit set to %d', soft_limit,
         )
-        if curr_soft_limit < soft_limit < curr_hard_limit:
-            resource.setrlimit(
-                resource.RLIMIT_NOFILE, (soft_limit, curr_hard_limit),
-            )
-            logger.debug(
-                'Open file soft limit set to %d', soft_limit,
-            )


Test patch:
diff --git a/.github/workflows/test-library.yml b/.github/workflows/test-library.yml
index 090d4cc..f668400 100644
--- a/.github/workflows/test-library.yml
+++ b/.github/workflows/test-library.yml
@@ -210,49 +210,6 @@ jobs:
             || steps.scm-version.outputs.dist-version
         }}-py3-none-any.whl')
 
-  integration:
-    runs-on: ${{ matrix.os }}-latest
-    name: >-
-      e2e: 🐍${{ matrix.python }} @ ${{ matrix.os }}
-    needs:
-    - build
-    - pre-setup  # transitive, for accessing settings
-    strategy:
-      matrix:
-        os: [Ubuntu, macOS]
-        python: ['3.6', '3.7', '3.8', '3.9', '3.10']
-      max-parallel: 4
-      fail-fast: false
-    steps:
-    - uses: actions/checkout@v2
-      with:
-        ref: ${{ github.event.inputs.release-commitish }}
-    - name: Make the env clean of non-test files
-      run: |
-        shopt -s extglob
-        mv -v tests/integration/main.sh integration-test.sh
-        rm -rf !integration-test.sh
-      shell: bash
-    - name: Setup Python
-      uses: actions/setup-python@v2
-      with:
-        python-version: ${{ matrix.python }}
-    - name: Download all the dists
-      uses: actions/download-artifact@v2
-      with:
-        name: python-package-distributions
-        path: dist/
-    - name: Integration testing
-      run: |
-        pip install 'dist/${{ needs.pre-setup.outputs.wheel-artifact-name }}'
-        proxy \
-          --hostname 127.0.0.1 \
-          --enable-web-server \
-          --pid-file proxy.pid \
-          --log-file proxy.log \
-          &
-        ./integration-test.sh
-
   build:
     name: build dists ${{ needs.pre-setup.outputs.git-tag }}
     needs:
@@ -577,7 +534,6 @@ jobs:
 
   check:  # This job does nothing and is only used for the branch protection
     needs:
-    - integration
     - lint
     - test
 
diff --git a/pytest.ini b/pytest.ini
index 650a369..bc8ff5b 100644
--- a/pytest.ini
+++ b/pytest.ini
@@ -65,6 +65,7 @@ junit_suite_name = proxy_py_test_suite
 
 # A mapping of markers to their descriptions allowed in strict mode:
 markers =
+  smoke: Quick self-check smoke tests
 
 minversion = 6.2.0
 
diff --git a/tests/core/test_listener.py b/tests/core/test_listener.py
index 33cf990..667b6f0 100644
--- a/tests/core/test_listener.py
+++ b/tests/core/test_listener.py
@@ -15,7 +15,10 @@ import unittest
 
 from unittest import mock
 
+import pytest
+
 from proxy.core.acceptor import Listener
+from proxy.common._compat import IS_WINDOWS  # noqa: WPS436
 from proxy.common.flag import FlagParser
 
 
@@ -43,7 +46,15 @@ class TestListener(unittest.TestCase):
         listener.shutdown()
         sock.close.assert_called_once()
 
-    @unittest.skipIf(os.name == 'nt', 'AF_UNIX not available on windows')
+    # FIXME: Ignore is necessary for as long as pytest hasn't figured out
+    # FIXME: typing for their fixtures.
+    # Refs:
+    # * https://github.com/pytest-dev/pytest/issues/7469#issuecomment-918345196
+    # * https://github.com/pytest-dev/pytest/issues/3342
+    @pytest.mark.skipif(
+        IS_WINDOWS,
+        reason='AF_UNIX not available on Windows',
+    )  # type: ignore[misc]
     @mock.patch('os.remove')
     @mock.patch('socket.socket')
     def test_unix_path_listener(self, mock_socket: mock.Mock, mock_remove: mock.Mock) -> None:
diff --git a/tests/integration/test_integration.py b/tests/integration/test_integration.py
new file mode 100644
index 0000000..cc07d0e
--- /dev/null
+++ b/tests/integration/test_integration.py
@@ -0,0 +1,52 @@
+"""Test the simplest proxy use scenario for smoke."""
+from pathlib import Path
+from subprocess import check_output, Popen
+from typing import Generator
+
+import pytest
+
+from proxy.common._compat import IS_WINDOWS  # noqa: WPS436
+
+
+# FIXME: Ignore is necessary for as long as pytest hasn't figured out
+# FIXME: typing for their fixtures.
+# Refs:
+# * https://github.com/pytest-dev/pytest/issues/7469#issuecomment-918345196
+# * https://github.com/pytest-dev/pytest/issues/3342
+@pytest.fixture  # type: ignore[misc]
+def _proxy_py_instance() -> Generator[None, None, None]:
+    """Instantiate proxy.py in a subprocess for testing.
+
+    After the testing is over, tear it down.
+    """
+    proxy_cmd = (
+        'proxy',
+        '--hostname', '127.0.0.1',
+        '--enable-web-server',
+    )
+    proxy_proc = Popen(proxy_cmd)
+    try:
+        yield
+    finally:
+        proxy_proc.terminate()
+        proxy_proc.wait(1)
+
+
+# FIXME: Ignore is necessary for as long as pytest hasn't figured out
+# FIXME: typing for their fixtures.
+# Refs:
+# * https://github.com/pytest-dev/pytest/issues/7469#issuecomment-918345196
+# * https://github.com/pytest-dev/pytest/issues/3342
+@pytest.mark.smoke  # type: ignore[misc]
+@pytest.mark.usefixtures('_proxy_py_instance')  # type: ignore[misc]
+@pytest.mark.xfail(
+    IS_WINDOWS,
+    reason='OSError: [WinError 193] %1 is not a valid Win32 application',
+    raises=OSError,
+)  # type: ignore[misc]
+def test_curl() -> None:
+    """An acceptance test with using ``curl`` through proxy.py."""
+    this_test_module = Path(__file__)
+    shell_script_test = this_test_module.with_suffix('.sh')
+
+    check_output(str(shell_script_test))
diff --git a/tests/integration/main.sh b/tests/integration/test_integration.sh
similarity index 100%
rename from tests/integration/main.sh
rename to tests/integration/test_integration.sh
diff --git a/tests/test_set_open_file_limit.py b/tests/test_set_open_file_limit.py
index 2c55eee..c785b5a 100644
--- a/tests/test_set_open_file_limit.py
+++ b/tests/test_set_open_file_limit.py
@@ -8,17 +8,22 @@
     :copyright: (c) 2013-present by Abhinav Singh and contributors.
     :license: BSD, see LICENSE for more details.
 """
-import os
 import unittest
 from unittest import mock
 
+import pytest
+
+from proxy.common._compat import IS_WINDOWS  # noqa: WPS436
 from proxy.common.utils import set_open_file_limit
 
-if os.name != 'nt':
+if not IS_WINDOWS:
     import resource
 
 
-@unittest.skipIf(os.name == 'nt', 'Open file limit tests disabled for Windows')
+@pytest.mark.skipif(
+    IS_WINDOWS,
+    reason='Open file limit tests disabled for Windows',
+)
 class TestSetOpenFileLimit(unittest.TestCase):
 
     @mock.patch('resource.getrlimit', return_value=(128, 1024))
diff --git a/tests/testing/test_embed.py b/tests/testing/test_embed.py
index a1e2af0..3bb9912 100644
--- a/tests/testing/test_embed.py
+++ b/tests/testing/test_embed.py
@@ -8,20 +8,24 @@
     :copyright: (c) 2013-present by Abhinav Singh and contributors.
     :license: BSD, see LICENSE for more details.
 """
-import os
-import unittest
 import http.client
 import urllib.request
 import urllib.error
 
+import pytest
+
 from proxy import TestCase
+from proxy.common._compat import IS_WINDOWS  # noqa: WPS436
 from proxy.common.constants import DEFAULT_CLIENT_RECVBUF_SIZE, PROXY_AGENT_HEADER_VALUE
 from proxy.common.utils import socket_connection, build_http_request
 from proxy.http import httpMethods
 from proxy.http.server import HttpWebServerPlugin
 
 
-@unittest.skipIf(os.name == 'nt', 'Disabled for Windows due to weird permission issues.')
+@pytest.mark.skipif(
+    IS_WINDOWS,
+    reason='Disabled for Windows due to weird permission issues.',
+)
 class TestProxyPyEmbedded(TestCase):
     """This test case is a demonstration of proxy.TestCase and also serves as
     integration test suite for proxy.py."""