Skip to content

TST: add hub-free unit tests for pyvo.samp.client - #779

Open
SebastianFoss wants to merge 1 commit into
astropy:mainfrom
SebastianFoss:tst-samp-client-coverage
Open

TST: add hub-free unit tests for pyvo.samp.client#779
SebastianFoss wants to merge 1 commit into
astropy:mainfrom
SebastianFoss:tst-samp-client-coverage

Conversation

@SebastianFoss

Copy link
Copy Markdown

TST: add hub-free unit tests for pyvo.samp.client

Summary

Adds 45 unit tests for pyvo/samp/client.py that require no hub, no sockets, and no time.sleep. All 6 existing tests are unchanged.

Coverage

Run Before After
pytest pyvo/samp/tests/test_client.py 143 miss, 35% 54 miss, 75%
pytest pyvo/samp 94 miss, 57% 53 miss, 76%
pytest pyvo/samp --slow 94 miss, 57% 53 miss, 76%
Raw '--cov-report=term-missing output

================================ tests coverage ================================
_______________ coverage: platform linux, python 3.13.11-final-0 _______________

Name Stmts Miss Cover Missing

pyvo/samp/client.py 220 143 35% 4-58, 76, 79, 102-106, 147, 158, 175-180, 186-193, 197, 201-202, 207, 211-615, 621, 626, 631, 644, 649-653, 664-668, 672, 677, 687, 692-720, 732-742

TOTAL 220 143 35%
========================= 4 passed, 1 skipped in 0.40s =========================

=== BEFORE pytest pyvo/samp ===
$ pytest pyvo/samp
================================ tests coverage ================================
_______________ coverage: platform linux, python 3.13.11-final-0 _______________

Name Stmts Miss Cover Missing

pyvo/samp/client.py 220 94 57% 4-58, 76, 79, 102-106, 147, 158, 175-180, 186-193, 197, 201-202, 211-233, 249, 255, 287, 304, 339, 346, 381-422, 462-464, 505-507, 531-615, 621, 626, 631, 644, 649-653, 664-668, 672, 687, 692-698, 714-720, 732

TOTAL 220 94 57%
======================== 18 passed, 45 skipped in 1.63s ========================

=== BEFORE pytest pyvo/samp --slow ===
$ pytest pyvo/samp --slow
================================ tests coverage ================================
_______________ coverage: platform linux, python 3.13.11-final-0 _______________

Name Stmts Miss Cover Missing

pyvo/samp/client.py 220 94 57% 4-58, 76, 79, 102-106, 147, 158, 175-180, 186-193, 197, 201-202, 211-233, 249, 255, 287, 304, 339, 346, 381-422, 462-464, 505-507, 531-615, 621, 626, 631, 644, 649-653, 664-668, 672, 687, 692-698, 714-720, 732

TOTAL 220 94 57%
======================== 21 passed, 42 skipped in 5.65s ========================

=== AFTER pytest pyvo/samp/tests/test_client.py ===
$ pytest pyvo/samp/tests/test_client.py
================================ tests coverage ================================
_______________ coverage: platform linux, python 3.13.11-final-0 _______________

Name Stmts Miss Cover Missing

pyvo/samp/client.py 220 54 75% 4-58, 102-106, 147, 158, 179-180, 186-187, 193, 197, 201-202, 207, 211, 216, 233, 255, 287, 304, 339, 346, 381, 422, 464, 507, 533, 555, 577, 591, 615, 653, 668, 672, 698, 720, 732

TOTAL 220 54 75%
======================== 50 passed, 1 skipped in 0.73s =========================

=== AFTER pytest pyvo/samp ===
$ pytest pyvo/samp
================================ tests coverage ================================
_______________ coverage: platform linux, python 3.13.11-final-0 _______________

Name Stmts Miss Cover Missing

pyvo/samp/client.py 220 53 76% 4-58, 102-106, 147, 158, 179-180, 186-187, 193, 197, 201-202, 211, 216, 233, 255, 287, 304, 339, 346, 381, 422, 464, 507, 533, 555, 577, 591, 615, 653, 668, 672, 698, 720, 732

TOTAL 220 53 76%
======================== 64 passed, 45 skipped in 1.86s ========================

=== AFTER pytest pyvo/samp --slow ===
$ pytest pyvo/samp --slow
================================ tests coverage ================================
_______________ coverage: platform linux, python 3.13.11-final-0 _______________

Name Stmts Miss Cover Missing

pyvo/samp/client.py 220 53 76% 4-58, 102-106, 147, 158, 179-180, 186-187, 193, 197, 201-202, 211, 216, 233, 255, 287, 304, 339, 346, 381, 422, 464, 507, 533, 555, 577, 591, 615, 653, 668, 672, 698, 720, 732

TOTAL 220 53 76%
======================== 67 passed, 42 skipped in 5.97s ========================'

Note: the 4-58 block in every Missing column is the module-level import and class-header region, executed at import time before pytest-cov begins tracing. Excluding those ~15 statements, executable-logic coverage goes from ~61% to ~81%.

Approach

Rather than adding hub-dependent integration tests, this introduces a StubHubProxy satisfying the small interface SAMPClient actually uses. Covers: registration error branches, bind/unbind and the callable=False rejection paths, the built-in samp.app.ping and client.env.get handlers, notification/call/response dispatch (5- vs 6-arg handler signatures, wildcard MType matching, private-key mismatch), subscription and metadata declaration, and the start/stop lifecycle.

Relation to #731

This does not fix #731. It is relevant to it in two ways:

  • Before this PR, test_client.py alone reached 35% while the full suite reached 57% — the 22-point gap came from the hub-based tests that CI: MacOS SAMP tests hang indefinitely #731 implicates. After this PR that gap is a single statement (line 207). The flaky tests are no longer load-bearing for client.py coverage.
  • The --slow tests add zero client.py coverage in both the before and after measurements.

Two new tests (test_start_and_stop_callable_client, test_stop_raises_if_thread_does_not_terminate) do start a thread, but synchronize via join(timeout) rather than sleep(). Happy to mark them slow if preferred.

Incidental finding

SAMPClient.stop()'s second if self._thread.is_alive() is not guarded by self._callable, so calling stop() on a client constructed with callable=False raises AttributeError. Not patched here — can fix in this PR or file separately.

@codecov

codecov Bot commented Aug 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.41%. Comparing base (79f1d85) to head (63467ca).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #779      +/-   ##
==========================================
+ Coverage   80.52%   81.41%   +0.88%     
==========================================
  Files          90       90              
  Lines       10352    10352              
==========================================
+ Hits         8336     8428      +92     
+ Misses       2016     1924      -92     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
@ManonMarchand

Copy link
Copy Markdown
Member

Thank you for tackling this. I'll check the tests in details later, but for your question about stop() it'd be best in a separate PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment