Skip to content

Bad attempt: Deprecate isolated_filesystem and document its limits - #3701

Closed
kdeldycke wants to merge 2 commits into
mainfrom
stable
Closed

Bad attempt: Deprecate isolated_filesystem and document its limits#3701
kdeldycke wants to merge 2 commits into
mainfrom
stable

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 this to the 8.5.0 milestone Jul 17, 2026
@kdeldycke kdeldycke added f:test runner feature: cli test runner docs labels Jul 17, 2026
@kdeldycke kdeldycke linked an issue Jul 17, 2026 that may be closed by this pull request
@kdeldycke
kdeldycke marked this pull request as draft July 17, 2026 20:49
@kdeldycke

Copy link
Copy Markdown
Collaborator Author

Ouch. I fucked up. This PR has been pushed to stable and not my own fork... 😖 Let me fix it...

@kdeldycke

Copy link
Copy Markdown
Collaborator Author

OK, I cannot revert back stable to its initial state as there is a protection rule on the branch to prevent forced pushed:

$ git push upstream 9ab346f6b57f2c99388b4e9db58a0961bc3027c8:refs/heads/stable --force-with-lease=refs/heads/stable:fbdd434028fe07ecf35084c945b43b938dfd639f
Total 0 (delta 0), reused 0 (delta 0), pack-reused 0 (from 0)
remote: error: GH013: Repository rule violations found for refs/heads/stable.
remote: Review all repository rules at https://github.com/pallets/click/rules?ref=refs%2Fheads%2Fstable
remote: 
remote: - Cannot force-push to this branch
remote: 
To github.com:pallets/click.git
 ! [remote rejected]   9ab346f6b57f2c99388b4e9db58a0961bc3027c8 -> stable (push declined due to repository rule violations)
error: failed to push some refs to 'github.com:pallets/click.git'

@davidism should I just fast forward to sync main and stable? Or just revert the 50+ commits in stable?

@kdeldycke
kdeldycke requested review from Rowlando13 and davidism and removed request for Rowlando13 July 17, 2026 20:58
@kdeldycke

Copy link
Copy Markdown
Collaborator Author

As for the root cause, as I was explaining in #3700 (comment) , I attempted to delete isolated_filesystem a while ago when we started discussing it, and did my experimentation locally, on a branch from stable. Then I reused a cleanup that branch, but it was tied to the upstream stable. I did not realized this when I pushed.

@davidism

Copy link
Copy Markdown
Member

It looks like you made one commit, where are the 50 you're referring to? Push a revert commit, then open a PR from a separate branch.

@kdeldycke

kdeldycke commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator Author

It looks like you made one commit, where are the 50 you're referring to? Push a revert commit, then open a PR from a separate branch.

Yes, there is only one commit difference between stable and main. But stable was supposed to be stuck further away as we moved our focus to preparing 8.5.0. So we got a free sync of main to stable because of my mistake. But that's not the first time I do it and not the first time @Rowlando13 is also doing it! 😅

Ok so I'll just revert the last one and keep the free main to stable sync.

@kdeldycke

Copy link
Copy Markdown
Collaborator Author

Just reverted commit fbdd434 from stable in 8ee83dd

@kdeldycke kdeldycke closed this Jul 17, 2026
@kdeldycke kdeldycke changed the title Deprecate isolated_filesystem and document its limits Bad attempt: Deprecate isolated_filesystem and document its limits Jul 17, 2026
@kdeldycke

Copy link
Copy Markdown
Collaborator Author

OK just fixed it. New PR is at: #3704

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()

2 participants