Skip to content

Document all hand-written source in tests/opentelemetry-test-utils - #64

Closed
ocelotl wants to merge 18 commits into
docs-basefrom
docs_test_utils
Closed

ocelotl wants to merge 18 commits into
docs-basefrom
docs_test_utils

Conversation

@ocelotl

@ocelotl ocelotl commented Oct 30, 2026 •

Copy link
Copy Markdown
Owner

Documents every hand-written module, class, function and method in opentelemetry-test-utils, so that each one carries a docstring, every parameter is documented, and every callable with a return value has a Returns: section.

No behaviour changes, with one exception explained under "One code change, and why" below.

Scope note

The package sits under the repository's tests/ directory, but it is a published distribution named opentelemetry-test-utils and the code under tests/opentelemetry-test-utils/src/ is its source. That src/ directory is what this change documents. tests/opentelemetry-test-utils/tests/, which holds the package's own tests, is untouched. None of the 15 files is generated.

This package has no page in the rendered API reference, so nothing here changes the published documentation. The audience is whoever reads or extends the test helpers, which is every contributor who writes a test against them.

What changed

15 files under tests/opentelemetry-test-utils/src/opentelemetry/test/, +1952/-44, in 15 commits.

How to review

There is exactly one commit per file, so any single file can be reviewed on its own without reading around it. The order is version and __init__, which are the package entry points, then globals_test, test_base, the HTTP, WSGI and ASGI fixtures, the span, metric and sampler builders, the propagator and entry point doubles, concurrency_test, _otlp_test_server, and finally weaver_live_check.

Conventions followed

docs/conf.py does not set special-members, so an __init__ docstring never renders. Constructor parameters therefore go on the class docstring, and __init__ carries a short docstring saying so. That applies to _AssertNotRaisesMixin._AssertNotRaises, FinishedTestSpans, IterEntryPoint, CapturingSampler, OtlpProtoTestServer, LiveCheckError, LiveCheckReport and WeaverLiveCheck. In the WeaverLiveCheck case the existing __init__ docstring already described four of the ten parameters in prose; that prose moved into the Args: section on the class rather than being duplicated.

Cross-references are written only where the target is actually documented, which is why some names are written as inline literals instead. opentelemetry.metrics.set_meter_provider and the opentelemetry._logs members are undocumented because docs/api/metrics.rst:10 and docs/api/_logs.rst:14 use automodule without :members:. opentelemetry.util._once.Once is undocumented for a different reason: no document under docs/ covers opentelemetry.util at all, as the toctree at docs/api/index.rst:14-25 shows. Inside this package the same rule applies to the three attributes of LiveCheckError and to __enter__ and __exit__, since plain assignments in __init__ get no py:attribute and docs/conf.py:215-221 sets no special-members.

Where a docstring could only repeat the name, it says something else: which argument is ignored, what an empty return means, which call consumes the queue it reads from, and which method does not call its parent.

Documentation that was wrong and is now corrected

48 docstring claims that the code contradicts were corrected, each checked against the line of code that disagrees with it, and most settled by running the code. The three largest:

  • get_sorted_metrics claimed a second read sees only what was recorded in between. The in-memory reader is cumulative, so it does not.
  • The three reset_*_globals functions claimed that replacing a proxy provider detaches tracers, meters and loggers already handed out. A proxy caches the real implementation on first use, so an object handed out earlier keeps pointing at the old one.
  • The TestBase class docstring gave an override recipe that cannot work. Reassigning the provider attributes after super().setUp() leaves the provider that setUp built installed as the API global.

Three Recorded* dataclasses also moved their field documentation from Args: to Attributes:, so that the napoleon_use_ivar setting at docs/conf.py:225 applies to them.

Verification

Every command below was run at the head of this branch.

  • Docstring coverage over tests/opentelemetry-test-utils/src, measured by walking the AST: 0 remaining gaps, down from 173 at the base commit, which were 15 missing module docstrings, 16 missing class docstrings, 119 missing function or method docstrings, 11 incomplete parameter lists and 12 missing Returns: sections.
  • ruff check and ruff format --check with ruff 0.16.1, the version pinned at dev-requirements.txt:23: pass on all 15 files.
  • tox -e py312-test-opentelemetry-test-utils, which runs pytest tests/opentelemetry-test-utils/tests: 58 passed, 7 skipped, 19 subtests passed, identical to the base commit. All 7 skips are test_weaver_live_check.py skipping because no weaver binary is on PATH.
  • tox -e lint-opentelemetry-test-utils: 10.00/10, no messages. No # pylint: disable= pragma was added anywhere.
  • tox -e typecheck: 0 errors.
  • tox -e public-symbols-check: adds no public symbols.
  • tox -e docs, which runs sphinx-build -W with nitpicky = True set at docs/conf.py:102: build succeeded, and the base commit built in the same environment gives the same result. This branch cannot change that either way: docs/conf.py:42-52 puts only opentelemetry-instrumentation/src, exporter/*/src and shim/*/src on sys.path, the string tests/ appears nowhere in docs/conf.py, and every file this diff touches is under tests/opentelemetry-test-utils/src, which Sphinx never reads.

One code change, and why

The explicit return None that was the whole body of NOOPTextMapPropagator.inject, at tests/opentelemetry-test-utils/src/opentelemetry/test/mock_textmap.py:38 on the base commit, is removed. Ruff exempts a function whose entire body is return None; once the docstring is added, that statement is no longer the entire body and ruff reports RET501. The method is annotated -> None and returns None either way, so behaviour is unchanged. It is in the same commit as the docstring that forced it, because splitting it out would leave one commit failing ruff check.

Things to decide

Four findings are described accurately in the new docstrings and deliberately not fixed, since this is a documentation change. Each could become its own issue.

  1. _AssertNotRaisesMixin._AssertNotRaises.__exit__ at tests/opentelemetry-test-utils/src/opentelemetry/test/__init__.py:81 returns True unconditionally, so it suppresses every exception raised in the block, including one of a type the caller never asked about. with self.assertNotRaises(ValueError): raise KeyError() passes silently. The same method matches types with in rather than issubclass, so a subclass of a watched type does not fail the test either.

  2. AsgiTestBase.tearDown at tests/opentelemetry-test-utils/src/opentelemetry/test/asgitestutil.py:90 and AsyncAsgiTestBase.tearDown at the same file's line 196 override TestBase.tearDown without calling super(), so the trace and metrics globals these classes install in setUp are never reset by them. A later test that is not based on TestBase therefore sees the providers of the last ASGI test until something else replaces them.

  3. _generate_metric at tests/opentelemetry-test-utils/src/opentelemetry/test/metrictestutil.py:33 and _generate_unsupported_metric at line 149 of the same file accept an attributes parameter that neither function reads. The docstrings say so rather than implying the argument has an effect.

  4. HttpTestBase.tearDownClass at tests/opentelemetry-test-utils/src/opentelemetry/test/httptest.py:113 never calls server_close(), so the listening socket of each test class's server is left open until the interpreter exits. The docstring records it.

RichardoMrMu and others added 2 commits November 2, 2026 03:44
…pen-telemetry#5704)

* fix(zipkin): tolerate invalid OTEL_EXPORTER_ZIPKIN_TIMEOUT env var

Both Zipkin exporters parsed OTEL_EXPORTER_ZIPKIN_TIMEOUT with a bare
int() in __init__, so an invalid value (e.g. "10s") raised ValueError
during exporter construction and took SDK startup down, unlike the OTLP
HTTP/gRPC exporters which warn and fall back to the default.

Add _timeout_from_env() in each exporter that skips unset/empty values
silently, logs a warning for non-integer values, and falls back to the
10 second default. Explicit timeout argument precedence is unchanged.

Assisted-by: Doubao

* style(zipkin): inline timeout assignment in both exporters

Collapse the two-line timeout resolution into a single assignment per
review nit; no behavior change.

Assisted-by: Doubao

* test(zipkin): patch os.environ in invalid-timeout fallback test

Use patch.dict to set OTEL_EXPORTER_ZIPKIN_TIMEOUT instead of assigning
os.environ directly, restoring the process environment after the test;
applied symmetrically to both exporters. No behavior change.

Assisted-by: Doubao

* test(zipkin): drop redundant environment restore assertion

patch.dict already restores the process environment, so asserting it
in the invalid-timeout fallback test adds nothing; remove it from both
exporters. No behavior change.

Assisted-by: Doubao

---------

Co-authored-by: Lukas Hering <40302054+herin049@users.noreply.github.com>
Co-authored-by: Leighton Chen <lechen@microsoft.com>
…y#5874)

* chore(deps): update dependency pylint to v4 [security]

* Fix lint compatibility with Pylint 4

Remove the obsolete suggestion-mode option from both Pylint configs and use NoneType when unwrapping optional annotations.

Assisted-by: GPT-6

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Lukas Hering <lukasheringb@gmail.com>
Co-authored-by: Lukas Hering <40302054+herin049@users.noreply.github.com>
@ocelotl

ocelotl commented Nov 2, 2026

Copy link
Copy Markdown
Owner Author

Superseded by open-telemetry#5879, which carries the same commits against upstream main. The branch stays, because that pull request uses it as its head.

@ocelotl ocelotl closed this Nov 2, 2026
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