tests/test_benchmark_pss.py (2)

45-61: Strengthen the regression assertion for the bug being fixed.

The key behavioral guarantee of the fix is that BenchmarkRecord.data_collected flips to True (and max_pss is 0) when pss is absent — that's precisely what was false before the fix and produced NA for every column. Asserting max_rss is not None would also pass if, say, a future refactor collected RSS but still swallowed the AttributeError elsewhere. Consider tightening:

Proposed assertion tightening

     timer._update_record()

-    assert record.max_rss is not None, "max_rss is None — metrics not collected"
-    assert record.max_vms is not None, "max_vms is None"
-    assert record.max_uss is not None, "max_uss is None"
-    assert record.max_rss == pytest.approx(100.0, abs=0.1)
+    assert record.data_collected, "benchmark metrics were not collected"
+    assert record.max_rss == pytest.approx(100.0, abs=0.1)
+    assert record.max_vms == pytest.approx(200.0, abs=0.1)
+    assert record.max_uss == pytest.approx(80.0, abs=0.1)
+    assert record.max_pss == 0, "pss should default to 0 when attribute is missing"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_benchmark_pss.py` around lines 45 - 61, The test
test_update_record_without_pss is too weak: it only asserts max_rss is not None
but should assert that BenchmarkRecord.data_collected becomes True and that
max_pss is set to 0 when pss is absent; update the test to check
record.data_collected is True and record.max_pss == 0 (in addition to existing
RSS/VMS/USS assertions) so the regression that previously left columns as NA is
prevented — locate the assertions in test_update_record_without_pss and add
checks for BenchmarkRecord.data_collected and max_pss.

---

49-54: Consider factoring out the timer construction to reduce duplication.

Both tests build a BenchmarkTimer via __new__ and manually wire the same four attributes. A small fixture/helper (e.g. _make_timer(proc)) would keep the tests focused on the behavioral difference (pss present vs absent) rather than boilerplate. Also, instantiating through __new__ bypasses __init__ — if future construction logic is added (e.g. additional state used by _update_record), these tests will silently go stale. A light alternative is patching psutil.Process and calling the real constructor.

Also applies to: 68-73

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_benchmark_pss.py` around lines 49 - 54, Factor out the repeated
BenchmarkTimer construction into a small helper (e.g. _make_timer(proc)) that
creates a BenchmarkRecord, constructs a BenchmarkTimer via its real constructor
(BenchmarkTimer(proc)) rather than using __new__, and returns the wired timer;
in the tests that currently do manual wiring (creating BenchmarkRecord(),
setting timer.pid, timer.main, timer.bench_record, timer.procs) replace that
block with calls to _make_timer(proc). If calling the real constructor requires
isolating psutil.Process behavior, patch psutil.Process in the test (or use a
fixture) so the real __init__ runs safely; update both test sites (the
pss-present and pss-absent tests) to use the new helper.