Prep for major release of rtm-api #1764
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hello-ashleyintech
approved these changes
Mar 28, 2024
breaking change update)
…transition back to disconnected. also add basic integration tests.
|
@hello-ashleyintech I added the tests here! |
10 tasks
hello-ashleyintech
approved these changes
Mar 28, 2024
| // Socket mode client pointing to the above two posers | ||
| let client = null; | ||
|
|
||
| describe('Integration tests with a WebSocket server', () => { |
zimeg
approved these changes
Mar 28, 2024
Comment on lines
+80
to
+83
| it('should not raise an exception if calling disconnect() when already disconnected', async () => { | ||
| // https://github.com/slackapi/node-slack-sdk/issues/842 | ||
| await client.disconnect(); | ||
| }); |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Mainly updating dependencies and dropping support for node below 18.
BUT! I am worried that this package has no tests. Esp. since I updated to the latest major of
websocketswhich has some breaking changes.Can I manually test an RTM app these days? I have no idea. EDIT: YES it's possible! I tested a RTM app w/ the old package prior to this change, works fine. Also tested it with the changes in this PR, also works OK!
Another thought: maybe I copy the basic web socket server integration test suite I added to the socket mode package into here (or factor the server out into aI did this! This package now includes basic integration tests. I was able to even add a test to confirm #842 is an issue and added a fix that passes the test 😄utils/dir or something in this repo and re-use it in both packages?). Then I could at least add some smoke test-style integration tests? That should in theory also be able to write a test for #842 and thus feel confident about a fix.