[WIP] bpo-40275: add filesystem_helper in test.support

[shihai1991]

https://bugs.python.org/issue40275

[shihai1991]

It's checkout from PR20263.

[shihai1991]

@vstinner Hi, victor. Can we split this PR become smaller? It's too huge :(.

[brettcannon]

@shihai1991 please don't open draft/WIP PRs here. As you can see you pinged 17 people with this PR for a review which isn't ready for any review.

[terryjreedy]

It's checkout from PR20263.

PR branches must be branched off of master. Because you branched off of an existing pr branch, #20263, adding a threading helper, this PR contains all of those changes plus the new changes for filesystem helper. That makes this unreviewable and the two PR interdependent. Hence the tag. This should be closed if you cannot rebase on master.

Aside from that, git said "Merge conflict in Lib/test/test_tools/test_md5sum.py' when I tried to make a PR from this. Also, Travis is obviously stuck.

I suggest that for a draft, you only move functions to the new module and have Victor review just that change. If there is a test of the support functions, include that. The devguide may not say how to review a branch pushed to a fork but not made into a PR, but I think I could do it and am sure Victor could.

I strongly suggest that you make the changes to individual test files as minimal as possible by respecting authors' imports and importing the same objects. If a person imported specific functions, continue to do so. Instead of

- from test.support import findfile
+from test.support import filesystem_helper

which requires an indefinite number of other changes in the file (and sometimes many), which are a nuisance to check,

- from test.support import findfile
+from test.support.filesystem_helper import findfile

which should require no other changes to be checked in that file.

[vstinner]

I suggest to wait until PR #20263 is merged. Maybe even close this PR in the meanwhile (it's up to you).

[shihai1991]

@shihai1991 please don't open draft/WIP PRs here. As you can see you pinged 17 people with this PR for a review which isn't ready for any review.

sorry for my noisy PR.

[shihai1991]

It's checkout from PR20263.

PR branches must be branched off of master. Because you branched off of an existing pr branch, #20263, adding a threading helper, this PR contains all of those changes plus the new changes for filesystem helper. That makes this unreviewable and the two PR interdependent. Hence the tag. This should be closed if you cannot rebase on master.

Aside from that, git said "Merge conflict in Lib/test/test_tools/test_md5sum.py' when I tried to make a PR from this. Also, Travis is obviously stuck.

I suggest that for a draft, you only move functions to the new module and have Victor review just that change. If there is a test of the support functions, include that. The devguide may not say how to review a branch pushed to a fork but not made into a PR, but I think I could do it and am sure Victor could.

I strongly suggest that you make the changes to individual test files as minimal as possible by respecting authors' imports and importing the same objects. If a person imported specific functions, continue to do so. Instead of

- from test.support import findfile
+from test.support import filesystem_helper

which requires an indefinite number of other changes in the file (and sometimes many), which are a nuisance to check,

- from test.support import findfile
+from test.support.filesystem_helper import findfile

which should require no other changes to be checked in that file.

Thanks for your suggestion, Terry. I will follow your suggestion.

why I import entire module: it could be more clearly where the function we used come from.

[terryjreedy]

Associating module and object names in the import versus use site each have advantages and tradeoffs. Different personal preferences would be an interesting python-list discussion.