Reveal previously hidden AttributeErrors and TypeErrors #3668
Conversation
|
Prob worth issuing as two separate PRs. |
|
Sure, I'll snip that one out. |
| @@ -169,7 +170,7 @@ class Meta: | |||
| file_path_field = FilePathField(path='/tmp/') | |||
| """) | |||
|
|
|||
| self.assertEqual(unicode_repr(TestSerializer()), expected) | |||
There was a problem hiding this comment.
Addressing any windows test failures should probably be treated as a separate PR.
I assume that's what this change is about, right?
There was a problem hiding this comment.
Yeah -- though it's a bit odd that repr would act differently across platforms...
|
I'll split the test stuff into yet another PR. :) |
akx
force-pushed
the
exc-hiding
branch
2 times, most recently
from
93e344f to
447ad2a
Compare
|
|
||
| class GetQuerysetRaises(generics.ListAPIView): | ||
| def get_queryset(self): | ||
| raise CheckAttributeError("Something terrible occurred deep down in the call stack") |
There was a problem hiding this comment.
Just a regular AttributeError would be okay here.
There was a problem hiding this comment.
Or perhaps return self.this_attribute_does_not_exist?
There was a problem hiding this comment.
I subclassed AttributeError for the test for the purpose of being able to check for that exact exception being propagated up, which was the point of the patch in the first place :)
|
Okay, on review at this point I'd actually rather have this, but without the test cases. |
|
Well, if you're sure! I'll take the test cases out. |
Quietly catching `AttributeError` and `TypeError` when calling `get_queryset()` is rather insidious, as those exceptions get caught no matter where they might happen in the call stack.
|
Tests are now gone, @tomchristie. |
|
May seem counter-intuitive, but they come with an associated cost, and can be okay not to strictly test against obviously better/cleaner behavior. |
Reveal previously hidden AttributeErrors and TypeErrors
This ensures AttributeErrors and TypeErrors do not get hidden when DRF is just looking for a
get_queryset().