Skip to content

chore: Move cli tests into correct directory#1406

Merged
abbiemery merged 2 commits intomainfrom
move-cli-test
Feb 26, 2026
Merged

chore: Move cli tests into correct directory#1406
abbiemery merged 2 commits intomainfrom
move-cli-test

Conversation

@abbiemery
Copy link
Contributor

The cli tests were in the root unit test dir, when the cli now has its own module. This makes it match the src layout.

@abbiemery abbiemery requested a review from a team as a code owner February 19, 2026 15:18
@abbiemery abbiemery changed the title Move cli tests into correct directory chore: Move cli tests into correct directory Feb 19, 2026
@abbiemery
Copy link
Contributor Author

Not the quick win I envisioned. The fixture for reloading opentelemetry tracing is causing havok.

@tpoliaw
Copy link
Contributor

tpoliaw commented Feb 26, 2026

The import reloading is breaking the setup_tracing function from observability utils (OU).

  • OU imports ProxyTracerProvider
  • OU.setup_tracing compares the global tracer_provider to ProxyTracerProvider and does nothing if it isn't one
  • The exporter fixture in conftest.py assumes setup_tracing did something

When the trace module is reloaded, the imported ProxyTracerProvider is not reloaded in OU so the isinstance always returns False.

The same effect can be seen in a repl

>>> from pathlib import Path
>>> import pathlib, importlib
>>> importlib.reload(pathlib)
<module 'pathlib' from '/home/qan22331/.local/share/uv/python/cpython-3.14.0-linux-x86_64-gnu/lib/python3.14/pathlib/__init__.py'>
>>> isinstance(pathlib.Path('/tmp'), Path)
False

I not sure the reload_opentelemetry_trace_after_tests fixture ever worked and the fact these tests ever pass is mainly down to luck - even on main they fail if you run them in the wrong order

$ uv run pytest tests/unit_tests/test_cli.py tests/unit_tests/client/test_client.py

Removing the fixture and autouseing the exporter fixture lets the tests pass both before and after moving the cli module.

@codecov
Copy link

codecov bot commented Feb 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.17%. Comparing base (a438d99) to head (683e8b9).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1406   +/-   ##
=======================================
  Coverage   95.17%   95.17%           
=======================================
  Files          43       43           
  Lines        3111     3111           
=======================================
  Hits         2961     2961           
  Misses        150      150           

☔ View full report in Codecov by Sentry.
📢 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.

@tpoliaw tpoliaw requested a review from keithralphs February 26, 2026 11:45
@abbiemery abbiemery merged commit fb0258b into main Feb 26, 2026
18 checks passed
@abbiemery abbiemery deleted the move-cli-test branch February 26, 2026 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants