Skip to content

Deprecate isolated_filesystem and document its limits - #3704

Merged
Rowlando13 merged 1 commit into
pallets:mainfrom
kdeldycke:deprecate-isolated-filesystem
Jul 20, 2026
Merged

Deprecate isolated_filesystem and document its limits#3704
Rowlando13 merged 1 commit into
pallets:mainfrom
kdeldycke:deprecate-isolated-filesystem

Conversation

@kdeldycke

Copy link
Copy Markdown
Collaborator

This is an attempt to deprecate isolated_filesystem from the CliRunner. It has been discussed in #3501.

In addition to the deprecation, I added some explicit documentation about thread-safety as discussed in #3501. I used our own usage of pytest-xdist as a good example to follow.

All occurrences of isolated_filesystem in our own test suite have been removed and we are now relying on Pytest's tmp_path.

I also inaugurated the docs/upgrade-guides.md section introduced a while ago by @Rowlando13 .

This PR closes #3700.

@kdeldycke kdeldycke added docs f:test runner feature: cli test runner labels Jul 17, 2026
@kdeldycke kdeldycke added this to the 8.5.0 milestone Jul 17, 2026
@kdeldycke kdeldycke linked an issue Jul 17, 2026 that may be closed by this pull request
@kdeldycke
kdeldycke force-pushed the deprecate-isolated-filesystem branch from 7afc4df to ea8140a Compare July 17, 2026 21:40
@kdeldycke
kdeldycke requested review from Rowlando13 and davidism and removed request for davidism July 17, 2026 21:41
@Rowlando13

Copy link
Copy Markdown
Member

Just a nit. Can you add a test of the deprecation warning?

@Rowlando13 Rowlando13 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good except nit.

@kdeldycke
kdeldycke force-pushed the deprecate-isolated-filesystem branch from ea8140a to c2ed414 Compare July 20, 2026 12:20
@kdeldycke

Copy link
Copy Markdown
Collaborator Author

Just a nit. Can you add a test of the deprecation warning?

Ah yes, you probably missed it because it was in tests/test_testing.py. So I just applied your convention from your other PR and moved it to tests/test_deprecations.py.

@Rowlando13
Rowlando13 merged commit cfa01ee into pallets:main Jul 20, 2026
12 checks passed
claytonlin1110 added a commit to claytonlin1110/diffmeter that referenced this pull request Jul 21, 2026
score_diff() and score_pull_request() now score files in parallel via a
thread pool (--jobs, default 8; --jobs 1 disables it). Biggest win is
--pr mode, dominated by network round-trips: measured 25s -> 7s on a
real 10-file PR (pallets/click#3704), with byte-identical output between
sequential and concurrent runs.

Before adding any of that, fixed a real hazard it would have introduced:
languages.get_parser() cached tree-sitter Parser objects in a process-wide
@lru_cache, so every thread would share and call .parse() on the *same*
Parser instance -- not documented as safe for concurrent use, and would
likely have meant intermittent corruption or crashes under load. Switched
to a thread-local cache (confirmed the underlying library returns a fresh
Parser per call, so each thread now gets its own). Locked in with a test
that forces two distinct OS threads via threading.Thread directly, since
a naive ThreadPoolExecutor-based version of that test can silently reuse
one idle worker for both calls and pass for the wrong reason.
@kdeldycke
kdeldycke deleted the deprecate-isolated-filesystem branch July 22, 2026 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs f:test runner feature: cli test runner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deprecate isolated_filesystem() isolated_filesystem() is not thread safe

2 participants