Skip to content

feat: add test for hash with empty string - #4814

Merged
jsjoeio merged 2 commits into
mainfrom
jsjoeio-hash-test
Feb 3, 2022
Merged

feat: add test for hash with empty string#4814
jsjoeio merged 2 commits into
mainfrom
jsjoeio-hash-test

Conversation

@jsjoeio

@jsjoeiojsjoeio commented Jan 31, 2022

Copy link
Copy Markdown
Contributor

This PR adds an additional test for the hash function.

Fixes N/A

#TestingMondays

@jsjoeiojsjoeio added the testing Anything related to testing label Jan 31, 2022
@jsjoeiojsjoeio added this to the 4.0.3 milestone Jan 31, 2022
@jsjoeiojsjoeio self-assigned this Jan 31, 2022
@jsjoeio
jsjoeio requested a review from a teamJanuary 31, 2022 23:12
@jsjoeio
jsjoeio temporarily deployed to CI January 31, 2022 23:12 Inactive
@github-actions

github-actionsBot commented Jan 31, 2022

Copy link
Copy Markdown

✨ Coder.com for PR #4814 deployed! It will be updated on every commit.

@codecov

codecovBot commented Jan 31, 2022

Copy link
Copy Markdown

Codecov Report

❗ No coverage uploaded for pull request base (main@fd643dc). Click here to learn what that means.
The diff coverage is 100.00%.

Impacted file tree graph

@@ Coverage Diff @@## main #4814 +/- ##
=======================================
Coverage ? 69.25% =======================================
Files ? 29 Lines ? 1649 Branches ? 363 =======================================
Hits ? 1142 Misses ? 430 Partials ? 77 
Impacted FilesCoverage Δ
src/node/util.ts82.06% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fd643dc...6db1ac2. Read the comment docs.

Comment threadtest/unit/node/util.test.ts Outdated
@jsjoeio
jsjoeio temporarily deployed to CI February 3, 2022 19:53 Inactive
@jsjoeiojsjoeio changed the title feat: add test for hash when errorfeat: add test for hash with empty stringFeb 3, 2022
Comment threadsrc/node/util.ts
Comment on lines -160 to -165
try {
return await argon2.hash(password)
} catch (error: any) {
logger.error(error)
return ""
}

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✍🏼 as discussed with @code-asher, this will only throw if you throw in something other than string (like number). Since TS will prevent that, we don't need the try/catch block.

@jsjoeio
jsjoeio temporarily deployed to CI February 3, 2022 20:54 Inactive

@code-ashercode-asher left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@jsjoeio
jsjoeio merged commit 00224fa into mainFeb 3, 2022
@jsjoeio
jsjoeio deleted the jsjoeio-hash-test branch February 3, 2022 21:22
TinLe pushed a commit to TinLe/code-server that referenced this pull request Apr 23, 2022
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testingAnything related to testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@jsjoeio@code-asher