DEVX-13: feat: add per-test timing quality gate to check_test_speed
This commit is contained in:
emil
@@ -566,7 +566,8 @@ class TestVikunjaClient:
|
||||
json={"done": True},
|
||||
)
|
||||
|
||||
def test_http_error_raises_api_error(self) -> None:
|
||||
@patch("devx.api_clients.time.sleep")
|
||||
def test_http_error_raises_api_error(self, mock_sleep: MagicMock) -> None:
|
||||
client = VikunjaClient("https://work.example.com", "tok")
|
||||
mock_resp = MagicMock()
|
||||
mock_resp.raise_for_status.side_effect = _mock_http_error(http.HTTPStatus.INTERNAL_SERVER_ERROR, "server error")
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
"""Unit tests for scripts/check_test_speed.py."""
|
||||
"""Unit tests for devx.tools.check_test_speed."""
|
||||
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
@@ -8,10 +8,13 @@ from click.testing import CliRunner
|
||||
|
||||
from devx.tools.check_test_speed import (
|
||||
DEFAULT_MAX_SECONDS,
|
||||
DEFAULT_MAX_SINGLE_SECONDS,
|
||||
TEST_COMMAND,
|
||||
check_per_test_speed,
|
||||
check_speed,
|
||||
cli,
|
||||
parse_duration,
|
||||
parse_per_test_durations,
|
||||
run_tests,
|
||||
)
|
||||
|
||||
@@ -23,12 +26,23 @@ class TestRunTests:
|
||||
stdout, stderr = run_tests()
|
||||
assert stdout == "out"
|
||||
assert stderr == "err"
|
||||
mock_run.assert_called_once_with(
|
||||
TEST_COMMAND,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=False,
|
||||
)
|
||||
mock_run.assert_called_once()
|
||||
call_kwargs = mock_run.call_args
|
||||
assert call_kwargs.args[0] == TEST_COMMAND
|
||||
assert call_kwargs.kwargs["capture_output"] is True
|
||||
assert call_kwargs.kwargs["text"] is True
|
||||
assert call_kwargs.kwargs["check"] is False
|
||||
env = call_kwargs.kwargs["env"]
|
||||
assert "--durations=0" in env["PYTEST_ADDOPTS"]
|
||||
|
||||
@patch("devx.tools.check_test_speed.subprocess.run")
|
||||
def test_run_tests_preserves_existing_pytest_addopts(self, mock_run: MagicMock) -> None:
|
||||
mock_run.return_value = MagicMock(stdout="out", stderr="err", returncode=0)
|
||||
with patch.dict("os.environ", {"PYTEST_ADDOPTS": "-x"}, clear=False):
|
||||
run_tests()
|
||||
env = mock_run.call_args.kwargs["env"]
|
||||
assert "--durations=0" in env["PYTEST_ADDOPTS"]
|
||||
assert "-x" in env["PYTEST_ADDOPTS"]
|
||||
|
||||
|
||||
class TestParseDuration:
|
||||
@@ -48,6 +62,38 @@ class TestParseDuration:
|
||||
assert "Could not parse" in str(exc.value)
|
||||
|
||||
|
||||
class TestParsePerTestDurations:
|
||||
def test_parses_call_lines(self) -> None:
|
||||
output = "0.01s call tests/test_foo.py::test_bar\n"
|
||||
durations = parse_per_test_durations(output)
|
||||
assert len(durations) == 1
|
||||
assert durations[0] == ("tests/test_foo.py::test_bar", 0.01)
|
||||
|
||||
def test_parses_setup_and_teardown(self) -> None:
|
||||
output = (
|
||||
"0.02s setup tests/test_foo.py::test_bar\n"
|
||||
"0.01s call tests/test_foo.py::test_bar\n"
|
||||
"0.00s teardown tests/test_foo.py::test_bar\n"
|
||||
)
|
||||
durations = parse_per_test_durations(output)
|
||||
assert len(durations) == 3
|
||||
names = [d[0] for d in durations]
|
||||
assert "tests/test_foo.py::test_bar" in names
|
||||
|
||||
def test_sorted_slowest_first(self) -> None:
|
||||
output = "0.01s call tests/test_a.py::test_slow\n0.50s call tests/test_b.py::test_fast\n"
|
||||
durations = parse_per_test_durations(output)
|
||||
assert durations[0][1] >= durations[1][1]
|
||||
assert durations[0][1] == 0.50
|
||||
|
||||
def test_empty_output(self) -> None:
|
||||
assert parse_per_test_durations("") == []
|
||||
|
||||
def test_ignores_non_duration_lines(self) -> None:
|
||||
output = "Some random line\n234 passed in 0.70s\n"
|
||||
assert parse_per_test_durations(output) == []
|
||||
|
||||
|
||||
class TestCheckSpeed:
|
||||
def test_under_budget_passes(self) -> None:
|
||||
check_speed(1.0, 2.0) # should not raise
|
||||
@@ -64,6 +110,31 @@ class TestCheckSpeed:
|
||||
assert "max allowed: 2.0s" in msg
|
||||
|
||||
|
||||
class TestCheckPerTestSpeed:
|
||||
def test_no_violations_when_all_fast(self) -> None:
|
||||
durations = [("test_a", 0.1), ("test_b", 0.2)]
|
||||
assert check_per_test_speed(durations, 0.5) == []
|
||||
|
||||
def test_violation_when_test_exceeds_limit(self) -> None:
|
||||
durations = [("test_slow", 0.6), ("test_fast", 0.1)]
|
||||
violations = check_per_test_speed(durations, 0.5)
|
||||
assert len(violations) == 1
|
||||
assert "test_slow" in violations[0]
|
||||
assert "0.60s" in violations[0]
|
||||
|
||||
def test_multiple_violations(self) -> None:
|
||||
durations = [("test_a", 0.7), ("test_b", 0.6), ("test_c", 0.1)]
|
||||
violations = check_per_test_speed(durations, 0.5)
|
||||
assert len(violations) == 2
|
||||
|
||||
def test_exact_limit_passes(self) -> None:
|
||||
durations = [("test_a", 0.5)]
|
||||
assert check_per_test_speed(durations, 0.5) == []
|
||||
|
||||
def test_empty_durations(self) -> None:
|
||||
assert check_per_test_speed([], 0.5) == []
|
||||
|
||||
|
||||
def test_main_module_block() -> None:
|
||||
import devx.tools.check_test_speed as cts
|
||||
|
||||
@@ -77,39 +148,71 @@ class TestMain:
|
||||
@patch("devx.tools.check_test_speed.run_tests")
|
||||
@patch("devx.tools.check_test_speed.parse_duration")
|
||||
@patch("devx.tools.check_test_speed.check_speed")
|
||||
@patch("devx.tools.check_test_speed.parse_per_test_durations")
|
||||
@patch("devx.tools.check_test_speed.check_per_test_speed")
|
||||
def test_successful_run(
|
||||
self,
|
||||
mock_check_per: MagicMock,
|
||||
mock_parse_per: MagicMock,
|
||||
mock_check: MagicMock,
|
||||
mock_parse: MagicMock,
|
||||
mock_run: MagicMock,
|
||||
) -> None:
|
||||
mock_run.return_value = ("stdout\n", "stderr\n")
|
||||
mock_parse.return_value = 1.5
|
||||
mock_parse_per.return_value = []
|
||||
mock_check_per.return_value = []
|
||||
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(cli, [])
|
||||
assert result.exit_code == 0
|
||||
assert "1.50s" in result.output
|
||||
assert "under 2.0s limit" in result.output
|
||||
assert "under 10.0s limit" in result.output
|
||||
mock_run.assert_called_once()
|
||||
mock_parse.assert_called_once_with("stdout\n\nstderr\n")
|
||||
mock_check.assert_called_once_with(1.5, DEFAULT_MAX_SECONDS)
|
||||
mock_parse_per.assert_called_once()
|
||||
mock_check_per.assert_called_once_with([], DEFAULT_MAX_SINGLE_SECONDS)
|
||||
|
||||
@patch("devx.tools.check_test_speed.run_tests")
|
||||
@patch("devx.tools.check_test_speed.parse_duration")
|
||||
def test_slow_tests_exit(
|
||||
def test_slow_total_exits(
|
||||
self,
|
||||
mock_parse: MagicMock,
|
||||
mock_run: MagicMock,
|
||||
) -> None:
|
||||
mock_run.return_value = ("out\n", "err\n")
|
||||
mock_parse.return_value = 3.0
|
||||
mock_parse.return_value = 15.0
|
||||
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(cli, [])
|
||||
assert result.exit_code == 1
|
||||
assert "too slow" in result.output.lower()
|
||||
|
||||
@patch("devx.tools.check_test_speed.run_tests")
|
||||
@patch("devx.tools.check_test_speed.parse_duration")
|
||||
@patch("devx.tools.check_test_speed.check_speed")
|
||||
@patch("devx.tools.check_test_speed.parse_per_test_durations")
|
||||
@patch("devx.tools.check_test_speed.check_per_test_speed")
|
||||
def test_per_test_violation_exits(
|
||||
self,
|
||||
mock_check_per: MagicMock,
|
||||
mock_parse_per: MagicMock,
|
||||
mock_check: MagicMock,
|
||||
mock_parse: MagicMock,
|
||||
mock_run: MagicMock,
|
||||
) -> None:
|
||||
mock_run.return_value = ("out\n", "err\n")
|
||||
mock_parse.return_value = 3.0
|
||||
mock_parse_per.return_value = [("test_slow", 0.8)]
|
||||
mock_check_per.return_value = ["Test 'test_slow' took 0.80s (limit: 0.5s)."]
|
||||
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(cli, [])
|
||||
assert result.exit_code == 1
|
||||
assert "Per-test speed check FAILED" in result.output
|
||||
assert "test_slow" in result.output
|
||||
|
||||
@patch("devx.tools.check_test_speed.run_tests")
|
||||
def test_parse_failure_exits(
|
||||
self,
|
||||
@@ -125,16 +228,67 @@ class TestMain:
|
||||
@patch("devx.tools.check_test_speed.run_tests")
|
||||
@patch("devx.tools.check_test_speed.parse_duration")
|
||||
@patch("devx.tools.check_test_speed.check_speed")
|
||||
@patch("devx.tools.check_test_speed.parse_per_test_durations")
|
||||
@patch("devx.tools.check_test_speed.check_per_test_speed")
|
||||
def test_custom_max_seconds(
|
||||
self,
|
||||
mock_check_per: MagicMock,
|
||||
mock_parse_per: MagicMock,
|
||||
mock_check: MagicMock,
|
||||
mock_parse: MagicMock,
|
||||
mock_run: MagicMock,
|
||||
) -> None:
|
||||
mock_run.return_value = ("out\n", "err\n")
|
||||
mock_parse.return_value = 0.5
|
||||
mock_parse_per.return_value = []
|
||||
mock_check_per.return_value = []
|
||||
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(cli, ["--max-seconds", "1.5"])
|
||||
assert result.exit_code == 0
|
||||
mock_check.assert_called_once_with(0.5, 1.5)
|
||||
|
||||
@patch("devx.tools.check_test_speed.run_tests")
|
||||
@patch("devx.tools.check_test_speed.parse_duration")
|
||||
@patch("devx.tools.check_test_speed.check_speed")
|
||||
@patch("devx.tools.check_test_speed.parse_per_test_durations")
|
||||
@patch("devx.tools.check_test_speed.check_per_test_speed")
|
||||
def test_disable_per_test_check(
|
||||
self,
|
||||
mock_check_per: MagicMock,
|
||||
mock_parse_per: MagicMock,
|
||||
mock_check: MagicMock,
|
||||
mock_parse: MagicMock,
|
||||
mock_run: MagicMock,
|
||||
) -> None:
|
||||
mock_run.return_value = ("out\n", "err\n")
|
||||
mock_parse.return_value = 1.0
|
||||
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(cli, ["--max-single-seconds", "0"])
|
||||
assert result.exit_code == 0
|
||||
mock_parse_per.assert_not_called()
|
||||
mock_check_per.assert_not_called()
|
||||
|
||||
@patch("devx.tools.check_test_speed.run_tests")
|
||||
@patch("devx.tools.check_test_speed.parse_duration")
|
||||
@patch("devx.tools.check_test_speed.check_speed")
|
||||
@patch("devx.tools.check_test_speed.parse_per_test_durations")
|
||||
@patch("devx.tools.check_test_speed.check_per_test_speed")
|
||||
def test_custom_max_single_seconds(
|
||||
self,
|
||||
mock_check_per: MagicMock,
|
||||
mock_parse_per: MagicMock,
|
||||
mock_check: MagicMock,
|
||||
mock_parse: MagicMock,
|
||||
mock_run: MagicMock,
|
||||
) -> None:
|
||||
mock_run.return_value = ("out\n", "err\n")
|
||||
mock_parse.return_value = 1.0
|
||||
mock_parse_per.return_value = []
|
||||
mock_check_per.return_value = []
|
||||
|
||||
runner = CliRunner()
|
||||
result = runner.invoke(cli, ["--max-single-seconds", "1.0"])
|
||||
assert result.exit_code == 0
|
||||
mock_check_per.assert_called_once_with([], 1.0)
|
||||
|
||||
@@ -131,6 +131,7 @@ class TestCli:
|
||||
clear=True,
|
||||
),
|
||||
patch("devx.ci.integration_guard.POLL_INTERVAL", 0.01),
|
||||
patch("devx.molecule.molecule_ci_guard.POLL_INTERVAL", 0.01),
|
||||
patch("devx.ci.integration_guard.subprocess.Popen") as mock_popen,
|
||||
patch("devx.molecule.molecule_ci_guard.get_running_jobs", side_effect=get_jobs_side_effect),
|
||||
patch("os.killpg") as mock_killpg,
|
||||
@@ -177,6 +178,7 @@ class TestCli:
|
||||
clear=True,
|
||||
),
|
||||
patch("devx.ci.integration_guard.POLL_INTERVAL", 0.01),
|
||||
patch("devx.molecule.molecule_ci_guard.POLL_INTERVAL", 0.01),
|
||||
patch("devx.ci.integration_guard.subprocess.Popen") as mock_popen,
|
||||
patch("devx.molecule.molecule_ci_guard.get_running_jobs", side_effect=get_jobs_side_effect),
|
||||
patch("os.killpg", side_effect=ProcessLookupError("no such process")),
|
||||
@@ -221,6 +223,7 @@ class TestCli:
|
||||
clear=True,
|
||||
),
|
||||
patch("devx.ci.integration_guard.POLL_INTERVAL", 0.01),
|
||||
patch("devx.molecule.molecule_ci_guard.POLL_INTERVAL", 0.01),
|
||||
patch("devx.ci.integration_guard.subprocess.Popen") as mock_popen,
|
||||
patch("devx.molecule.molecule_ci_guard.get_running_jobs", side_effect=get_jobs_side_effect),
|
||||
patch("os.killpg") as mock_killpg,
|
||||
|
||||
Reference in New Issue
Block a user