ML-3730: fix support for client running on windows OS #106
Conversation
…join_url logic[1~
assaf758
force-pushed
the
ML-3730
branch
3 times, most recently
from
6bad94d
to
b425feb
Compare
assaf758
changed the title
| @@ -266,7 +266,7 @@ def test_verifier_transport(self): | |||
|
|
|||
| # verify some stuff from the request | |||
| self.assertEqual(request.container, container_name) | |||
| self.assertEqual(request.path, os.path.join(os.sep, container_name, 'some/path')) | |||
There was a problem hiding this comment.
Do we really want to put this in the readme? If so, we should put the import there as well.
| # | ||
| import unittest | ||
|
|
||
| from v3io.common.helpers import url_join |
There was a problem hiding this comment.
Maybe this file should be called test_helpers.py?
|
|
||
|
|
||
| class Test(unittest.TestCase): | ||
| def test_url_join(self): |
There was a problem hiding this comment.
Probably better make this a parameterized test, so that we can see which test cases pass and which fail.
tests/test_common.py
Outdated
| self.assertEqual(url_join("a", "b"), "a/b") # keep suffix "/" exist/not-exist invariant | ||
| self.assertEqual(url_join("a", "b/"), "a/b/") | ||
| self.assertEqual(url_join("a", "b//"), "a/b//") | ||
| self.assertEqual(url_join("a", "b//", "/"), "a/b//") # suffix "/" count (if > 0) may change (but we don"t care) |
There was a problem hiding this comment.
| self.assertEqual(url_join("a", "b//", "/"), "a/b//") # suffix "/" count (if > 0) may change (but we don"t care) | |
| self.assertEqual(url_join("a", "b//", "/"), "a/b//") # suffix "/" count (if > 0) may change (but we don't care) |
Also, I wonder if we don't want to remove redundant slashes in any case.
|
|
||
| for part in parts: | ||
| if part[0] != "/": | ||
| def url_join(*parts): |
There was a problem hiding this comment.
I think this might be clearer:
def url_join(*parts):
slash_suffix = False
result = ""
for part_index, part in enumerate(parts):
if part == "":
continue
if slash_suffix:
# if slash suffix existed in prev trim slash prefix from this part
result += part.lstrip("/")
else:
# add slash prefix before part if:
# 1. slash suffix did not exist in prev part and
# 2. slash prefix does not exist in this part and
# 3. part is not the first
if part[0] != "/" and part_index != 0:
result += "/"
result += part
slash_suffix = part[-1] == "/"
return result| # add slash prefix before part if: | ||
| # 1. slash suffix did not exit in prev part | ||
| # 2. slash prefix does not exit in this part | ||
| # 3. part is not the first |
There was a problem hiding this comment.
exit -> exist
Need to make it clear that all 3 points need to be true (could be read as just one of them).
tests/test_common.py
Outdated
|
|
||
| class Test(unittest.TestCase): | ||
| def test_url_join(self): | ||
| self.assertEqual(url_join("a", "b"), "a/b") # add just exactly one "/" between parts |
There was a problem hiding this comment.
For whaterver reason, assertEqual treats the left side as the expected and right side as the actual. Opposite of a simple assert.
| @@ -24,7 +24,7 @@ | |||
|
|
|||
|
|
|||
| def encode_list(list_value): | |||
| typecode = "l" | |||
| typecode = "q" | |||
There was a problem hiding this comment.
Maybe use a constant or a comment to communicate that this means 8 bytes.
Fix support for client running on windows OS:
longtypes in arraysTested on a windows vm by installing v3io-py private branch and running the CI test suit