cyprieng / swagger-parser

Give useful informations about your swagger files
MIT License
62 stars 59 forks source link

Reraise validator exception to avoid losing traceback #57

Closed ceridwen closed 6 years ago

ceridwen commented 6 years ago

At the moment, this raise statement loses the traceback from the validation, which makes it very hard to find out where and why exactly the spec is invalid. (I needed to debug a problem in a 73k line spec.) Using six.reraise saves the traceback and allows the use of pdb to figure out where the validation failed.

flavianh commented 6 years ago

Thanks! Good catch! If that's possible could you write a unit test to check this case?

flavianh commented 6 years ago

@cyprieng any idea why flake8 is not found in the CI?

ceridwen commented 6 years ago

I don't know how to unit test whether an error has a traceback or not without some fragile string matching or traceback introspection. Do you have an existing test for this error path? I couldn't find one.

flavianh commented 6 years ago

You can trigger an exception with the mock library at the right place, and then use the assert raises feature of pytest to catch the exception and check that it has the right paths (see here as well). It's a great exercise if you want to test some tricky stuff in Python, but if it gives you too much trouble I'll merge anyway. It's your call

ceridwen commented 6 years ago

How would you use mock here to trigger the exception?

flavianh commented 6 years ago

@ceridwen use .side_effect = Exception -- see here also: https://stackoverflow.com/questions/28305406/mocking-a-function-to-raise-an-exception-to-test-an-except-block

ceridwen commented 6 years ago

I spent some time looking at mock this afternoon, and honestly getting it working across both Python 2.7 and Python 3 reliably requires too much work for this small a patch, sorry. If you want to refactor the packaging to add support for mock, I can look again then, or you can merge without the test.

flavianh commented 6 years ago

Sure, could you fix the linting issues from the continuous integration though?

ceridwen commented 6 years ago

I fixed the line length issue, the bare except is something that preexists my changes and that I haven't touched.

flavianh commented 6 years ago

I understand, and could you add except Exception on this line so that it's not bare?

ceridwen commented 6 years ago

Done.