[harshkoli1255]

Description

Add focused unit test coverage for the existing JSON file exporter's stream lifecycle and concurrent export behavior.

The tests verify:

• Caller-provided streams remain open after shutdown.
• Default stdout remains open after shutdown.
• Exporter-owned file streams close after shutdown.
• Closed streams are handled gracefully with a warning.
• Concurrent exports produce complete, valid JSON lines without overlapping writes.
• The concurrency test uses a custom stream to detect overlapping writes.

Testing

• JSON file exporter test suite: 50 passed.
• Ruff lint: passed.
• Ruff formatting: passed.
• No production code, public APIs, or dependencies were modified.

Changelog

Skip Changelog

[opentelemetry-pr-dashboard]

Pull request dashboard status

Waiting on the author · refreshed 2026-10-28 14:15 UTC

Investigate required status check failures.

Status above doesn't look right?

• Just replied or pushed? Anything around or after the refresh time above may not be picked up yet — give it a few minutes.
• Should this be with reviewers? Comment /dashboard route:reviewers to route it to them.
• Anything wrong — including the routing? Report it with what you expected; it helps us improve the dashboard.

[harshkoli1255]

Hi maintainers,

The CI workflows for this PR are currently awaiting approval to run.

Could someone approve them when convenient? The local tests and Ruff checks are passing.

Thanks!

[harshkoli1255]

Hi maintainers,

I've pushed fixes for the Pyright and Pylint CI failures on PR #5815. All 50 tests, Pyright, Pylint, and Ruff checks now pass locally.

The updated GitHub Actions workflows are awaiting approval. Could someone approve the runs and apply the Skip Changelog label, since this is a test-only PR?

PR: #5815

Thanks!

[chrikrah]

@harshkoli1255 The new tests hold under mutation at ed9e845: each one fails when I break the behaviour it names in _internal.py.

$ pytest exporter/opentelemetry-exporter-otlp-json-file/tests -q -p no:cacheprovider   # Python 3.12
50 passed in 0.21s                                    # ed9e845
45 passed in 0.30s                                    # test_internal.py from bba3bc3

# mutations of _internal.py at ed9e845, then pytest .../tests/test_internal.py
no `with self._lock:` in export()             1 failed, 8 passed   (concurrent_exports: "Concurrent writes were detected")
no `self._stream.closed` check                1 failed, 8 passed
shutdown() never closes the stream            1 failed, 8 passed
shutdown() closes any stream                  2 failed, 5 passed, 5 errors
same, with the stdout test under mock.patch   2 failed, 7 passed

non-blocking: in the "closes any stream" row, test_default_stdout_remains_open_after_shutdown closes pytest's own sys.stdout, and the 5 errors are the tests after it. Under mock.patch("sys.stdout", new_callable=io.StringIO) the same mutation fails only the two tests that name it. Would you fold that in?

[harshkoli1255]

@harshkoli1255 The new tests hold under mutation at ed9e845: each one fails when I break the behaviour it names in _internal.py.

$ pytest exporter/opentelemetry-exporter-otlp-json-file/tests -q -p no:cacheprovider   # Python 3.12
50 passed in 0.21s                                    # ed9e845
45 passed in 0.30s                                    # test_internal.py from bba3bc3

# mutations of _internal.py at ed9e845, then pytest .../tests/test_internal.py
no `with self._lock:` in export()             1 failed, 8 passed   (concurrent_exports: "Concurrent writes were detected")
no `self._stream.closed` check                1 failed, 8 passed
shutdown() never closes the stream            1 failed, 8 passed
shutdown() closes any stream                  2 failed, 5 passed, 5 errors
same, with the stdout test under mock.patch   2 failed, 7 passed

non-blocking: in the "closes any stream" row, test_default_stdout_remains_open_after_shutdown closes pytest's own sys.stdout, and the 5 errors are the tests after it. Under mock.patch("sys.stdout", new_callable=io.StringIO) the same mutation fails only the two tests that name it. Would you fold that in?

Thanks @chrikrah! I've included the suggested mock.patch change in commit 673b23fc. All tests and lint checks pass locally.

[chrikrah]

@harshkoli1255 with 673b23f the "shutdown closes any stream" mutation now fails only the two tests that name it, with no errors spilling into the rest of the file. Still approving.

$ pytest exporter/opentelemetry-exporter-otlp-json-file/tests -q -p no:cacheprovider   # export of 673b23f, _internal.py imported from it, Python 3.12.3
50 passed in 1.41s
# _internal.py at 673b23f with `if self._owns_stream:` replaced by `if True:` in shutdown()
$ pytest .../tests/test_internal.py -q -p no:cacheprovider
2 failed, 7 passed in 0.77s     # was 2 failed, 5 passed, 5 errors at ed9e845
FAILED .../test_internal.py::TestFileExporter::test_caller_stream_remains_open_after_shutdown
FAILED .../test_internal.py::TestFileExporter::test_default_stdout_remains_open_after_shutdown

[harshkoli1255]

$ pytest exporter/opentelemetry-exporter-otlp-json-file/tests -q -p no:cacheprovider # export of 673b23f, _internal.py imported from it, Python 3.12.3
50 passed in 1.41s

_internal.py at 673b23f with if self._owns_stream: replaced by if True: in shutdown()

$ pytest .../tests/test_internal.py -q -p no:cacheprovider
2 failed, 7 passed in 0.77s # was 2 failed, 5 passed, 5 errors at ed9e845
FAILED .../test_internal.py::TestFileExporter::test_caller_stream_remains

Thanks @chrikrah for reviewing again and verifying the updated tests! Really appreciate the detailed feedback.