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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI: MacOS SAMP tests hang indefinitely

2 participants