From bb465fff6f01f3f90a77229468f7e08a3bdbce20 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Francis=20Clairicia-Rose-Claire-Jos=C3=A9phine?=
 <francis.clairicia-rose-claire-josephine@epitech.eu>
Date: Mon, 24 Jun 2024 23:36:20 +0200
Subject: [PATCH] Fix: Prevent `ResourceWarning` when using `pytest-xdist`
 (#39)

* Prevent ResourceWarning when using pytest-xdist

* Do not need to call shutdown() in ClientReporter

* tox: Run tests with AND without xdist

* tox: Restrict xdist version

* Added a unit test for socket closing when using xdist

* Fix: the unit test uses the '-Werror' flag to catch ResourceWarnings

* Fix: Updated branch from main

* Fix: Formatting

* Fix: flake8 issues fixed

* Fix: Clean post review

* fix: Removed tox matrix for tests

* fix: Skip yanked version of xdist

* Removed concurrency directive in CI
---
 .gitignore                 |  2 ++
 pytest_retry/server.py     | 21 ++++++++++--------
 tests/test_retry_plugin.py | 44 ++++++++++++++++++++++++++++++++++----
 tox.ini                    |  3 ++-
 4 files changed, 56 insertions(+), 14 deletions(-)

diff --git a/.gitignore b/.gitignore
index ec8438e..fd5e659 100644
--- a/.gitignore
+++ b/.gitignore
@@ -11,3 +11,5 @@ config.ini
 credentials.ini
 dist/
 venv/
+.vscode/
+.python-version
diff --git a/pytest_retry/server.py b/pytest_retry/server.py
index b63b79a..060823b 100644
--- a/pytest_retry/server.py
+++ b/pytest_retry/server.py
@@ -1,16 +1,12 @@
 import socket
 import threading
 from io import StringIO
-from _pytest.terminal import TerminalReporter
 
 
 class ReportHandler:
     def __init__(self) -> None:
         self.stream = StringIO()
 
-    def build_retry_report(self, terminalreporter: TerminalReporter) -> None:
-        pass
-
     def record_attempt(self, lines: list[str]) -> None:
         pass
 
@@ -30,6 +26,9 @@ def __init__(self) -> None:
         self.sock.setblocking(True)
         self.sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
 
+    def __del__(self) -> None:
+        self.sock.close()
+
     def initialize_server(self) -> int:
         self.sock.bind(("localhost", 0))
         t = threading.Thread(target=self.run_server, daemon=True)
@@ -41,11 +40,12 @@ def run_server(self) -> None:
         while True:
             conn, _ = self.sock.accept()
 
-            while True:
-                chunk = conn.recv(128)
-                if not chunk:
-                    break
-                self.stream.write(chunk.decode("utf-8"))
+            with conn:
+                while True:
+                    chunk = conn.recv(4096)
+                    if not chunk:
+                        break
+                    self.stream.write(chunk.decode("utf-8"))
 
 
 class ClientReporter(ReportHandler):
@@ -55,6 +55,9 @@ def __init__(self, port: int) -> None:
         self.sock.setblocking(True)
         self.sock.connect(("localhost", port))
 
+    def __del__(self) -> None:
+        self.sock.close()
+
     def record_attempt(self, lines: list[str]) -> None:
         self.stream.writelines(lines)
         # Group reports for each item together before sending and resetting stream
diff --git a/tests/test_retry_plugin.py b/tests/test_retry_plugin.py
index 89b129a..5842d3f 100644
--- a/tests/test_retry_plugin.py
+++ b/tests/test_retry_plugin.py
@@ -9,6 +9,11 @@
 
 pytest_plugins = ["pytester"]
 
+xdist_test_marker = mark.skipif(
+    not xdist_installed,
+    reason="Only run if xdist is installed locally"
+)
+
 
 def check_outcome_field(outcomes, field_name, expected_value):
     field_value = outcomes.get(field_name, 0)
@@ -940,12 +945,10 @@ def test_eventually_passes():
     assert len([line for line in result.outlines if line.startswith('\t  File')]) == len(verbosity)
 
 
-@mark.skipif(xdist_installed is False, reason="Only run if xdist is installed locally")
-def test_xdist_reporting_compatability(testdir):
+@xdist_test_marker
+def test_xdist_reporting_compatibility(testdir):
     testdir.makepyfile(
         """
-        import pytest
-
         a = 0
         b = 0
 
@@ -969,3 +972,36 @@ def test_moar_flaky() -> None:
     assert "\ttest_flaky passed on attempt 3!" in result.outlines
     assert "\ttest_moar_flaky failed on attempt 1! Retrying!" in result.outlines
     assert "\ttest_moar_flaky passed on attempt 2!" in result.outlines
+
+
+@xdist_test_marker
+def test_xdist_resources_properly_closed_server_side(testdir):
+    # TODO: This test works for the sockets opened in the main process,
+    #       but there is no way to catch them inside the workers
+    #       (or at least, the author of this test didn't find it)
+
+    testdir.makepyfile(
+        """
+        a = 0
+        b = 0
+
+        def test_flaky() -> None:
+            global a
+
+            a += 1
+            assert a == 3
+
+        def test_moar_flaky() -> None:
+            global b
+
+            b += 1
+            assert b == 2
+        """
+    )
+
+    # The test MUST be run in a subprocess because the warnings appear
+    # on pytest teardown
+    result = testdir.runpytest_subprocess("-n", "2", "--retries", "3", "-W", "error")
+
+    for line in result.errlines:
+        assert "ResourceWarning" not in line
diff --git a/tox.ini b/tox.ini
index 3801764..d5ae3bb 100644
--- a/tox.ini
+++ b/tox.ini
@@ -10,11 +10,12 @@ python =
     3.10: py310, mypy, flake8
     3.11: py311
 
-[testenv]
+[testenv:{py39,py310,py311}]
 setenv =
     PYTHONPATH = {toxinidir}
 deps =
     -r{toxinidir}/dev-requirements.txt
+    pytest-xdist>=3.6.1,<4
 commands =
     pytest --basetemp={envtmpdir}