Skip to content

test: cover file reading, sources and utilities - #1830

Open
ariostas wants to merge 3 commits into
scikit-hep:mainfrom
ariostas:test/io-coverage
Open

ariostas wants to merge 3 commits into
scikit-hep:mainfrom
ariostas:test/io-coverage

Conversation

@ariostas

@ariostas ariostas commented Nov 6, 2026 •

Copy link
Copy Markdown
Member

🤖 AI text below 🤖

Part of the split of #1814 (the list is in #1834). These are the new tests of #1814 for file reading, compression, cursors and chunks, exceptions and extras, file-like objects, fsspec, futures, HTTP, XRootD and utilities, apart from the regression tests of its fixes, which are in the fix PRs. All of them pass on main.

It adds test files and deletes test_future_result_timeout_raises and test_threadpool_shutdown_joins_workers of test_1654, which test_1830_cov_io_futures.py replaces.

@github-actions github-actions Bot added the type/test PR title type: test (set automatically) label Nov 6, 2026
@ariostas
ariostas marked this pull request as ready for review November 6, 2026 14:47
@codecov

codecov Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.18%. Comparing base (e12d2a4) to head (3f9a14d).

Additional details and impacted files

see 21 files with indirect coverage changes

@ariostas
ariostas force-pushed the test/existing-tests branch from c192219 to 5253a21 Compare November 6, 2026 15:10
ariostas added a commit to ariostas/uproot5 that referenced this pull request Nov 6, 2026
Six tests (of Future.result timeouts, ThreadPoolExecutor.shutdown, the order
of LRUCache, and Library equality) were deleted here in favor of tests in
scikit-hep#1829, scikit-hep#1830 and scikit-hep#1832, which are based on this PR; until those are merged
nothing else would cover them. They are now deleted together with the tests
that replace them. A comment no longer points to a file of scikit-hep#1828.

Assisted-by: Claude Code:claude-opus-5-5
ianna pushed a commit that referenced this pull request Nov 7, 2026
* test: tighten the existing tests and make their fixtures robust

Changes to existing tests from #1814 that don't depend on its fixes: they
check values instead of only running, restore checks that had been dropped,
close the files they open, and run concurrently (each XRootD server has its
own admin path and waits until it accepts connections, and reset_classes puts
the original model classes back).

Assisted-by: Claude Code:claude-opus-5-5

* test: keep the tests that the new coverage tests replace

Six tests (of Future.result timeouts, ThreadPoolExecutor.shutdown, the order
of LRUCache, and Library equality) were deleted here in favor of tests in
#1829, #1830 and #1832, which are based on this PR; until those are merged
nothing else would cover them. They are now deleted together with the tests
that replace them. A comment no longer points to a file of #1828.

Assisted-by: Claude Code:claude-opus-5-5

* test: iterate over XRootD in one step per file

MET_pt is in one TBasket, so with steps of 15 entries the later steps of a
file had nothing to request, and the XRootD source then sent an empty vector
read, which failed now and then. #1825 stops sending it; until then, these
tests iterate in one step per file.

Assisted-by: Claude Code:claude-opus-5-5
@ariostas
ariostas changed the base branch from test/existing-tests to main November 12, 2026 16:02
New tests from scikit-hep#1814, apart from the regression tests of its fixes.

Assisted-by: Claude Code:claude-opus-5-5
The test of closing an fsspec source with failing workarounds now checks
that both were tried, and the test of splitting XRootD ranges checks the
sizes of the vector-read elements. The local HTTP server polls for shutdown
every 0.01 s instead of 0.5 s, which every test waited for. The two tests of
test_1654 that test_1830_cov_io_futures.py replaces are deleted here.

Assisted-by: Claude Code:claude-opus-5-5
Assisted-by: Claude Code:claude-opus-5-5

This branch has not been deployed

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

Labels

type/test PR title type: test (set automatically)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant