-
-
Notifications
You must be signed in to change notification settings - Fork 479
refactor: re-enable default_deserializer coverage
#4430
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
d8f6657 to
39cec47
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4430 +/- ##
==========================================
- Coverage 97.83% 97.78% -0.05%
==========================================
Files 296 296
Lines 15286 15308 +22
Branches 1711 1720 +9
==========================================
+ Hits 14955 14969 +14
- Misses 189 196 +7
- Partials 142 143 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Documentation preview will be available shortly at https://litestar-org.github.io/litestar-docs-preview/4430 |
|
Here's the coverage result: https://app.codecov.io/gh/litestar-org/litestar/commit/39cec47ffa145a7ef8a12d78bd06fd2d2253489e/indirect-changes?dropdown=coverage&el=desc I will add tests for the new red paths. |
|
@provinzkraut I have a hard time figuring out how to actually test this 🤔 |
|
Hmm. For L119-L124 there exists a case that should cover this:
At least that's the test added in #3519, which introduced these lines in the first place. Apparently it's not testing what it's supposed to be testing. Maybe this is a test regression, and that case is getting caught somewhere upstream now? Would be a good starting point for investigating I think. |
this takes the red path I think, or you're talking about the line 125-126 ? |
|
@provinzkraut this test does not trigger the red path :( @euri10 I guess that this test it so not quite correct, because we don't use |

This function is rather important, we need to add more tests if it is not covered, not ignore it.
Let's see the results.