Refactoring: use tmp path fixture to mock remote and local for transport plugins #6627
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6627 +/- ##
==========================================
+ Coverage 77.51% 77.89% +0.38%
==========================================
Files 560 567 +7
Lines 41444 42180 +736
==========================================
+ Hits 32120 32850 +730
- Misses 9324 9330 +6 ☔ View full report in Codecov by Sentry. |
unkcpz
force-pushed
the
Remove-the-use-of-deprecated-tranport-methods-in-tests
branch
2 times, most recently
from
aeb255f to
77e0074
Compare
unkcpz
force-pushed
the
Remove-the-use-of-deprecated-tranport-methods-in-tests
branch
from
58b1668 to
f19067c
Compare
unkcpz
force-pushed
the
Remove-the-use-of-deprecated-tranport-methods-in-tests
branch
from
f2c9064 to
db72976
Compare
unkcpz
force-pushed
the
Remove-the-use-of-deprecated-tranport-methods-in-tests
branch
from
56527c6 to
f9bfea9
Compare
for more information, see https://pre-commit.ci
|
When running tests, I got a lot resource not clean warnings which may indicate some resource leak: This require some investigation. |
unkcpz
changed the title
unkcpz
force-pushed
the
Remove-the-use-of-deprecated-tranport-methods-in-tests
branch
3 times, most recently
from
a0c5056 to
3c1a702
Compare
tests/transports/test_all_plugins.py
Outdated
| def test_put_get_abs_path_file(custom_transport): | ||
| def test_put_get_abs_path_file(custom_transport, tmp_path_factory): | ||
| """Test of exception for non existing files and abs path""" | ||
| local_dir = os.path.join('/', 'tmp') | ||
| remote_dir = local_dir | ||
| local_dir = tmp_path_factory.mktemp('local') | ||
| remote_dir = tmp_path_factory.mktemp('remote') |
There was a problem hiding this comment.
to be consistence, can you also define local_tmp_path fixture?
Because now, in some tests only use remote_tmp_path, some other directly uses tmp_path_factory ..
Either that, or I suggest just remove that remote_tmp_path fixture and always to use tmp_path
tests/transports/test_all_plugins.py
Outdated
|
|
||
|
|
||
| def test_exec_pwd(custom_transport): | ||
| def test_exec_pwd(custom_transport, remote_tmp_path): |
There was a problem hiding this comment.
I would keep the previous pwd and instead make it exclusive to core.ssh & core.auto_ssh & core.local.
This test would not make sense for plugins without getcwd
There was a problem hiding this comment.
plugins without getcwd
what you mean by plugins without getcwd? I think ssh/auto_ssh/local all has this method implemented, no?
|
This PR require the change in #6639 to not fail the mypy. |
unkcpz
force-pushed
the
Remove-the-use-of-deprecated-tranport-methods-in-tests
branch
from
cc051b4 to
f4de64d
Compare
|
Thanks @khsrali, I addressed the comments you mentioned, and improve a test to make sure the |
[pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci
unkcpz
force-pushed
the
Remove-the-use-of-deprecated-tranport-methods-in-tests
branch
from
8f120ac to
cd035ad
Compare
unkcpz
deleted the
Remove-the-use-of-deprecated-tranport-methods-in-tests
branch
These are changes required by #6620, and some changes are overlap with changes in #6626. The changes are focus on following two purposes:
chdirandgetcwdmethods.tmp_pathandtmp_path_factoryfixtures to mock remote and local folder to test transport plugins.The change make every test has its own tmp folder therefore can run independent.