Enable 'Format' and 'Content' assertions during schema validation #116
Conversation
…ormat' and 'content'
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #116 +/- ##
==========================================
- Coverage 99.74% 99.56% -0.19%
==========================================
Files 29 30 +1
Lines 3163 3203 +40
==========================================
+ Hits 3155 3189 +34
- Misses 8 14 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
- Updated to cover all situations where JSON schema compilation occurs. - A few new Unts tests to improve coverage.
config/config.go
Outdated
| // Sanity | ||
| if opt != nil { | ||
| opt(o) | ||
| } |
There was a problem hiding this comment.
Maybe this
| // Sanity | |
| if opt != nil { | |
| opt(o) | |
| } | |
| if opt == nil { | |
| // Sanity | |
| continue | |
| } | |
| opt(o) |
The sanity is in the nil check. The comment is misleading otherwise
There was a problem hiding this comment.
opted (no pun intended) to remove the comment but leave the logic as-is ... seemed more logical to me.
|
If you can bump those couple of lines, we can add this! |
Will take a look. How would I trigger those checks ... they seemed to trigger after the PR got approved |
- Added more 'nil' protection
|
Let me know how far down this rabbit hole you'd like me to go. If I'm ready this correctly the coverage failed coverage results from some very small edge-cases around error checks (where in the past there were no checks!) - coercing tests to get the desired coverage is presumably doable but is that a blocker ? |
It's not a blocker, I just hate to see coverage drop with new additions. I need to pull down the code and see if the work required top test it, is more work than the value its creating before waving it through. Sometimes code just needs a hall pass, other times it needs to be run through the machine. I'll pull your branch and run the code and evaluate the level of effort to continue going down the hole or not. |
|
Thanks @daveshanley - I understand the concern and can try and go deeper as needed. I guess this is the dark-side of refactoring and adding more error-checks ;-) |
Introduced more validation configuration options to apply during schema validation.
These changes have only currently been made to the query-parameter validation as a proof-of-concept, if this looks like an appropriate approach I will apply it more broadly - as such please don't merge at this time.
Included are some additional unit tests for query parameters to demonstrate the enforcement of 'date' and 'date-time' forma ts.