👍 Improved test error handling. #16
Conversation
WalkthroughThe recent updates refine connection handling in Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- README.md (1 hunks)
- with.ts (3 hunks)
Additional Context Used
LanguageTool (3)
README.md (3)
Near line 14: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 1403 characters long)
Context: ...://github.com/vim-denops/denops.vim > [!NOTE] > > To use thetestfunction, an...
Near line 23: Loose punctuation mark.
Context: ...tion: > > -DENOPS_TEST_VIM_EXECUTABLE: Path to the Vim executable (default: "v...
Near line 72: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...ll("abs", -4), 4); }, }); ``` If you want to test denops plugins without a real Vim ...
Additional comments not posted (2)
with.ts (2)
92-98: Ensure proper error handling for connection timeouts.The restructuring of error handling in
getConnectionto specifically throw a new error forDeadlineErrorinstances is a good practice as it provides clearer error messages to the users.
135-145: Verify the concurrent handling ofperformandproc.output().
|
Wait #17 |
Milly
force-pushed
the
proc
branch
2 times, most recently
from
2bf4935
to
ec641c6
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16 +/- ##
==========================================
+ Coverage 82.00% 83.82% +1.82%
==========================================
Files 7 7
Lines 389 470 +81
Branches 40 50 +10
==========================================
+ Hits 319 394 +75
- Misses 70 75 +5
- Partials 0 1 +1 ☔ View full report in Codecov by Sentry. |
Milly
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
Out of diff range and nitpick comments (1)
with.ts (1)
Line range hint
32-183: The refactoring of thewithDenopsfunction improves modularity and error handling. Ensure to add test coverage for the error handling during session shutdown.+ // TODO: Add tests to cover error handling during session shutdown in the withDenops function
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- README.md (1 hunks)
- conf.ts (3 hunks)
- runner.ts (4 hunks)
- with.ts (4 hunks)
- with_test.ts (1 hunks)
Additional Context Used
LanguageTool (3)
README.md (3)
Near line 14: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 1625 characters long)
Context: ...://github.com/vim-denops/denops.vim > [!NOTE] > > To use thetestfunction, an...
Near line 23: Loose punctuation mark.
Context: ...tion: > > -DENOPS_TEST_VIM_EXECUTABLE: Path to the Vim executable (default: "v...
Near line 70: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...ll("abs", -4), 4); }, }); ``` If you want to test denops plugins without a real Vim ...
GitHub Check Runs (1)
codecov/patch success (3)
conf.ts: [warning] 78-78: conf.ts#L78
Added line #L78 was not covered by tests
runner.ts: [warning] 78-82: runner.ts#L78-L82
Added lines #L78 - L82 were not covered by tests
with.ts: [warning] 160-161: with.ts#L160-L161
Added lines #L160 - L161 were not covered by tests
Additional comments not posted (6)
with_test.ts (3)
13-19: The test structure and assertions are correctly implemented for "vim" mode.
21-27: The test structure and assertions are correctly implemented for "nvim" mode.
29-61: The loop correctly generates tests for both process abortion and connection failure scenarios in "vim" and "nvim" modes.conf.ts (1)
32-37: The addition of theconnectTimeoutfield to theConfiginterface is well-documented and enhances configuration flexibility.runner.ts (1)
22-34: TheRunResultinterface is well-designed, providing encapsulated process management methods and extendingAsyncDisposablefor better resource management.README.md (1)
27-28: The documentation updates clearly explain the newDENOPS_TEST_CONNECT_TIMEOUTenvironment variable and its default value.
Improved test error handling.
verbose=true) was not supported on Windows. Supported now.DENOPS_TEST_CONNECT_TIMEOUTenvironment variable.using.Summary by CodeRabbit
Summary by CodeRabbit
withDenopsfunction by restructuring logic for improved error management and operational efficiency.