Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(cli): add test for readSocketPath #4284

Merged
merged 4 commits into from
Oct 29, 2021
Merged

feat(cli): add test for readSocketPath #4284

merged 4 commits into from
Oct 29, 2021

Conversation

jsjoeio
Copy link
Contributor

@jsjoeio jsjoeio commented Sep 30, 2021

This PR adds tests for readSocketPath.

TODOs

  • extract readSocketPath and export
  • fix TS error
  • write test for readSocketPath

Fixes N/A

@jsjoeio jsjoeio added the testing Anything related to testing label Sep 30, 2021
@jsjoeio jsjoeio self-assigned this Sep 30, 2021
jsjoeio
jsjoeio commented Oct 1, 2021
View reviewed changes
src/node/cli.ts Show resolved Hide resolved
src/node/util.ts Show resolved Hide resolved
@jsjoeio jsjoeio marked this pull request as ready for review October 1, 2021 00:01
@jsjoeio jsjoeio requested a review from a team as a code owner October 1, 2021 00:01
@jsjoeio jsjoeio marked this pull request as draft October 1, 2021 00:01
@github-actions
Copy link

github-actions bot commented Oct 1, 2021 •
edited
Loading

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

@jsjoeio jsjoeio marked this pull request as ready for review October 1, 2021 18:04
bryphe-coder
bryphe-coder reviewed Oct 1, 2021
View reviewed changes
src/node/cli.ts Outdated Show resolved Hide resolved
greyscaled
greyscaled reviewed Oct 1, 2021
View reviewed changes
Copy link
Contributor

@greyscaled greyscaled left a comment

Choose a reason for hiding this comment

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

✔️ looks good to me - just had a question about using a type predicate on ErrnoException

@jsjoeio jsjoeio marked this pull request as draft October 1, 2021 19:24
@jsjoeio
Copy link
Contributor Author

jsjoeio commented Oct 1, 2021

Waiting on #4290

@jsjoeio jsjoeio mentioned this pull request Oct 4, 2021
Closed
@jsjoeio jsjoeio marked this pull request as ready for review October 4, 2021 18:43
@jsjoeio jsjoeio marked this pull request as draft October 4, 2021 18:44
@codecov
Copy link

codecov bot commented Oct 4, 2021 •
edited
Loading

Codecov Report

Merging #4284 (703a502) into main (0100d4c) will increase coverage by 0.08%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4284      +/-   ##
==========================================
+ Coverage   66.21%   66.29%   +0.08%     
==========================================
  Files          31       31              
  Lines        1619     1620       +1     
  Branches      316      316              
==========================================
+ Hits         1072     1074       +2     
+ Misses        469      468       -1     
  Partials       78       78
Impacted Files Coverage Δ
src/node/cli.ts 82.50% <100.00%> (+0.49%) ⬆️
src/node/util.ts 79.23% <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 0100d4c...703a502. Read the comment docs.

@jsjoeio jsjoeio marked this pull request as ready for review October 29, 2021 23:03
@jsjoeio jsjoeio merged commit 946e4e8 into main Oct 29, 2021
@jsjoeio jsjoeio deleted the jsjoeio-test-add branch October 29, 2021 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@greyscaled greyscaled greyscaled left review comments

@code-asher code-asher code-asher left review comments

@bryphe-coder bryphe-coder bryphe-coder approved these changes

Assignees

@jsjoeio jsjoeio

Labels
testing Anything related to testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jsjoeio @greyscaled @code-asher @bryphe-coder