snok/drf-openapi-tester

Trailing slash in parametrized path

Closed this issue · 4 comments

radekjg commented

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?

sondrelg commented

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 🙂

darduf commented

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 ==============================
sondrelg commented

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?

darduf commented

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

  • 🚀1