Fix ValidationInfo repr #776
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #776 +/- ##
=======================================
Coverage 93.67% 93.67%
=======================================
Files 99 99
Lines 14290 14290
Branches 25 25
=======================================
Hits 13386 13386
Misses 898 898
Partials 6 6
Continue to review full report in Codecov by Sentry.
|
CodSpeed Performance ReportMerging #776 will not alter performanceComparing Summary
|
|
please review |
|
@dmontagu let me know if you think this needs a test or not. I hate testing reprs, I almost always find that the benefit / cost (brittle tests) is not there since ideally nothing should be parsing the repr programmatically -> the worst case is that users see an incorrect repr and report it (like happened to lead to this PR). |
|
While I agree that testing reprs is annoying I think there is value in having a test; ultimately quality of user experience can be degraded by a broken repr even if it's unlikely to cause a production crash. |
|
Having tests also makes the reviewer's life easier because it makes it clearer what the new output is ;) |
|
Ok will add one |
|
Done in 2d759bd |
|
|
||
| v.validate_python(Foo()) | ||
|
|
||
| # insert_assert(reprs) |
There was a problem hiding this comment.
Is this some kind of helper to update the test? 🤔
There was a problem hiding this comment.
Yup comes from devtools that Samuel made
| assert reprs == [ | ||
| 'ValidationInfo(config=None, context=None)', | ||
| "FieldValidationInfo(config=None, context=None, field_name='x')", | ||
| ] |
There was a problem hiding this comment.
It might be nice to also have the case with data present, though I'll forgive you for not having it 😂
There was a problem hiding this comment.
I think it’s tested elsewhere
Original-commit-hash: fe9a462
3 participants
Fixes #775
Selected Reviewer: @dmontagu