From 2ea1935287f679fe799f13d2181950d1bdc3a5d3 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Sat, 26 Sep 2026 22:15:04 -0400 Subject: [PATCH 01/13] Add code duplication analysis to the dev workflow Add .jscpd.json (threshold 4%, matching the current measured baseline in test/) and a tox env backed by tools/check-duplication.sh, so duplication checking becomes a standing part of `tox`. CHANGELOG.md is excluded from scanning since its duplication is auto-generated release-note boilerplate (intuit/auto), not code to refactor. Co-Authored-By: Claude Sonnet 5 --- .gitignore | 1 + .jscpd.json | 26 ++++++++++++++++++++++++++ tools/check-duplication.sh | 17 +++++++++++++++++ tox.ini | 10 +++++++++- 4 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 .jscpd.json create mode 100755 tools/check-duplication.sh diff --git a/.gitignore b/.gitignore index 670257ea..995028cd 100644 --- a/.gitignore +++ b/.gitignore @@ -10,6 +10,7 @@ build/ dist/ venvs/ .DS_Store +.tmp/ # Produced by versioningit src/con_duct/_version.py diff --git a/.jscpd.json b/.jscpd.json new file mode 100644 index 00000000..20cc386d --- /dev/null +++ b/.jscpd.json @@ -0,0 +1,26 @@ +{ + "threshold": 4, + "reporters": ["console"], + "ignore": [ + "**/node_modules/**", + "**/.git/**", + "**/.tox/**", + "**/.venv/**", + "**/venvs/**", + "**/__pycache__/**", + "**/*.egg-info/**", + "**/.eggs/**", + "**/.cache/**", + "**/.npm/**", + "**/.idea/**", + "**/.mypy_cache/**", + "**/.ruff_cache/**", + "**/.pytest_cache/**", + "**/.tmp/**", + "**/dist/**", + "**/build/**", + "**/CHANGELOG.md" + ], + "minTokens": 50, + "minLines": 5 +} diff --git a/tools/check-duplication.sh b/tools/check-duplication.sh new file mode 100755 index 00000000..2c2da5ae --- /dev/null +++ b/tools/check-duplication.sh @@ -0,0 +1,17 @@ +#!/bin/bash +# Code duplication detection. +# Run via: tox -e duplication +# Requires: npx (Node.js) + +set -eu + +if ! command -v npx >/dev/null 2>&1; then + echo "ERROR: npx not found. Install Node.js to run duplication checks." + exit 1 +fi + +echo "=== Code duplication check ===" +echo + +# jscpd reads .jscpd.json for config (threshold, ignores, etc.) +npx --yes jscpd@latest . diff --git a/tox.ini b/tox.ini index e6710d69..6aedbc15 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = lint,typing,py310,py311,py312,py313,py314,pypy3,minimal +envlist = lint,typing,duplication,py310,py311,py312,py313,py314,pypy3,minimal skip_missing_interpreters = True isolated_build = True minversion = 3.3.0 @@ -32,6 +32,14 @@ deps = commands = mypy src test +[testenv:duplication] +description = Detect code duplication +skip_install = true +deps = +allowlist_externals = bash +commands = + bash {toxinidir}/tools/check-duplication.sh + [testenv:minimal] description = Test with minimal dependencies (no optional extras) deps = From 4564c549b55e82d8b6b0ac7dc6bfbc2cae60d71d Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Sat, 26 Sep 2026 22:15:11 -0400 Subject: [PATCH 02/13] Run duplication check in CI Add the `duplication` toxenv to the existing include-matrix job in test.yaml, alongside lint/typing/minimal. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/test.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 3bd538d9..5fc501f9 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -43,6 +43,9 @@ jobs: - python-version: '3.10' toxenv: typing os: ubuntu-latest + - python-version: '3.10' + toxenv: duplication + os: ubuntu-latest - python-version: '3.10' toxenv: minimal os: ubuntu-latest From d8d67c810bb3e1ce7013ba3d3d9ac667d82e2df5 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Sat, 26 Sep 2026 22:17:44 -0400 Subject: [PATCH 03/13] Deduplicate test_prepare_outputs.py by parametrizing over Outputs Replace 16 near-identical capture/output combination tests with a single test parametrized over itertools.product(Outputs, Outputs), using Outputs.has_stdout/has_stderr to compute expected behavior per case. Removes the jscpd-flagged duplication in this file. Co-Authored-By: Claude Sonnet 5 --- test/duct_main/test_prepare_outputs.py | 373 +++---------------------- 1 file changed, 38 insertions(+), 335 deletions(-) diff --git a/test/duct_main/test_prepare_outputs.py b/test/duct_main/test_prepare_outputs.py index 9a799ca1..90e64537 100644 --- a/test/duct_main/test_prepare_outputs.py +++ b/test/duct_main/test_prepare_outputs.py @@ -1,360 +1,63 @@ from __future__ import annotations +import itertools import subprocess +from typing import Callable from unittest.mock import MagicMock, call, patch +import pytest from utils import MockStream from con_duct._models import LogPaths, Outputs from con_duct._output import prepare_outputs - -@patch("builtins.open", new_callable=MagicMock) -@patch("con_duct._output.TailPipe") -@patch("con_duct._output.LogPaths") -@patch("con_duct._output.sys.stderr", new_callable=MockStream) -@patch("con_duct._output.sys.stdout", new_callable=MockStream) -def test_prepare_outputs_capture_none_output_none( - _mock_stdout: MockStream, - _mock_stderr: MockStream, - mock_LogPaths: LogPaths, - mock_tee_stream: MagicMock, - mock_open: MagicMock, -) -> None: - mock_log_paths = mock_LogPaths.create("mock_prefix") - mock_tee_stream.return_value.start = MagicMock() - stdout, stderr = prepare_outputs(Outputs.NONE, Outputs.NONE, mock_log_paths) - mock_tee_stream.assert_not_called() - mock_open.assert_not_called() - assert stdout == subprocess.DEVNULL - assert stderr == subprocess.DEVNULL - - -@patch("builtins.open", new_callable=MagicMock) -@patch("con_duct._output.TailPipe") -@patch("con_duct._output.LogPaths") -@patch("con_duct._output.sys.stderr", new_callable=MockStream) -@patch("con_duct._output.sys.stdout", new_callable=MockStream) -def test_prepare_outputs_capture_none_output_stdout( - _mock_stdout: MockStream, - _mock_stderr: MockStream, - mock_LogPaths: LogPaths, - mock_tee_stream: MagicMock, - mock_open: MagicMock, -) -> None: - mock_log_paths = mock_LogPaths.create("mock_prefix") - mock_tee_stream.return_value.start = MagicMock() - stdout, stderr = prepare_outputs(Outputs.NONE, Outputs.STDOUT, mock_log_paths) - mock_tee_stream.assert_not_called() - mock_open.assert_not_called() - assert stdout is None - assert stderr == subprocess.DEVNULL +_STREAMS: list[tuple[str, Callable[[Outputs], bool]]] = [ + ("stdout", Outputs.has_stdout), + ("stderr", Outputs.has_stderr), +] +@pytest.mark.parametrize( + "capture_outputs,outputs", list(itertools.product(Outputs, Outputs)) +) @patch("builtins.open", new_callable=MagicMock) @patch("con_duct._output.TailPipe") @patch("con_duct._output.LogPaths") @patch("con_duct._output.sys.stderr", new_callable=MockStream) @patch("con_duct._output.sys.stdout", new_callable=MockStream) -def test_prepare_outputs_capture_none_output_stderr( - _mock_stdout: MockStream, - _mock_stderr: MockStream, - mock_LogPaths: LogPaths, - mock_tee_stream: MagicMock, - mock_open: MagicMock, -) -> None: - mock_log_paths = mock_LogPaths.create("mock_prefix") - mock_tee_stream.return_value.start = MagicMock() - stdout, stderr = prepare_outputs(Outputs.NONE, Outputs.STDERR, mock_log_paths) - mock_tee_stream.assert_not_called() - mock_open.assert_not_called() - assert stdout == subprocess.DEVNULL - assert stderr is None - - -@patch("builtins.open", new_callable=MagicMock) -@patch("con_duct._output.TailPipe") -@patch("con_duct._output.LogPaths") -@patch("con_duct._output.sys.stderr", new_callable=MockStream) -@patch("con_duct._output.sys.stdout", new_callable=MockStream) -def test_prepare_outputs_capture_none_output_all( - _mock_stdout: MockStream, - _mock_stderr: MockStream, - mock_LogPaths: LogPaths, - mock_tee_stream: MagicMock, - mock_open: MagicMock, -) -> None: - mock_log_paths = mock_LogPaths.create("mock_prefix") - mock_tee_stream.return_value.start = MagicMock() - stdout, stderr = prepare_outputs(Outputs.NONE, Outputs.ALL, mock_log_paths) - mock_tee_stream.assert_not_called() - mock_open.assert_not_called() - assert stdout is None - assert stderr is None - - -@patch("builtins.open", new_callable=MagicMock) -@patch("con_duct._output.TailPipe") -@patch("con_duct._output.LogPaths") -@patch("con_duct._output.sys.stderr", new_callable=MockStream) -@patch("con_duct._output.sys.stdout", new_callable=MockStream) -def test_prepare_outputs_capture_stdout_output_none( - _mock_stdout: MockStream, - _mock_stderr: MockStream, - mock_LogPaths: LogPaths, - mock_tee_stream: MagicMock, - mock_open: MagicMock, -) -> None: - mock_log_paths = mock_LogPaths.create("mock_prefix") - mock_tee_stream.return_value.start = MagicMock() - stdout, stderr = prepare_outputs(Outputs.STDOUT, Outputs.NONE, mock_log_paths) - mock_tee_stream.assert_not_called() - mock_open.assert_called_once_with(mock_log_paths.stdout, "w") - assert stdout == mock_open.return_value - assert stderr == subprocess.DEVNULL - - -@patch("builtins.open", new_callable=MagicMock) -@patch("con_duct._output.TailPipe") -@patch("con_duct._output.LogPaths") -@patch("con_duct._output.sys.stderr", new_callable=MockStream) -@patch("con_duct._output.sys.stdout", new_callable=MockStream) -def test_prepare_outputs_capture_stdout_output_stdout( +def test_prepare_outputs( mock_stdout: MockStream, - _mock_stderr: MockStream, - mock_LogPaths: LogPaths, - mock_tee_stream: MagicMock, - mock_open: MagicMock, -) -> None: - mock_log_paths = mock_LogPaths.create("mock_prefix") - mock_tee_stream.return_value.start = MagicMock() - stdout, stderr = prepare_outputs(Outputs.STDOUT, Outputs.STDOUT, mock_log_paths) - mock_tee_stream.assert_called_once_with( - mock_log_paths.stdout, buffer=mock_stdout.buffer - ) - mock_open.assert_not_called() - assert stdout == mock_tee_stream.return_value - assert stderr == subprocess.DEVNULL - - -@patch("builtins.open", new_callable=MagicMock) -@patch("con_duct._output.TailPipe") -@patch("con_duct._output.LogPaths") -@patch("con_duct._output.sys.stderr", new_callable=MockStream) -@patch("con_duct._output.sys.stdout", new_callable=MockStream) -def test_prepare_outputs_capture_stdout_output_stderr( - _mock_stdout: MockStream, - _mock_stderr: MockStream, - mock_LogPaths: LogPaths, - mock_tee_stream: MagicMock, - mock_open: MagicMock, -) -> None: - mock_log_paths = mock_LogPaths.create("mock_prefix") - mock_tee_stream.return_value.start = MagicMock() - stdout, stderr = prepare_outputs(Outputs.STDOUT, Outputs.STDERR, mock_log_paths) - mock_open.assert_called_once_with(mock_log_paths.stdout, "w") - mock_tee_stream.assert_not_called() - assert stdout == mock_open.return_value - assert stderr is None - - -@patch("builtins.open", new_callable=MagicMock) -@patch("con_duct._output.TailPipe") -@patch("con_duct._output.LogPaths") -@patch("con_duct._output.sys.stderr", new_callable=MockStream) -@patch("con_duct._output.sys.stdout", new_callable=MockStream) -def test_prepare_outputs_capture_stdout_output_all( - mock_stdout: MockStream, - _mock_stderr: MockStream, - mock_LogPaths: LogPaths, - mock_tee_stream: MagicMock, - mock_open: MagicMock, -) -> None: - mock_log_paths = mock_LogPaths.create("mock_prefix") - mock_tee_stream.return_value.start = MagicMock() - stdout, stderr = prepare_outputs(Outputs.STDOUT, Outputs.ALL, mock_log_paths) - mock_tee_stream.assert_called_once_with( - mock_log_paths.stdout, buffer=mock_stdout.buffer - ) - mock_open.assert_not_called() - assert stdout == mock_tee_stream.return_value - assert stderr is None - - -@patch("builtins.open", new_callable=MagicMock) -@patch("con_duct._output.TailPipe") -@patch("con_duct._output.LogPaths") -@patch("con_duct._output.sys.stderr", new_callable=MockStream) -@patch("con_duct._output.sys.stdout", new_callable=MockStream) -def test_prepare_outputs_capture_stderr_output_none( - _mock_stdout: MockStream, - _mock_stderr: MockStream, - mock_LogPaths: LogPaths, - mock_tee_stream: MagicMock, - mock_open: MagicMock, -) -> None: - mock_log_paths = mock_LogPaths.create("mock_prefix") - mock_tee_stream.return_value.start = MagicMock() - stdout, stderr = prepare_outputs(Outputs.STDERR, Outputs.NONE, mock_log_paths) - mock_tee_stream.assert_not_called() - mock_open.assert_called_once_with(mock_log_paths.stderr, "w") - assert stdout == subprocess.DEVNULL - assert stderr == mock_open.return_value - - -@patch("builtins.open", new_callable=MagicMock) -@patch("con_duct._output.TailPipe") -@patch("con_duct._output.LogPaths") -@patch("con_duct._output.sys.stderr", new_callable=MockStream) -@patch("con_duct._output.sys.stdout", new_callable=MockStream) -def test_prepare_outputs_capture_stderr_output_stdout( - _mock_stdout: MockStream, - _mock_stderr: MockStream, - mock_LogPaths: LogPaths, - mock_tee_stream: MagicMock, - mock_open: MagicMock, -) -> None: - mock_log_paths = mock_LogPaths.create("mock_prefix") - mock_tee_stream.return_value.start = MagicMock() - stdout, stderr = prepare_outputs(Outputs.STDERR, Outputs.STDOUT, mock_log_paths) - mock_tee_stream.assert_not_called() - mock_open.assert_called_once_with(mock_log_paths.stderr, "w") - assert stdout is None - assert stderr == mock_open.return_value - - -@patch("builtins.open", new_callable=MagicMock) -@patch("con_duct._output.TailPipe") -@patch("con_duct._output.LogPaths") -@patch("con_duct._output.sys.stderr", new_callable=MockStream) -@patch("con_duct._output.sys.stdout", new_callable=MockStream) -def test_prepare_outputs_capture_stderr_output_stderr( - _mock_stdout: MockStream, - mock_stderr: MockStream, - mock_LogPaths: LogPaths, - mock_tee_stream: MagicMock, - mock_open: MagicMock, -) -> None: - mock_log_paths = mock_LogPaths.create("mock_prefix") - mock_tee_stream.return_value.start = MagicMock() - stdout, stderr = prepare_outputs(Outputs.STDERR, Outputs.STDERR, mock_log_paths) - mock_tee_stream.assert_called_once_with( - mock_log_paths.stderr, buffer=mock_stderr.buffer - ) - mock_open.assert_not_called() - assert stdout == subprocess.DEVNULL - assert stderr == mock_tee_stream.return_value - - -@patch("builtins.open", new_callable=MagicMock) -@patch("con_duct._output.TailPipe") -@patch("con_duct._output.LogPaths") -@patch("con_duct._output.sys.stderr", new_callable=MockStream) -@patch("con_duct._output.sys.stdout", new_callable=MockStream) -def test_prepare_outputs_capture_stderr_output_all( - _mock_stdout: MockStream, mock_stderr: MockStream, mock_LogPaths: LogPaths, mock_tee_stream: MagicMock, mock_open: MagicMock, + capture_outputs: Outputs, + outputs: Outputs, ) -> None: mock_log_paths = mock_LogPaths.create("mock_prefix") mock_tee_stream.return_value.start = MagicMock() - stdout, stderr = prepare_outputs(Outputs.STDERR, Outputs.ALL, mock_log_paths) - mock_tee_stream.assert_called_once_with( - mock_log_paths.stderr, buffer=mock_stderr.buffer - ) - mock_open.assert_not_called() - assert stdout is None - assert stderr == mock_tee_stream.return_value - - -@patch("builtins.open", new_callable=MagicMock) -@patch("con_duct._output.TailPipe") -@patch("con_duct._output.LogPaths") -@patch("con_duct._output.sys.stderr", new_callable=MockStream) -@patch("con_duct._output.sys.stdout", new_callable=MockStream) -def test_prepare_outputs_capture_all_output_none( - _mock_stdout: MockStream, - _mock_stderr: MockStream, - mock_LogPaths: LogPaths, - mock_tee_stream: MagicMock, - mock_open: MagicMock, -) -> None: - mock_log_paths = mock_LogPaths.create("mock_prefix") - mock_tee_stream.return_value.start = MagicMock() - stdout, stderr = prepare_outputs(Outputs.ALL, Outputs.NONE, mock_log_paths) - mock_tee_stream.assert_not_called() - mock_open.assert_has_calls( - [call(mock_log_paths.stdout, "w"), call(mock_log_paths.stderr, "w")] - ) - assert stdout == mock_open.return_value - assert stderr == mock_open.return_value + stdout, stderr = prepare_outputs(capture_outputs, outputs, mock_log_paths) + buffers = {"stdout": mock_stdout.buffer, "stderr": mock_stderr.buffer} + actuals = {"stdout": stdout, "stderr": stderr} + log_paths = {"stdout": mock_log_paths.stdout, "stderr": mock_log_paths.stderr} + expected_open_calls = [] + expected_tee_calls = [] -@patch("builtins.open", new_callable=MagicMock) -@patch("con_duct._output.TailPipe") -@patch("con_duct._output.LogPaths") -@patch("con_duct._output.sys.stderr", new_callable=MockStream) -@patch("con_duct._output.sys.stdout", new_callable=MockStream) -def test_prepare_outputs_capture_all_output_stdout( - mock_stdout: MockStream, - _mock_stderr: MockStream, - mock_LogPaths: LogPaths, - mock_tee_stream: MagicMock, - mock_open: MagicMock, -) -> None: - mock_log_paths = mock_LogPaths.create("mock_prefix") - mock_tee_stream.return_value.start = MagicMock() - stdout, stderr = prepare_outputs(Outputs.ALL, Outputs.STDOUT, mock_log_paths) - mock_tee_stream.assert_called_with(mock_log_paths.stdout, buffer=mock_stdout.buffer) - mock_open.assert_called_once_with(mock_log_paths.stderr, "w") - assert stdout == mock_tee_stream.return_value - assert stderr == mock_open.return_value - + for name, has_stream in _STREAMS: + actual = actuals[name] + if has_stream(capture_outputs): + if has_stream(outputs): + expected_tee_calls.append(call(log_paths[name], buffer=buffers[name])) + assert actual == mock_tee_stream.return_value + else: + expected_open_calls.append(call(log_paths[name], "w")) + assert actual == mock_open.return_value + elif has_stream(outputs): + assert actual is None + else: + assert actual == subprocess.DEVNULL -@patch("builtins.open", new_callable=MagicMock) -@patch("con_duct._output.TailPipe") -@patch("con_duct._output.LogPaths") -@patch("con_duct._output.sys.stderr", new_callable=MockStream) -@patch("con_duct._output.sys.stdout", new_callable=MockStream) -def test_prepare_outputs_capture_all_output_stderr( - _mock_stdout: MockStream, - mock_stderr: MockStream, - mock_LogPaths: LogPaths, - mock_tee_stream: MagicMock, - mock_open: MagicMock, -) -> None: - mock_log_paths = mock_LogPaths.create("mock_prefix") - mock_tee_stream.return_value.start = MagicMock() - stdout, stderr = prepare_outputs(Outputs.ALL, Outputs.STDERR, mock_log_paths) - mock_tee_stream.assert_called_once_with( - mock_log_paths.stderr, buffer=mock_stderr.buffer - ) - mock_open.assert_called_once_with(mock_log_paths.stdout, "w") - assert stdout == mock_open.return_value - assert stderr == mock_tee_stream.return_value - - -@patch("builtins.open", new_callable=MagicMock) -@patch("con_duct._output.TailPipe") -@patch("con_duct._output.LogPaths") -@patch("con_duct._output.sys.stderr", new_callable=MockStream) -@patch("con_duct._output.sys.stdout", new_callable=MockStream) -def test_prepare_outputs_capture_all_output_all( - mock_stdout: MockStream, - mock_stderr: MockStream, - mock_LogPaths: LogPaths, - mock_tee_stream: MagicMock, - mock_open: MagicMock, -) -> None: - mock_log_paths = mock_LogPaths.create("mock_prefix") - mock_tee_stream.return_value.start = MagicMock() - stdout, stderr = prepare_outputs(Outputs.ALL, Outputs.ALL, mock_log_paths) - mock_tee_stream.assert_has_calls( - [ - call(mock_log_paths.stdout, buffer=mock_stdout.buffer), - call(mock_log_paths.stderr, buffer=mock_stderr.buffer), - ], - any_order=True, - ) - assert stdout == mock_tee_stream.return_value - assert stderr == mock_tee_stream.return_value - mock_open.assert_not_called() + if expected_open_calls: + mock_open.assert_has_calls(expected_open_calls, any_order=True) + assert mock_open.call_count == len(expected_open_calls) + if expected_tee_calls: + mock_tee_stream.assert_has_calls(expected_tee_calls, any_order=True) + assert mock_tee_stream.call_count == len(expected_tee_calls) From 9fe5e5b4a889298ebae9398855e95ed60fbccd60 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Sat, 26 Sep 2026 22:20:25 -0400 Subject: [PATCH 04/13] Deduplicate test_aggregation.py by extracting shared Report helpers Extract _make_report, _current_sample, _assert_current_matches_full_run_averages, and _report_with_three_samples to remove the repeated Report-construction and averages-assertion boilerplate jscpd flagged across several test functions. Co-Authored-By: Claude Sonnet 5 --- test/duct_main/test_aggregation.py | 261 +++++++++++------------------ 1 file changed, 98 insertions(+), 163 deletions(-) diff --git a/test/duct_main/test_aggregation.py b/test/duct_main/test_aggregation.py index fb6925ca..90fd9a00 100644 --- a/test/duct_main/test_aggregation.py +++ b/test/duct_main/test_aggregation.py @@ -1,7 +1,6 @@ from collections import Counter from copy import deepcopy import os -from typing import cast from unittest import mock import pytest from con_duct._duct_main import EXECUTION_SUMMARY_FORMAT @@ -63,29 +62,62 @@ ) +def _make_report(mock_log_paths: mock.MagicMock) -> Report: + mock_log_paths.prefix = "mock_prefix" + return Report( + "_cmd", [], mock_log_paths, EXECUTION_SUMMARY_FORMAT, os.getcwd(), clobber=False + ) + + +def _current_sample(report: Report) -> Sample: + """Narrow report.current_sample for mypy and assert it was populated.""" + assert report.current_sample is not None + return report.current_sample + + +def _assert_current_matches_full_run_averages( + current_sample: Sample, report: Report +) -> None: + assert current_sample.averages.rss == report.full_run_stats.averages.rss + assert current_sample.averages.vsz == report.full_run_stats.averages.vsz + assert current_sample.averages.pmem == report.full_run_stats.averages.pmem + assert current_sample.averages.pcpu == report.full_run_stats.averages.pcpu + + +def _report_with_three_samples( + mock_log_paths: mock.MagicMock, +) -> tuple[Report, Sample, Sample]: + """Build a Report and feed it one sample each of stat0, stat1, stat2.""" + sample0 = Sample() + sample0.add_pid(1, deepcopy(stat0)) + sample1 = Sample() + sample1.add_pid(1, deepcopy(stat1)) + sample2 = Sample() + sample2.add_pid(1, deepcopy(stat2)) + report = _make_report(mock_log_paths) + assert report.current_sample is None + assert report.full_run_stats.averages.num_samples == 0 + report.update_from_sample(sample0) + report.update_from_sample(sample1) + report.update_from_sample(sample2) + return report, _current_sample(report), sample2 + + @mock.patch("con_duct._duct_main.LogPaths") def test_aggregation_num_samples_increment(mock_log_paths: mock.MagicMock) -> None: ex0 = Sample() ex0.add_pid(1, deepcopy(stat1)) - mock_log_paths.prefix = "mock_prefix" - cwd = os.getcwd() - report = Report( - "_cmd", [], mock_log_paths, EXECUTION_SUMMARY_FORMAT, cwd, clobber=False - ) + report = _make_report(mock_log_paths) assert report.current_sample is None assert report.full_run_stats.averages.num_samples == 0 report.update_from_sample(ex0) - report.current_sample = cast( - Sample, report.current_sample - ) # So mypy is convcinced it is not None - assert report.current_sample is not None - assert report.current_sample.averages.num_samples == 1 + assert _current_sample(report).averages.num_samples == 1 assert report.full_run_stats.averages.num_samples == 1 report.update_from_sample(ex0) - assert report.current_sample.averages.num_samples == 2 + assert _current_sample(report).averages.num_samples == 2 assert report.full_run_stats.averages.num_samples == 2 report.update_from_sample(ex0) - assert report.current_sample.averages.num_samples == 3 + assert _current_sample(report).averages.num_samples == 3 assert report.full_run_stats.averages.num_samples == 3 @@ -95,34 +127,27 @@ def test_aggregation_single_sample_sanity(mock_log_paths: mock.MagicMock) -> Non ex0.add_pid(0, deepcopy(stat0)) ex0.add_pid(1, deepcopy(stat1)) ex0.add_pid(2, deepcopy(stat2)) - mock_log_paths.prefix = "mock_prefix" - cwd = os.getcwd() - report = Report( - "_cmd", [], mock_log_paths, EXECUTION_SUMMARY_FORMAT, cwd, clobber=False - ) + report = _make_report(mock_log_paths) assert report.current_sample is None assert report.full_run_stats.averages.num_samples == 0 report.update_from_sample(ex0) # 3 pids in a single sample should still be "1" sample - report.current_sample = cast( - Sample, report.current_sample - ) # So mypy is convcinced it is not None - assert report.current_sample is not None + current_sample = _current_sample(report) assert report.full_run_stats is not None - assert report.current_sample.averages.num_samples == 1 + assert current_sample.averages.num_samples == 1 assert report.full_run_stats.averages.num_samples == 1 # assert totals sanity - assert report.current_sample.total_rss == stat0.rss + stat1.rss + stat2.rss - assert report.current_sample.total_vsz == stat0.vsz + stat1.vsz + stat2.vsz - assert report.current_sample.total_pmem == stat0.pmem + stat1.pmem + stat2.pmem - assert report.current_sample.total_pcpu == stat0.pcpu + stat1.pcpu + stat2.pcpu + assert current_sample.total_rss == stat0.rss + stat1.rss + stat2.rss + assert current_sample.total_vsz == stat0.vsz + stat1.vsz + stat2.vsz + assert current_sample.total_pmem == stat0.pmem + stat1.pmem + stat2.pmem + assert current_sample.total_pcpu == stat0.pcpu + stat1.pcpu + stat2.pcpu # With one sample averages should be equal to totals - assert report.current_sample.averages.rss == report.current_sample.averages.rss - assert report.current_sample.averages.vsz == report.current_sample.averages.vsz - assert report.current_sample.averages.pmem == report.current_sample.averages.pmem - assert report.current_sample.averages.pcpu == report.current_sample.averages.pcpu + assert current_sample.averages.rss == current_sample.averages.rss + assert current_sample.averages.vsz == current_sample.averages.vsz + assert current_sample.averages.pmem == current_sample.averages.pmem + assert current_sample.averages.pcpu == current_sample.averages.pcpu @pytest.mark.parametrize("stat", [stat0, stat1, stat2, stat_big]) @@ -132,114 +157,64 @@ def test_aggregation_single_stat_multiple_samples_sanity( ) -> None: ex0 = Sample() ex0.add_pid(1, deepcopy(stat)) - mock_log_paths.prefix = "mock_prefix" - cwd = os.getcwd() - report = Report( - "_cmd", [], mock_log_paths, EXECUTION_SUMMARY_FORMAT, cwd, clobber=False - ) + report = _make_report(mock_log_paths) assert report.current_sample is None assert report.full_run_stats.averages.num_samples == 0 report.update_from_sample(ex0) report.update_from_sample(ex0) report.update_from_sample(ex0) - report.current_sample = cast( - Sample, report.current_sample - ) # So mypy is convcinced it is not None - assert report.current_sample is not None - assert report.current_sample.averages.num_samples == 3 + current_sample = _current_sample(report) + assert current_sample.averages.num_samples == 3 assert report.full_run_stats.averages.num_samples == 3 # With 3 identical samples, totals should be identical to 1 sample - assert report.current_sample.total_rss == stat.rss - assert report.current_sample.total_vsz == stat.vsz - assert report.current_sample.total_pmem == stat.pmem - assert report.current_sample.total_pcpu == stat.pcpu + assert current_sample.total_rss == stat.rss + assert current_sample.total_vsz == stat.vsz + assert current_sample.total_pmem == stat.pmem + assert current_sample.total_pcpu == stat.pcpu # Without resetting the current sample, full_run_stats should equal current_sample - assert report.full_run_stats.total_rss == report.current_sample.total_rss - assert report.full_run_stats.total_vsz == report.current_sample.total_vsz - assert report.full_run_stats.total_pmem == report.current_sample.total_pmem - assert report.full_run_stats.total_pcpu == report.current_sample.total_pcpu + assert report.full_run_stats.total_rss == current_sample.total_rss + assert report.full_run_stats.total_vsz == current_sample.total_vsz + assert report.full_run_stats.total_pmem == current_sample.total_pmem + assert report.full_run_stats.total_pcpu == current_sample.total_pcpu # Averages too - assert report.current_sample.averages.rss == report.full_run_stats.averages.rss - assert report.current_sample.averages.vsz == report.full_run_stats.averages.vsz - assert report.current_sample.averages.pmem == report.full_run_stats.averages.pmem - assert report.current_sample.averages.pcpu == report.full_run_stats.averages.pcpu + _assert_current_matches_full_run_averages(current_sample, report) # With 3 identical samples, averages should be identical to 1 sample - assert report.current_sample.averages.rss == report.current_sample.total_rss - assert report.current_sample.averages.vsz == report.current_sample.total_vsz - assert report.current_sample.averages.pmem == report.current_sample.total_pmem - assert report.current_sample.averages.pcpu == report.current_sample.total_pcpu + assert current_sample.averages.rss == current_sample.total_rss + assert current_sample.averages.vsz == current_sample.total_vsz + assert current_sample.averages.pmem == current_sample.total_pmem + assert current_sample.averages.pcpu == current_sample.total_pcpu @mock.patch("con_duct._duct_main.LogPaths") def test_aggregation_averages(mock_log_paths: mock.MagicMock) -> None: - sample0 = Sample() - sample0.add_pid(1, deepcopy(stat0)) - sample1 = Sample() - sample1.add_pid(1, deepcopy(stat1)) - sample2 = Sample() - sample2.add_pid(1, deepcopy(stat2)) - mock_log_paths.prefix = "mock_prefix" - cwd = os.getcwd() - report = Report( - "_cmd", [], mock_log_paths, EXECUTION_SUMMARY_FORMAT, cwd, clobber=False - ) - assert report.current_sample is None - assert report.full_run_stats.averages.num_samples == 0 - report.update_from_sample(sample0) - report.update_from_sample(sample1) - report.update_from_sample(sample2) - report.current_sample = cast( - Sample, report.current_sample - ) # So mypy is convcinced it is not None - assert report.current_sample is not None - assert report.current_sample.averages.num_samples == 3 + report, current_sample, sample2 = _report_with_three_samples(mock_log_paths) + assert current_sample.averages.num_samples == 3 assert report.full_run_stats.averages.num_samples == 3 # Assert that average calculation works as expected - assert ( - report.current_sample.averages.rss == (stat0.rss + stat1.rss + stat2.rss) / 3.0 - ) - assert ( - report.current_sample.averages.vsz == (stat0.vsz + stat1.vsz + stat2.vsz) / 3.0 - ) - assert ( - report.current_sample.averages.pmem - == (stat0.pmem + stat1.pmem + stat2.pmem) / 3.0 - ) - assert ( - report.current_sample.averages.pcpu - == (stat0.pcpu + stat1.pcpu + stat2.pcpu) / 3.0 - ) + assert current_sample.averages.rss == (stat0.rss + stat1.rss + stat2.rss) / 3.0 + assert current_sample.averages.vsz == (stat0.vsz + stat1.vsz + stat2.vsz) / 3.0 + assert current_sample.averages.pmem == (stat0.pmem + stat1.pmem + stat2.pmem) / 3.0 + assert current_sample.averages.pcpu == (stat0.pcpu + stat1.pcpu + stat2.pcpu) / 3.0 # And full_run_stats.averages is still identical - assert report.current_sample.averages.rss == report.full_run_stats.averages.rss - assert report.current_sample.averages.vsz == report.full_run_stats.averages.vsz - assert report.current_sample.averages.pmem == report.full_run_stats.averages.pmem - assert report.current_sample.averages.pcpu == report.full_run_stats.averages.pcpu + _assert_current_matches_full_run_averages(current_sample, report) # Lets make the arithmetic a little less round report.update_from_sample(sample2) report.update_from_sample(sample2) report.update_from_sample(sample2) - assert report.current_sample.averages.num_samples == 6 + assert current_sample.averages.num_samples == 6 assert report.full_run_stats.averages.num_samples == 6 + assert current_sample.averages.rss == (stat0.rss + stat1.rss + stat2.rss * 4) / 6.0 + assert current_sample.averages.vsz == (stat0.vsz + stat1.vsz + stat2.vsz * 4) / 6.0 assert ( - report.current_sample.averages.rss - == (stat0.rss + stat1.rss + stat2.rss * 4) / 6.0 - ) - assert ( - report.current_sample.averages.vsz - == (stat0.vsz + stat1.vsz + stat2.vsz * 4) / 6.0 - ) - assert ( - report.current_sample.averages.pmem - == (stat0.pmem + stat1.pmem + stat2.pmem * 4) / 6.0 + current_sample.averages.pmem == (stat0.pmem + stat1.pmem + stat2.pmem * 4) / 6.0 ) assert ( - report.current_sample.averages.pcpu - == (stat0.pcpu + stat1.pcpu + stat2.pcpu * 4) / 6.0 + current_sample.averages.pcpu == (stat0.pcpu + stat1.pcpu + stat2.pcpu * 4) / 6.0 ) @@ -247,51 +222,26 @@ def test_aggregation_averages(mock_log_paths: mock.MagicMock) -> None: def test_aggregation_current_ave_diverges_from_total_ave( mock_log_paths: mock.MagicMock, ) -> None: - sample0 = Sample() - sample0.add_pid(1, deepcopy(stat0)) - sample1 = Sample() - sample1.add_pid(1, deepcopy(stat1)) - sample2 = Sample() - sample2.add_pid(1, deepcopy(stat2)) - mock_log_paths.prefix = "mock_prefix" - cwd = os.getcwd() - report = Report( - "_cmd", [], mock_log_paths, EXECUTION_SUMMARY_FORMAT, cwd, clobber=False - ) - assert report.current_sample is None - assert report.full_run_stats.averages.num_samples == 0 - report.update_from_sample(sample0) - report.update_from_sample(sample1) - report.update_from_sample(sample2) - report.current_sample = cast( - Sample, report.current_sample - ) # So mypy is convcinced it is not None - assert report.current_sample is not None - assert report.current_sample.averages.num_samples == 3 + report, current_sample, sample2 = _report_with_three_samples(mock_log_paths) + assert current_sample.averages.num_samples == 3 assert report.full_run_stats.averages.num_samples == 3 # full_run_stats.averages is still identical to current_sample - assert report.current_sample.averages.rss == report.full_run_stats.averages.rss - assert report.current_sample.averages.vsz == report.full_run_stats.averages.vsz - assert report.current_sample.averages.pmem == report.full_run_stats.averages.pmem - assert report.current_sample.averages.pcpu == report.full_run_stats.averages.pcpu + _assert_current_matches_full_run_averages(current_sample, report) # Reset current_sample so averages will diverge from full_run_stats.averages report.current_sample = None report.update_from_sample(sample2) report.update_from_sample(sample2) report.update_from_sample(sample2) - report.current_sample = cast( - Sample, report.current_sample - ) # So mypy is convcinced it is not None - assert report.current_sample is not None - assert report.current_sample.averages.num_samples == 3 + current_sample = _current_sample(report) + assert current_sample.averages.num_samples == 3 assert report.full_run_stats.averages.num_samples == 6 # Current sample should only contain sample2 - assert report.current_sample.averages.rss == sample2.total_rss - assert report.current_sample.averages.vsz == sample2.total_vsz - assert report.current_sample.averages.pmem == sample2.total_pmem - assert report.current_sample.averages.pcpu == sample2.total_pcpu + assert current_sample.averages.rss == sample2.total_rss + assert current_sample.averages.vsz == sample2.total_vsz + assert current_sample.averages.pmem == sample2.total_pmem + assert current_sample.averages.pcpu == sample2.total_pcpu # Full sample average should == (samples_sum/num_samples) assert ( @@ -320,11 +270,7 @@ def test_aggregation_many_samples( sample1 = Sample() pid = 1 sample1.add_pid(pid, deepcopy(stat)) - mock_log_paths.prefix = "mock_prefix" - cwd = os.getcwd() - report = Report( - "_cmd", [], mock_log_paths, EXECUTION_SUMMARY_FORMAT, cwd, clobber=False - ) + report = _make_report(mock_log_paths) assert report.current_sample is None assert report.full_run_stats.averages.num_samples == 0 @@ -332,13 +278,10 @@ def test_aggregation_many_samples( for _ in range(100): report.update_from_sample(sample1) - report.current_sample = cast( - Sample, report.current_sample - ) # So mypy is convcinced it is not None - assert report.current_sample is not None + current_sample = _current_sample(report) # Assert that there is exactly 1 ProcessStat.stat count per update assert ( - sum(report.current_sample.stats[pid].stat.values()) + sum(current_sample.stats[pid].stat.values()) == report.full_run_stats.averages.num_samples == 100 ) @@ -361,11 +304,7 @@ def test_aggregation_many_samples( @mock.patch("con_duct._duct_main.LogPaths") def test_aggregation_sample_no_pids(mock_log_paths: mock.MagicMock) -> None: sample0 = Sample() - mock_log_paths.prefix = "mock_prefix" - cwd = os.getcwd() - report = Report( - "_cmd", [], mock_log_paths, EXECUTION_SUMMARY_FORMAT, cwd, clobber=False - ) + report = _make_report(mock_log_paths) # When there are no pids, finalization should be triggered because the exe is finished, # so a Sample with no PIDs should never be passed to update_from_sample. with pytest.raises(AssertionError): @@ -376,11 +315,7 @@ def test_aggregation_sample_no_pids(mock_log_paths: mock.MagicMock) -> None: def test_aggregation_no_false_peak(mock_log_paths: mock.MagicMock) -> None: sample1 = Sample() sample2 = Sample() - mock_log_paths.prefix = "mock_prefix" - cwd = os.getcwd() - report = Report( - "_cmd", [], mock_log_paths, EXECUTION_SUMMARY_FORMAT, cwd, clobber=False - ) + report = _make_report(mock_log_paths) sample1.add_pid(1, deepcopy(stat100)) sample1.add_pid(2, deepcopy(stat0)) report.update_from_sample(sample1) From be3fe661dcaa1766531595ddb0c0d8556ff0456f Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Sat, 26 Sep 2026 22:21:39 -0400 Subject: [PATCH 05/13] Deduplicate test_report.py by parametrizing/extracting shared setup Merge test_averages_two_samples/test_averages_three_samples into one test parametrized over the number of updates, and extract _make_process_stats to share the ProcessStats construction between the green/red parametrized cases. Co-Authored-By: Claude Sonnet 5 --- test/duct_main/test_report.py | 56 ++++++++++++++--------------------- 1 file changed, 22 insertions(+), 34 deletions(-) diff --git a/test/duct_main/test_report.py b/test/duct_main/test_report.py index 5e08d2d2..83438701 100644 --- a/test/duct_main/test_report.py +++ b/test/duct_main/test_report.py @@ -130,25 +130,16 @@ def test_averages_one_sample() -> None: assert averages.num_samples == 1 -def test_averages_two_samples() -> None: +@pytest.mark.parametrize("num_updates", [1, 2], ids=["two_samples", "three_samples"]) +def test_averages_multiple_samples(num_updates: int) -> None: sample = Sample() sample.add_pid(1, deepcopy(stat0)) averages = Averages.from_sample(sample) sample2 = Sample() sample2.add_pid(2, deepcopy(stat1)) - averages.update(sample2) - assert averages.pcpu == (stat0.pcpu + stat1.pcpu) / 2 - - -def test_averages_three_samples() -> None: - sample = Sample() - sample.add_pid(1, deepcopy(stat0)) - averages = Averages.from_sample(sample) - sample2 = Sample() - sample2.add_pid(2, deepcopy(stat1)) - averages.update(sample2) - averages.update(sample2) - assert averages.pcpu == (stat0.pcpu + (2 * stat1.pcpu)) / 3 + for _ in range(num_updates): + averages.update(sample2) + assert averages.pcpu == (stat0.pcpu + num_updates * stat1.pcpu) / (num_updates + 1) def test_sample_totals() -> None: @@ -161,6 +152,21 @@ def test_sample_totals() -> None: assert sample.total_pcpu == stat2.pcpu * 2 +def _make_process_stats( + pcpu: float, pmem: float, rss: int, vsz: int, etime: str, cmd: str +) -> ProcessStats: + return ProcessStats( + pcpu=pcpu, + pmem=pmem, + rss=rss, + vsz=vsz, + timestamp=datetime.now().astimezone().isoformat(), + etime=etime, + cmd=cmd, + stat=Counter(["stat0"]), + ) + + @pytest.mark.parametrize( "pcpu, pmem, rss, vsz, etime, cmd", [ @@ -176,16 +182,7 @@ def test_process_stats_green( pcpu: float, pmem: float, rss: int, vsz: int, etime: str, cmd: str ) -> None: # Assert does not raise - ProcessStats( - pcpu=pcpu, - pmem=pmem, - rss=rss, - vsz=vsz, - timestamp=datetime.now().astimezone().isoformat(), - etime=etime, - cmd=cmd, - stat=Counter(["stat0"]), - ) + _make_process_stats(pcpu, pmem, rss, vsz, etime, cmd) @pytest.mark.parametrize( @@ -202,16 +199,7 @@ def test_process_stats_red( pcpu: float, pmem: float, rss: int, vsz: int, etime: str, cmd: str ) -> None: with pytest.raises(AssertionError): - ProcessStats( - pcpu=pcpu, - pmem=pmem, - rss=rss, - vsz=vsz, - timestamp=datetime.now().astimezone().isoformat(), - etime=etime, - cmd=cmd, - stat=Counter(["stat0"]), - ) + _make_process_stats(pcpu, pmem, rss, vsz, etime, cmd) @mock.patch("con_duct._tracker.LogPaths") From 1c70821e4bb0de3e4c2d778b8ca831a7b5d53b11 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Sat, 26 Sep 2026 22:22:30 -0400 Subject: [PATCH 06/13] Deduplicate test_ls.py by extracting _extract_prefixes helper Replace six repeated "parse Prefix: lines out of ls() output" blocks with a shared _extract_prefixes staticmethod. Co-Authored-By: Claude Sonnet 5 --- test/test_ls.py | 45 +++++++++++++++------------------------------ 1 file changed, 15 insertions(+), 30 deletions(-) diff --git a/test/test_ls.py b/test/test_ls.py index 73ff8adb..15f9f138 100644 --- a/test/test_ls.py +++ b/test/test_ls.py @@ -326,17 +326,22 @@ def _run_ls( assert exit_code == 0 return buf.getvalue().strip() + @staticmethod + def _extract_prefixes(result: str) -> list[str]: + """Helper function to extract "Prefix:" field values from ls() output.""" + return [ + line.split(":", 1)[1].strip() + for line in result.splitlines() + if line.startswith("Prefix:") + ] + def test_ls_sanity(self) -> None: """Basic sanity test to ensure ls() runs without crashing.""" just_file1 = ["file1_info.json"] result = self._run_ls(just_file1, "summaries") assert "Prefix:" in result - prefixes = [ - line.split(":", 1)[1].strip() - for line in result.splitlines() - if line.startswith("Prefix:") - ] + prefixes = self._extract_prefixes(result) assert len(prefixes) == 1 assert any("file1" in p for p in prefixes) @@ -356,11 +361,7 @@ def test_ls_with_filter(self) -> None: result = self._run_ls(paths, "summaries", args) assert "Prefix:" in result - prefixes = [ - line.split(":", 1)[1].strip() - for line in result.splitlines() - if line.startswith("Prefix:") - ] + prefixes = self._extract_prefixes(result) assert len(prefixes) == 1 assert any("file1" in p for p in prefixes) # filter_this == 'no' @@ -370,11 +371,7 @@ def test_ls_no_pos_args(self) -> None: result = self._run_ls([], "summaries") assert "Prefix:" in result - prefixes = [ - line.split(":", 1)[1].strip() - for line in result.splitlines() - if line.startswith("Prefix:") - ] + prefixes = self._extract_prefixes(result) assert len(prefixes) == 1 assert any("default_logpath" in p for p in prefixes) @@ -389,11 +386,7 @@ def test_ls_multiple_paths(self) -> None: result = self._run_ls(files_1_and_2, "summaries") assert "Prefix:" in result - prefixes = [ - line.split(":", 1)[1].strip() - for line in result.splitlines() - if line.startswith("Prefix:") - ] + prefixes = self._extract_prefixes(result) assert len(prefixes) == 2 assert any("file1" in p for p in prefixes) assert any("file2" in p for p in prefixes) @@ -404,11 +397,7 @@ def test_ls_ignore_old_schema(self) -> None: result = self._run_ls(files_1_2_3, "summaries") assert "Prefix:" in result - prefixes = [ - line.split(":", 1)[1].strip() - for line in result.splitlines() - if line.startswith("Prefix:") - ] + prefixes = self._extract_prefixes(result) assert len(prefixes) == 2 assert any("file1" in p for p in prefixes) assert any("file2" in p for p in prefixes) @@ -421,11 +410,7 @@ def test_ls_ignore_non_infojson(self) -> None: result = self._run_ls(all_files, "summaries") assert "Prefix:" in result - prefixes = [ - line.split(":", 1)[1].strip() - for line in result.splitlines() - if line.startswith("Prefix:") - ] + prefixes = self._extract_prefixes(result) assert len(prefixes) == 2 assert any("file1" in p for p in prefixes) assert any("file2" in p for p in prefixes) From 4f885c0f3c524a0aaaf06a8c3f28d5d04d1540b8 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Sat, 26 Sep 2026 22:24:39 -0400 Subject: [PATCH 07/13] Deduplicate test_plot.py by extracting _plot_args helper Extract a _plot_args staticmethod for the argparse.Namespace (usage.json, min_ratio=3.0, cpu=ps-pcpu) that the plotting tests rebuilt inline: the two output="outfile.png" sanity tests and all the backend-selection tests that plot with no output file, including those added on main by the interactive-backend probing work (#449). Co-Authored-By: Claude Sonnet 5 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01D9HgYZ2Q66KTvBD4cLZeEZ --- test/test_plot.py | 114 ++++++++-------------------------------------- 1 file changed, 19 insertions(+), 95 deletions(-) diff --git a/test/test_plot.py b/test/test_plot.py index 67b1d13e..6241f51c 100644 --- a/test/test_plot.py +++ b/test/test_plot.py @@ -4,7 +4,7 @@ import json import os from pathlib import Path -from typing import Any, List, Tuple +from typing import Any, List, Optional, Tuple from unittest.mock import MagicMock, Mock, call, mock_open, patch import pytest @@ -91,17 +91,21 @@ def test_formatter_output( class TestPlotMatplotlib: - @patch("matplotlib.pyplot.savefig") - def test_matplotlib_plot_sanity(self, mock_plot_save: MagicMock) -> None: - args = argparse.Namespace( + @staticmethod + def _plot_args(output: Optional[str] = None) -> argparse.Namespace: + return argparse.Namespace( command="plot", file_path="test/data/mriqc-example/usage.json", - output="outfile.png", + output=output, func=plot.matplotlib_plot, log_level="INFO", min_ratio=3.0, cpu="ps-pcpu", ) + + @patch("matplotlib.pyplot.savefig") + def test_matplotlib_plot_sanity(self, mock_plot_save: MagicMock) -> None: + args = self._plot_args(output="outfile.png") assert cli.execute(args) == 0 mock_plot_save.assert_called_once_with("outfile.png") @@ -111,15 +115,7 @@ def test_matplotlib_plot_uses_agg_backend_with_output( self, mock_use: MagicMock, mock_plot_save: MagicMock ) -> None: """Test that Agg backend is used when --output is specified.""" - args = argparse.Namespace( - command="plot", - file_path="test/data/mriqc-example/usage.json", - output="outfile.png", - func=plot.matplotlib_plot, - log_level="INFO", - min_ratio=3.0, - cpu="ps-pcpu", - ) + args = self._plot_args(output="outfile.png") assert cli.execute(args) == 0 mock_use.assert_called_once_with("Agg") mock_plot_save.assert_called_once_with("outfile.png") @@ -225,15 +221,7 @@ def test_matplotlib_plot_non_interactive_backend_pinned( overridden.""" monkeypatch.setenv("MPLBACKEND", "Agg") - args = argparse.Namespace( - command="plot", - file_path="test/data/mriqc-example/usage.json", - output=None, # No output file specified - func=plot.matplotlib_plot, - log_level="INFO", - min_ratio=3.0, - cpu="ps-pcpu", - ) + args = self._plot_args() result = cli.execute(args) assert result == 1 assert "MPLBACKEND=Agg" in caplog.text @@ -249,15 +237,7 @@ def test_matplotlib_plot_non_interactive_backend_pinned_with_get_backend( """Same as above, exercising the get_backend() (matplotlib >= 3.10) path.""" monkeypatch.setenv("MPLBACKEND", "Agg") - args = argparse.Namespace( - command="plot", - file_path="test/data/mriqc-example/usage.json", - output=None, # No output file specified - func=plot.matplotlib_plot, - log_level="INFO", - min_ratio=3.0, - cpu="ps-pcpu", - ) + args = self._plot_args() result = cli.execute(args) assert result == 1 @@ -283,15 +263,7 @@ def fake_use(name: str) -> None: if name != "qtagg": raise ImportError(f"No module for {name}") - args = argparse.Namespace( - command="plot", - file_path="test/data/mriqc-example/usage.json", - output=None, - func=plot.matplotlib_plot, - log_level="INFO", - min_ratio=3.0, - cpu="ps-pcpu", - ) + args = self._plot_args() with patch("matplotlib.use", side_effect=fake_use) as mock_use: result = cli.execute(args) assert result == 0 @@ -322,15 +294,7 @@ def fake_use(name: str) -> None: "interactive framework, as 'headless' is currently running" ) - args = argparse.Namespace( - command="plot", - file_path="test/data/mriqc-example/usage.json", - output=None, - func=plot.matplotlib_plot, - log_level="INFO", - min_ratio=3.0, - cpu="ps-pcpu", - ) + args = self._plot_args() with patch("matplotlib.use", side_effect=fake_use) as mock_use: result = cli.execute(args) assert result == 0 @@ -353,15 +317,7 @@ def test_matplotlib_plot_auto_probe_all_fail( traceback.""" monkeypatch.delenv("MPLBACKEND", raising=False) - args = argparse.Namespace( - command="plot", - file_path="test/data/mriqc-example/usage.json", - output=None, - func=plot.matplotlib_plot, - log_level="INFO", - min_ratio=3.0, - cpu="ps-pcpu", - ) + args = self._plot_args() with patch("matplotlib.use", side_effect=ImportError("nope")): result = cli.execute(args) assert result == 1 @@ -381,15 +337,7 @@ def test_matplotlib_plot_interactive_backend_with_get_backend( ) -> None: """Test that plotting without output in interactive backend calls plt.show() successfully.""" - args = argparse.Namespace( - command="plot", - file_path="test/data/mriqc-example/usage.json", - output=None, # No output file specified - func=plot.matplotlib_plot, - log_level="INFO", - min_ratio=3.0, - cpu="ps-pcpu", - ) + args = self._plot_args() result = cli.execute(args) assert result == 0 mock_show.assert_called_once() @@ -411,15 +359,7 @@ def test_matplotlib_plot_backend_fails_to_load( ) -> None: """A backend that reports as interactive but fails to import (e.g. a broken tkinter install) should fail with guidance, not plt.show().""" - args = argparse.Namespace( - command="plot", - file_path="test/data/mriqc-example/usage.json", - output=None, - func=plot.matplotlib_plot, - log_level="INFO", - min_ratio=3.0, - cpu="ps-pcpu", - ) + args = self._plot_args() result = cli.execute(args) assert result == 1 mock_show.assert_not_called() @@ -444,15 +384,7 @@ def test_matplotlib_plot_backend_fails_to_load_non_import_error( """Some backends (e.g. webagg without tornado) raise RuntimeError, not ImportError, when their dependency is missing -- that must be caught with guidance too, not leak as a raw traceback.""" - args = argparse.Namespace( - command="plot", - file_path="test/data/mriqc-example/usage.json", - output=None, - func=plot.matplotlib_plot, - log_level="INFO", - min_ratio=3.0, - cpu="ps-pcpu", - ) + args = self._plot_args() result = cli.execute(args) assert result == 1 mock_show.assert_not_called() @@ -489,15 +421,7 @@ def test_matplotlib_plot_no_backend_registry( if hasattr(matplotlib.backends, "backend_registry"): monkeypatch.delattr(matplotlib.backends, "backend_registry") - args = argparse.Namespace( - command="plot", - file_path="test/data/mriqc-example/usage.json", - output=None, - func=plot.matplotlib_plot, - log_level="INFO", - min_ratio=3.0, - cpu="ps-pcpu", - ) + args = self._plot_args() result = cli.execute(args) assert result == 0 mock_show.assert_called_once() From b8a08906d3663d2a1803f003ef6a8631e15cd6e9 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Sat, 26 Sep 2026 22:25:20 -0400 Subject: [PATCH 08/13] Tighten duplication threshold to 0% after cleanup The mitigation commits above brought the repo to 0% duplication; enforce zero-tolerance going forward. Co-Authored-By: Claude Sonnet 5 --- .jscpd.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.jscpd.json b/.jscpd.json index 20cc386d..19664949 100644 --- a/.jscpd.json +++ b/.jscpd.json @@ -1,5 +1,5 @@ { - "threshold": 4, + "threshold": 0, "reporters": ["console"], "ignore": [ "**/node_modules/**", From fe85dcc707839035415b8c1f8c96544d2443d75f Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Tue, 29 Sep 2026 13:54:54 -0400 Subject: [PATCH 09/13] Address PR review feedback: pin jscpd, refresh stale current_sample - tools/check-duplication.sh: pin jscpd to 5.3.3 instead of @latest, so a new jscpd release can't change parsing/config or fail CI on an otherwise-unchanged commit (Copilot review). - test_aggregation.py: re-fetch current_sample via _current_sample() after the extra update_from_sample() calls in test_aggregation_averages, instead of relying on Sample.aggregate() happening to carry the same Averages object forward across reassignment. Correct either way today, but the previous form relied on an implementation detail a future refactor of aggregate() could break silently (Copilot review). Co-Authored-By: Claude Sonnet 5 --- test/duct_main/test_aggregation.py | 1 + tools/check-duplication.sh | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/test/duct_main/test_aggregation.py b/test/duct_main/test_aggregation.py index 90fd9a00..12847e3f 100644 --- a/test/duct_main/test_aggregation.py +++ b/test/duct_main/test_aggregation.py @@ -206,6 +206,7 @@ def test_aggregation_averages(mock_log_paths: mock.MagicMock) -> None: report.update_from_sample(sample2) report.update_from_sample(sample2) report.update_from_sample(sample2) + current_sample = _current_sample(report) assert current_sample.averages.num_samples == 6 assert report.full_run_stats.averages.num_samples == 6 assert current_sample.averages.rss == (stat0.rss + stat1.rss + stat2.rss * 4) / 6.0 diff --git a/tools/check-duplication.sh b/tools/check-duplication.sh index 2c2da5ae..e67bb1f4 100755 --- a/tools/check-duplication.sh +++ b/tools/check-duplication.sh @@ -5,13 +5,17 @@ set -eu +# Pinned so a new jscpd release can't change behavior or fail CI on an +# otherwise-unchanged commit. Bump deliberately (and re-verify) when needed. +JSCPD_VERSION=5.3.3 + if ! command -v npx >/dev/null 2>&1; then echo "ERROR: npx not found. Install Node.js to run duplication checks." exit 1 fi -echo "=== Code duplication check ===" +echo "=== Code duplication check (jscpd ${JSCPD_VERSION}) ===" echo # jscpd reads .jscpd.json for config (threshold, ignores, etc.) -npx --yes jscpd@latest . +npx --yes "jscpd@${JSCPD_VERSION}" . From 1acfa75fb0884c873bd9113efa913931d92a80b8 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 12:38:26 +0000 Subject: [PATCH 10/13] test_aggregation: assert single-sample averages against totals test_aggregation_single_sample_sanity compared each average to itself (`averages.rss == averages.rss`), so it could never fail -- carried over verbatim from main by the dedup refactor. Compare against the totals as the comment says, sharing an _assert_averages_match_totals() helper with test_aggregation_single_stat_multiple_samples_sanity, which already did the intended check. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01D9HgYZ2Q66KTvBD4cLZeEZ --- test/duct_main/test_aggregation.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/test/duct_main/test_aggregation.py b/test/duct_main/test_aggregation.py index 12847e3f..cf6bef1b 100644 --- a/test/duct_main/test_aggregation.py +++ b/test/duct_main/test_aggregation.py @@ -84,6 +84,13 @@ def _assert_current_matches_full_run_averages( assert current_sample.averages.pcpu == report.full_run_stats.averages.pcpu +def _assert_averages_match_totals(sample: Sample) -> None: + assert sample.averages.rss == sample.total_rss + assert sample.averages.vsz == sample.total_vsz + assert sample.averages.pmem == sample.total_pmem + assert sample.averages.pcpu == sample.total_pcpu + + def _report_with_three_samples( mock_log_paths: mock.MagicMock, ) -> tuple[Report, Sample, Sample]: @@ -144,10 +151,7 @@ def test_aggregation_single_sample_sanity(mock_log_paths: mock.MagicMock) -> Non assert current_sample.total_pcpu == stat0.pcpu + stat1.pcpu + stat2.pcpu # With one sample averages should be equal to totals - assert current_sample.averages.rss == current_sample.averages.rss - assert current_sample.averages.vsz == current_sample.averages.vsz - assert current_sample.averages.pmem == current_sample.averages.pmem - assert current_sample.averages.pcpu == current_sample.averages.pcpu + _assert_averages_match_totals(current_sample) @pytest.mark.parametrize("stat", [stat0, stat1, stat2, stat_big]) @@ -182,10 +186,7 @@ def test_aggregation_single_stat_multiple_samples_sanity( _assert_current_matches_full_run_averages(current_sample, report) # With 3 identical samples, averages should be identical to 1 sample - assert current_sample.averages.rss == current_sample.total_rss - assert current_sample.averages.vsz == current_sample.total_vsz - assert current_sample.averages.pmem == current_sample.total_pmem - assert current_sample.averages.pcpu == current_sample.total_pcpu + _assert_averages_match_totals(current_sample) @mock.patch("con_duct._duct_main.LogPaths") From fdb6a92d7791a144a8b570947c77e5ff8b111f21 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 12:38:26 +0000 Subject: [PATCH 11/13] CI: provision Node.js explicitly for the duplication job jscpd is run via npx, so the job silently depended on whatever Node the runner image happens to ship. Set it up explicitly (jscpd needs >= 18), as Copilot's review overview suggested. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01D9HgYZ2Q66KTvBD4cLZeEZ --- .github/workflows/test.yaml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 5fc501f9..31b59c87 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -66,6 +66,12 @@ jobs: python-version: ${{ matrix.python-version }} allow-prereleases: true + - name: Set up Node.js (for jscpd) + if: matrix.toxenv == 'duplication' + uses: actions/setup-node@v4 + with: + node-version: '22' + - name: Install dependencies run: | python -m pip install --upgrade pip wheel From 5e520a76e6b5e078f016a9da9d469fea00d385b7 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 12:52:26 +0000 Subject: [PATCH 12/13] test_prepare_outputs: don't derive expectations from the code under test The parametrized test computed what to expect via Outputs.has_stdout / has_stderr -- the same predicates prepare_outputs branches on -- so a bug in them moved expected and actual together and nothing failed (e.g. has_stdout() true for every value but NONE passed the whole suite; the 16 original hard-coded tests caught it in 7 cases). Spell out which Outputs enable each stream instead. Also assert TailPipe.start() is called once per tee'd stream: dropping stdout.start() went unnoticed both before and after the refactor. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01D9HgYZ2Q66KTvBD4cLZeEZ --- test/duct_main/test_prepare_outputs.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/test/duct_main/test_prepare_outputs.py b/test/duct_main/test_prepare_outputs.py index 90e64537..69296d79 100644 --- a/test/duct_main/test_prepare_outputs.py +++ b/test/duct_main/test_prepare_outputs.py @@ -1,16 +1,18 @@ from __future__ import annotations import itertools import subprocess -from typing import Callable from unittest.mock import MagicMock, call, patch import pytest from utils import MockStream from con_duct._models import LogPaths, Outputs from con_duct._output import prepare_outputs -_STREAMS: list[tuple[str, Callable[[Outputs], bool]]] = [ - ("stdout", Outputs.has_stdout), - ("stderr", Outputs.has_stderr), +# Spelled out rather than derived from Outputs.has_stdout/has_stderr, which +# prepare_outputs itself branches on -- a bug there must not shift the +# expectations along with the behavior. +_STREAMS: list[tuple[str, frozenset[Outputs]]] = [ + ("stdout", frozenset({Outputs.ALL, Outputs.STDOUT})), + ("stderr", frozenset({Outputs.ALL, Outputs.STDERR})), ] @@ -41,16 +43,16 @@ def test_prepare_outputs( expected_open_calls = [] expected_tee_calls = [] - for name, has_stream in _STREAMS: + for name, enabled_for in _STREAMS: actual = actuals[name] - if has_stream(capture_outputs): - if has_stream(outputs): + if capture_outputs in enabled_for: + if outputs in enabled_for: expected_tee_calls.append(call(log_paths[name], buffer=buffers[name])) assert actual == mock_tee_stream.return_value else: expected_open_calls.append(call(log_paths[name], "w")) assert actual == mock_open.return_value - elif has_stream(outputs): + elif outputs in enabled_for: assert actual is None else: assert actual == subprocess.DEVNULL @@ -61,3 +63,4 @@ def test_prepare_outputs( if expected_tee_calls: mock_tee_stream.assert_has_calls(expected_tee_calls, any_order=True) assert mock_tee_stream.call_count == len(expected_tee_calls) + assert mock_tee_stream.return_value.start.call_count == len(expected_tee_calls) From edba47e00099114a2cc7ba2aebafbbfdf47880e9 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 2 Oct 2026 13:08:35 +0000 Subject: [PATCH 13/13] test_aggregation: cover ProcessStats.aggregate's "[cmd]" replacement The branch that swaps a kernel-abbreviated "[name]" cmd for the full command line had no dedicated test; it was hit only when a live process sampled during e2e runs happened to show bracketed, so codecov/project flapped by ~0.2% between otherwise identical runs. Test it directly. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01D9HgYZ2Q66KTvBD4cLZeEZ --- test/duct_main/test_aggregation.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/duct_main/test_aggregation.py b/test/duct_main/test_aggregation.py index cf6bef1b..c386794a 100644 --- a/test/duct_main/test_aggregation.py +++ b/test/duct_main/test_aggregation.py @@ -326,3 +326,23 @@ def test_aggregation_no_false_peak(mock_log_paths: mock.MagicMock) -> None: report.update_from_sample(sample2) assert report.current_sample is not None assert report.current_sample.total_pcpu == 100 + + +@pytest.mark.parametrize( + "first_cmd,second_cmd,expected_cmd", + [ + # Kernel-abbreviated "[name]" is replaced by the full command line + ("[python]", "python script.py", "python script.py"), + # Otherwise the first-seen command line is kept + ("python script.py", "[python]", "python script.py"), + ("cmd 1", "cmd 2", "cmd 1"), + ], +) +def test_aggregation_cmd_change( + first_cmd: str, second_cmd: str, expected_cmd: str +) -> None: + first = deepcopy(stat1) + first.cmd = first_cmd + second = deepcopy(stat2) + second.cmd = second_cmd + assert first.aggregate(second).cmd == expected_cmd