-
Notifications
You must be signed in to change notification settings - Fork 629
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
test(crypto): improve test coverage #4620
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4620 +/- ##
==========================================
+ Coverage 90.89% 90.99% +0.09%
==========================================
Files 475 478 +3
Lines 37325 37383 +58
Branches 5293 5308 +15
==========================================
+ Hits 33927 34016 +89
+ Misses 3336 3305 -31
Partials 62 62 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent work so far! I don't think we can do much about crypto/_wasm/lib/deno_std_wasm_crypto.generated.mjs
. Maybe we should just ignore it in Codecov.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you able to target https://app.codecov.io/gh/denoland/deno_std/pull/4620/blob/crypto/crypto.ts#L260 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i've tried by stubbing webCrypto
to be able to get into the remaining else
block, but no luck so far.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, that last else
clause might be redundant. I'll handle this in a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great stuff! Thank you.
working towards #3713.
pushes code coverage of the
crypto
sub-module closer to 100%.currently the following files aren't at 100%:
_wasm
:crypto
:webCrypto
is bound toglobalThis.crypto
my usual strategy ofstub
bing didn't seem to work out.webCrypto.subtle.digest
ever be falsy to begin with?unstable_keystack
:Symbol.for("nodejs.util.inspect.custom")
i tried to simply importinspect
fromnode:util
and compare the outcome, similar on how the test case forSymbol.for("Deno.customInspect")
looked like. however this didn't change anything on cov.