-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
testing: some cleanups #184
Conversation
62b546f
to
275bd60
Compare
i'tll be a bit before i can review, this may conflict with #168 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Fix: ``` testing/test_basics.py::TestPureChannel::test_factory_create[thread] testing/test_basics.py::TestPureChannel::test_factory_getitem[thread] testing/test_basics.py::TestPureChannel::test_channel_timeouterror[thread] testing/test_basics.py::TestPureChannel::test_channel_makefile_incompatmode[thread] /home/ran/src/execnet/.tox/py310/lib/python3.10/site-packages/_pytest/unraisableexception.py:78: PytestUnraisableExceptionWarning: Exception ignored in: <function Channel.__del__ at 0x7fcb5b6a13f0> Traceback (most recent call last): File "/home/ran/src/execnet/execnet/gateway_base.py", line 605, in __del__ self.gateway._send(msgcode, self.id) AttributeError: 'Gateway' object has no attribute '_send'. Did you mean: '_send_'? ```
`startswith` is incorrect for paths in general, e.g. doesn't handle Windows extended-length paths and other such. The particular code here also wasn't careful about `/a/bc` not being relative to `/a/b`.
Use modern alternatives instead.
Finally it passes! There was major headache on the rsync tests on windows. Because @RonnyPfannschmidt I didn't notice #168 regrettably. Some of the changes are similar, I'll try to do the rebase and see the differences. |
Mostly removing use of
py
andtestdir
- see commits.