Skip to content

Commit

Permalink
chore: Remove requirement for pytest
Browse files Browse the repository at this point in the history
pytest was required only by the `PythonStandardRunner`, and only in the `strict_mode`.
No one uses that.
At first it was develop because we were concerned that users would be picky by us running pytest with `subprocess.run`,
so `strict_mode` was an alternative. Since then people are starting to use ATS and that hasn't been an issue.

Plus with custom runners we can solve this problem without the complications of requiring pytest.
And there's a possibility (although I think it's very unlikely) that importing pytest from the CLI
might cause some of the issues we were seeing when running tests from it in ATS. Who knows.

In any case, removing pytest and all code associated with it.
  • Loading branch information
giovanni-guidini committed Oct 2, 2023
1 parent 127e8af commit 3cef305
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 328 deletions.
97 changes: 3 additions & 94 deletions codecov_cli/runners/python_standard_runner.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,13 @@
import logging
import random
import subprocess
from contextlib import redirect_stdout
from io import StringIO, TextIOWrapper
from multiprocessing import Process, Queue, get_context
from os import getcwd
from multiprocessing import Process, Queue
from queue import Empty
from subprocess import CalledProcessError
from sys import path, stdout
from typing import List, Optional

import click
import pytest

from codecov_cli.runners.types import (
LabelAnalysisRequestResult,
Expand Down Expand Up @@ -43,58 +39,6 @@ def coverage_root(self) -> str:
"""
return self.get("coverage_root", "./")

@property
def strict_mode(self) -> bool:
"""
Run pytest from within Python instead of using subprocess.run
This is potentailly safer than using subprocess.run because it guarantees better that
the program running is indeed pytest.
But it might not work everytime due to import issues related to Python caching modules.
"""
return self.get("strict_mode", False)


def _include_curr_dir(method):
"""
Account for the difference 'pytest' vs 'python -m pytest'
https://docs.pytest.org/en/7.1.x/how-to/usage.html#calling-pytest-through-python-m-pytest
Used only in strict_mode
"""

def call_method(self, *args, **kwargs):
curr_dir = getcwd()
path.append(curr_dir)

result = method(self, *args, **kwargs)

path.remove(curr_dir)
return result

return call_method


def _execute_pytest_subprocess(
pytest_args: List[str],
queue: Queue,
parent_stdout: TextIOWrapper,
capture_output: bool = True,
):
"""Runs pytest from python in a subprocess.
This is because we call it twice in the label-analysis process,
so we might have import errors if calling it directly.
Check the warning: https://docs.pytest.org/en/7.1.x/how-to/usage.html#calling-pytest-from-python-code
Returns the output value and pytest exit code via queue
"""
subproces_stdout = parent_stdout
if capture_output:
subproces_stdout = StringIO()
with redirect_stdout(subproces_stdout):
result = pytest.main(pytest_args)
if capture_output:
queue.put({"output": subproces_stdout.getvalue()})
queue.put({"result": result})


class PythonStandardRunner(LabelAnalysisRunnerInterface):

Expand Down Expand Up @@ -125,35 +69,6 @@ def _wait_pytest(self, pytest_process: Process, queue: Queue):
pytest_process.join()
return result, output

@_include_curr_dir
def _execute_pytest_strict(
self, pytest_args: List[str], capture_output: bool = True
) -> str:
"""Handles calling pytest from Python in a subprocess.
Raises Exception if pytest fails
Returns the complete pytest output
"""
ctx = get_context(method="fork")
queue = ctx.Queue(2)
p = ctx.Process(
target=_execute_pytest_subprocess,
args=[pytest_args, queue, stdout, capture_output],
)
result, output = self._wait_pytest(p, queue)

if p.exitcode != 0 or (result != pytest.ExitCode.OK and result != 0):
message = f"Pytest exited with non-zero code {result}."
message += "\nThis is likely not a problem with label-analysis. Check pytest's output and options."
if capture_output:
# If pytest failed but we captured its output the user won't know what's wrong
# So we need to include that in the error message
message += "\nPYTEST OUTPUT:"
message += "\n" + output
else:
message += "\n(you can check pytest options on the logs before the test session start)"
raise click.ClickException(message)
return output

def parse_captured_output_error(self, exp: CalledProcessError) -> str:
result = ""
for out_stream in [exp.stdout, exp.stderr]:
Expand Down Expand Up @@ -202,10 +117,7 @@ def collect_tests(self):
),
)

if self.params.strict_mode:
output = self._execute_pytest_strict(options_to_use)
else:
output = self._execute_pytest(options_to_use)
output = self._execute_pytest(options_to_use)
lines = output.split(sep="\n")
test_names = list(line for line in lines if ("::" in line and "test" in line))
return test_names
Expand Down Expand Up @@ -254,10 +166,7 @@ def process_labelanalysis_result(self, result: LabelAnalysisRequestResult):
"List of tests executed",
extra=dict(extra_log_attributes=dict(executed_tests=tests_to_run)),
)
if self.params.strict_mode:
output = self._execute_pytest_strict(command_array, capture_output=False)
else:
output = self._execute_pytest(command_array, capture_output=False)
output = self._execute_pytest(command_array, capture_output=False)
logger.info(f"Finished running {len(tests_to_run)} tests successfully")
logger.info(f" pytest options: \"{' '.join(default_options)}\"")
logger.debug(output)
36 changes: 3 additions & 33 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,19 @@
# This file is autogenerated by pip-compile with Python 3.10
# by the following command:
#
# pip-compile --output-file=requirements.txt requirements.in setup.py
# pip-compile setup.py
#
anyio==4.0.0
# via httpcore
attrs==21.4.0
# via pytest
certifi==2023.7.22
# via
# httpcore
# httpx
# requests
charset-normalizer==3.2.0
charset-normalizer==3.3.0
# via requests
click==8.1.7
# via codecov-cli (setup.py)
coverage[toml]==7.3.1
# via pytest-cov
exceptiongroup==1.1.3
# via anyio
h11==0.14.0
Expand All @@ -34,28 +30,6 @@ idna==3.4
# rfc3986
ijson==3.2.3
# via codecov-cli (setup.py)
iniconfig==1.1.1
# via pytest
packaging==21.3
# via pytest
pluggy==1.0.0
# via pytest
py==1.11.0
# via pytest
pyparsing==3.0.9
# via packaging
pytest==7.1.2
# via
# codecov-cli (setup.py)
# pytest-asyncio
# pytest-cov
# pytest-mock
pytest-asyncio==0.21.1
# via -r requirements.in
pytest-cov==4.1.0
# via codecov-cli (setup.py)
pytest-mock==3.11.1
# via -r requirements.in
pyyaml==6.0.1
# via codecov-cli (setup.py)
requests==2.31.0
Expand All @@ -71,13 +45,9 @@ sniffio==1.3.0
# anyio
# httpcore
# httpx
tomli==2.0.1
# via
# coverage
# pytest
tree-sitter==0.20.2
# via codecov-cli (setup.py)
urllib3==2.0.4
urllib3==2.0.6
# via
# requests
# responses
2 changes: 0 additions & 2 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@
"click==8.*",
"httpx==0.23.*",
"ijson==3.*",
"pytest==7.*",
"pytest-cov>=3",
"pyyaml==6.*",
"responses==0.21.*",
"smart-open==6.*",
Expand Down
2 changes: 2 additions & 0 deletions requirements.in → tests/requirements.in
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
pytest
pytest-cov
pytest-mock
pytest-asyncio
30 changes: 30 additions & 0 deletions tests/requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#
# This file is autogenerated by pip-compile with Python 3.10
# by the following command:
#
# pip-compile tests/requirements.in
#
coverage[toml]==7.3.1
# via pytest-cov
exceptiongroup==1.1.3
# via pytest
iniconfig==2.0.0
# via pytest
packaging==23.2
# via pytest
pluggy==1.3.0
# via pytest
pytest==7.4.2
# via
# -r tests/requirements.in
# pytest-asyncio
# pytest-cov
# pytest-mock
pytest-asyncio==0.21.1
# via -r tests/requirements.in
pytest-cov==4.1.0
# via -r tests/requirements.in
pytest-mock==3.11.1
# via -r tests/requirements.in
tomli==2.0.1
# via pytest
Loading

0 comments on commit 3cef305

Please sign in to comment.