From dec2c9778563a287cf3e0d7c5042c32448d39d32 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Wed, 28 Aug 2024 11:45:22 -0700 Subject: [PATCH 1/2] fix django manage.py path validation --- python_files/tests/pytestadapter/helpers.py | 96 +++++++++++-------- .../tests/unittestadapter/test_discovery.py | 19 ++++ .../unittestadapter/django_handler.py | 12 +-- 3 files changed, 78 insertions(+), 49 deletions(-) diff --git a/python_files/tests/pytestadapter/helpers.py b/python_files/tests/pytestadapter/helpers.py index 4f6631a44c00..b537b4834994 100644 --- a/python_files/tests/pytestadapter/helpers.py +++ b/python_files/tests/pytestadapter/helpers.py @@ -139,57 +139,68 @@ def _listen_on_pipe_new(listener, result: List[str], completed: threading.Event) Created as a separate function for clarity in threading context. """ # Windows design - if sys.platform == "win32": - all_data: list = [] - stream = listener.wait() - while True: - # Read data from collection - close = stream.closed - if close: - break - data = stream.readlines() - if not data: - if completed.is_set(): - break # Exit loop if completed event is set - else: - try: - # Attempt to accept another connection if the current one closes unexpectedly - print("attempt another connection") - except socket.timeout: - # On timeout, append all collected data to result and return - # result.append("".join(all_data)) - return - data_decoded = "".join(data) - all_data.append(data_decoded) - # Append all collected data to result array - result.append("".join(all_data)) - else: # Unix design - connection, _ = listener.socket.accept() - listener.socket.settimeout(1) - all_data: list = [] - while True: - # Reading from connection - data: bytes = connection.recv(1024 * 1024) - if not data: - if completed.is_set(): - break # Exit loop if completed event is set + while not completed.is_set(): + listener.socket.settimeout(10) # Set a timeout for the accept call + if sys.platform == "win32": + all_data: list = [] + stream = listener.wait() + while not completed.is_set(): + # Read data from collection + close = stream.closed + if close: + break + data = stream.readlines() + if not data: + if completed.is_set(): + break # Exit loop if completed event is set else: try: # Attempt to accept another connection if the current one closes unexpectedly - connection, _ = listener.socket.accept() + print("attempt another connection") except socket.timeout: # On timeout, append all collected data to result and return - result.append("".join(all_data)) + # result.append("".join(all_data)) return - all_data.append(data.decode("utf-8")) - # Append all collected data to result array + data_decoded = "".join(data) + all_data.append(data_decoded) + # Append all collected data to result array + result.append("".join(all_data)) + else: # Unix design + while not completed.is_set(): + listener.socket.settimeout(10) # Set a timeout for the accept call + connection, _ = listener.socket.accept() + listener.socket.settimeout(1) + all_data: list = [] + while not completed.is_set(): + # Reading from connection + data: bytes = connection.recv(1024 * 1024) + if not data: + if completed.is_set(): + break # Exit loop if completed event is set + else: + try: + # Attempt to accept another connection if the current one closes unexpectedly + connection, _ = listener.socket.accept() + except socket.timeout: + # On timeout, append all collected data to result and return + result.append("".join(all_data)) + return + all_data.append(data.decode("utf-8")) + # Append all collected data to result array result.append("".join(all_data)) def _run_test_code(proc_args: List[str], proc_env, proc_cwd: str, completed: threading.Event): - result = subprocess.run(proc_args, env=proc_env, cwd=proc_cwd) - completed.set() - return result + try: + result = subprocess.run(proc_args, env=proc_env, cwd=proc_cwd) + if result.returncode != 0: + return "ERROR" + completed.set() + return result + except Exception: + return "ERROR" + finally: + completed.set() def runner(args: List[str]) -> Optional[List[Dict[str, Any]]]: @@ -289,6 +300,9 @@ def runner_with_cwd_env( t1.join() t2.join() + if result == [] or result[0] == "ERROR": + raise RuntimeError("Error occurred when running test code") + return process_data_received(result[0]) if result else None diff --git a/python_files/tests/unittestadapter/test_discovery.py b/python_files/tests/unittestadapter/test_discovery.py index 972556de999b..919d304ef018 100644 --- a/python_files/tests/unittestadapter/test_discovery.py +++ b/python_files/tests/unittestadapter/test_discovery.py @@ -325,3 +325,22 @@ def test_simple_django_collect(): assert ( len(actual_item["tests"]["children"][0]["children"][0]["children"][0]["children"]) == 3 ) + + +def test_django_bad_manage_py_path(): + test_data_path: pathlib.Path = pathlib.Path(__file__).parent / ".data" + python_files_path: pathlib.Path = pathlib.Path(__file__).parent.parent.parent + discovery_script_path: str = os.fsdecode(python_files_path / "unittestadapter" / "discovery.py") + data_path: pathlib.Path = test_data_path / "simple_django" + data_path_wrong: pathlib.Path = test_data_path / "simple_django_wrong" + manage_py_path: str = os.fsdecode(pathlib.Path(data_path_wrong, "manage.py")) + + with pytest.raises(RuntimeError): + helpers.runner_with_cwd_env( + [ + discovery_script_path, + "--udiscovery", + ], + data_path, + {"MANAGE_PY_PATH": manage_py_path}, + ) diff --git a/python_files/unittestadapter/django_handler.py b/python_files/unittestadapter/django_handler.py index dc520c13aff1..9daa816d0918 100644 --- a/python_files/unittestadapter/django_handler.py +++ b/python_files/unittestadapter/django_handler.py @@ -18,10 +18,8 @@ def django_discovery_runner(manage_py_path: str, args: List[str]) -> None: # Attempt a small amount of validation on the manage.py path. - try: - pathlib.Path(manage_py_path) - except Exception as e: - raise VSCodeUnittestError(f"Error running Django, manage.py path is not a valid path: {e}") # noqa: B904 + if not pathlib.Path(manage_py_path).exists(): + raise VSCodeUnittestError("Error running Django, manage.py path does not exist.") try: # Get path to the custom_test_runner.py parent folder, add to sys.path and new environment used for subprocess. @@ -61,10 +59,8 @@ def django_discovery_runner(manage_py_path: str, args: List[str]) -> None: def django_execution_runner(manage_py_path: str, test_ids: List[str], args: List[str]) -> None: # Attempt a small amount of validation on the manage.py path. - try: - pathlib.Path(manage_py_path) - except Exception as e: - raise VSCodeUnittestError(f"Error running Django, manage.py path is not a valid path: {e}") # noqa: B904 + if not pathlib.Path(manage_py_path).exists(): + raise VSCodeUnittestError("Error running Django, manage.py path does not exist.") try: # Get path to the custom_test_runner.py parent folder, add to sys.path. From 7603160dd161a1bb669bce07fe8ef05769a25779 Mon Sep 17 00:00:00 2001 From: eleanorjboyd Date: Wed, 28 Aug 2024 14:20:54 -0700 Subject: [PATCH 2/2] remove incorrect tests --- python_files/tests/pytestadapter/helpers.py | 96 ++++++++----------- .../tests/unittestadapter/test_discovery.py | 19 ---- 2 files changed, 41 insertions(+), 74 deletions(-) diff --git a/python_files/tests/pytestadapter/helpers.py b/python_files/tests/pytestadapter/helpers.py index b537b4834994..4f6631a44c00 100644 --- a/python_files/tests/pytestadapter/helpers.py +++ b/python_files/tests/pytestadapter/helpers.py @@ -139,68 +139,57 @@ def _listen_on_pipe_new(listener, result: List[str], completed: threading.Event) Created as a separate function for clarity in threading context. """ # Windows design - while not completed.is_set(): - listener.socket.settimeout(10) # Set a timeout for the accept call - if sys.platform == "win32": - all_data: list = [] - stream = listener.wait() - while not completed.is_set(): - # Read data from collection - close = stream.closed - if close: - break - data = stream.readlines() - if not data: - if completed.is_set(): - break # Exit loop if completed event is set + if sys.platform == "win32": + all_data: list = [] + stream = listener.wait() + while True: + # Read data from collection + close = stream.closed + if close: + break + data = stream.readlines() + if not data: + if completed.is_set(): + break # Exit loop if completed event is set + else: + try: + # Attempt to accept another connection if the current one closes unexpectedly + print("attempt another connection") + except socket.timeout: + # On timeout, append all collected data to result and return + # result.append("".join(all_data)) + return + data_decoded = "".join(data) + all_data.append(data_decoded) + # Append all collected data to result array + result.append("".join(all_data)) + else: # Unix design + connection, _ = listener.socket.accept() + listener.socket.settimeout(1) + all_data: list = [] + while True: + # Reading from connection + data: bytes = connection.recv(1024 * 1024) + if not data: + if completed.is_set(): + break # Exit loop if completed event is set else: try: # Attempt to accept another connection if the current one closes unexpectedly - print("attempt another connection") + connection, _ = listener.socket.accept() except socket.timeout: # On timeout, append all collected data to result and return - # result.append("".join(all_data)) + result.append("".join(all_data)) return - data_decoded = "".join(data) - all_data.append(data_decoded) - # Append all collected data to result array - result.append("".join(all_data)) - else: # Unix design - while not completed.is_set(): - listener.socket.settimeout(10) # Set a timeout for the accept call - connection, _ = listener.socket.accept() - listener.socket.settimeout(1) - all_data: list = [] - while not completed.is_set(): - # Reading from connection - data: bytes = connection.recv(1024 * 1024) - if not data: - if completed.is_set(): - break # Exit loop if completed event is set - else: - try: - # Attempt to accept another connection if the current one closes unexpectedly - connection, _ = listener.socket.accept() - except socket.timeout: - # On timeout, append all collected data to result and return - result.append("".join(all_data)) - return - all_data.append(data.decode("utf-8")) - # Append all collected data to result array + all_data.append(data.decode("utf-8")) + # Append all collected data to result array result.append("".join(all_data)) def _run_test_code(proc_args: List[str], proc_env, proc_cwd: str, completed: threading.Event): - try: - result = subprocess.run(proc_args, env=proc_env, cwd=proc_cwd) - if result.returncode != 0: - return "ERROR" - completed.set() - return result - except Exception: - return "ERROR" - finally: - completed.set() + result = subprocess.run(proc_args, env=proc_env, cwd=proc_cwd) + completed.set() + return result def runner(args: List[str]) -> Optional[List[Dict[str, Any]]]: @@ -300,9 +289,6 @@ def runner_with_cwd_env( t1.join() t2.join() - if result == [] or result[0] == "ERROR": - raise RuntimeError("Error occurred when running test code") - return process_data_received(result[0]) if result else None diff --git a/python_files/tests/unittestadapter/test_discovery.py b/python_files/tests/unittestadapter/test_discovery.py index 919d304ef018..972556de999b 100644 --- a/python_files/tests/unittestadapter/test_discovery.py +++ b/python_files/tests/unittestadapter/test_discovery.py @@ -325,22 +325,3 @@ def test_simple_django_collect(): assert ( len(actual_item["tests"]["children"][0]["children"][0]["children"][0]["children"]) == 3 ) - - -def test_django_bad_manage_py_path(): - test_data_path: pathlib.Path = pathlib.Path(__file__).parent / ".data" - python_files_path: pathlib.Path = pathlib.Path(__file__).parent.parent.parent - discovery_script_path: str = os.fsdecode(python_files_path / "unittestadapter" / "discovery.py") - data_path: pathlib.Path = test_data_path / "simple_django" - data_path_wrong: pathlib.Path = test_data_path / "simple_django_wrong" - manage_py_path: str = os.fsdecode(pathlib.Path(data_path_wrong, "manage.py")) - - with pytest.raises(RuntimeError): - helpers.runner_with_cwd_env( - [ - discovery_script_path, - "--udiscovery", - ], - data_path, - {"MANAGE_PY_PATH": manage_py_path}, - )