drf-openapi-tester icon indicating copy to clipboard operation
drf-openapi-tester copied to clipboard

Trailing slash in parametrized path

Open radekjg opened this issue 3 years ago • 3 comments

I have a question about a certain line of code in drf-openapi-tester/openapi_tester/loaders.py

parsed_path = url_object.path if url_object.path.endswith("/") else url_object.path + "/"

Why do you append slash to a parametrized url? I have a problem now, because I have a path without slash and schema tester raises an error. Could we not add here a slash? Would it break something?

radekjg avatar Apr 14 '22 16:04 radekjg

I tried digging into the (not so great) git history, but I'm not able to remember or figure our why we added this. My best guess would be that drf-yasg which is what I primarily used for my own projects might have output trailing slashes in the generated schema or something like that.

Have you tried running the test suite without this trailing-slash code? We might be fine removing it 🙂

sondrelg avatar Apr 15 '22 16:04 sondrelg

I also have an issue with trailing slashes. Some of our path definitions have no trailing slashes and raises errors.

Is it possible to remove this?

Removing the slash from code causes 4 failures:

======================================= short test summary info =======================================
FAILED tests/test_loaders.py::test_loader_get_route[loader0] - ValueError: Could not resolve path `/...
FAILED tests/test_loaders.py::test_loader_get_route[loader1] - ValueError: Could not resolve path `/...
FAILED tests/test_loaders.py::test_loader_get_route[loader2] - ValueError: Could not resolve path `/...
FAILED tests/test_loaders.py::test_loader_get_route[loader3] - ValueError: Could not resolve path `/...
============================== 4 failed, 166 passed, 1 warning in 10.42s ==============================

darduf avatar Sep 28 '22 11:09 darduf

It's been a while since I've touched this library, so the details are a bit fuzzy. I have no objections to changing the logic here, but need:

  • A way to reproduce the issue, or even better
  • A PR

to fix 🙏

Interested in creating a PR for this @darduf or @radowit?

sondrelg avatar Sep 29 '22 00:09 sondrelg

@sondrelg I have added a PR that reproduces the issue we are seeing 👍

darduf avatar Nov 25 '22 15:11 darduf