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

Add websocket / misc tests (#3190) #3299

Merged
merged 9 commits into from
Jan 15, 2020
Merged

Add websocket / misc tests (#3190) #3299

merged 9 commits into from
Jan 15, 2020

Conversation

cgewecke
Copy link
Collaborator

@cgewecke cgewecke commented Jan 10, 2020 •
edited
Loading

Description

PR targets #3190 (Provider Improvements)

Part of another round of review for provider improvements :)

Mostly just closing some gaps in the coverage report and testing the behavior of auto-reconnect.

There's one logic change, will discuss in the diff below.

@coveralls
Copy link

coveralls commented Jan 10, 2020 •
edited
Loading

Coverage Status

Coverage increased (+0.9%) to 87.558% when pulling 6428287 on pri/testing-gaps into 0252714 on provider-related-improvements.

@cgewecke cgewecke requested a review from nivida January 10, 2020 04:01
nivida
nivida reviewed Jan 10, 2020
View reviewed changes
Copy link
Contributor

@nivida nivida left a comment

Choose a reason for hiding this comment

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

I've only added some comments about the websocket.misc test file because it looks like something we do not really want to have in this format. :)

test/websocket.misc.js Outdated Show resolved Hide resolved
test/websocket.misc.js Outdated Show resolved Hide resolved
test/websocket.misc.js Outdated Show resolved Hide resolved
test/websocket.misc.js Outdated Show resolved Hide resolved
test/websocket.misc.js Outdated Show resolved Hide resolved
@cgewecke cgewecke changed the title Add websocket / misc tests Add websocket / misc tests (#3190) Jan 11, 2020
cgewecke
cgewecke commented Jan 14, 2020
View reviewed changes
Copy link
Collaborator Author

@cgewecke cgewecke left a comment

Choose a reason for hiding this comment

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

@nivida Made the recommended changes (+/-) and also added a couple tests for mid-chunk parsing errors in 7ad8d34

nivida
nivida approved these changes Jan 15, 2020
View reviewed changes
Copy link
Contributor

@nivida nivida left a comment

Choose a reason for hiding this comment

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

I've improved the code style consistency (missing semicolons and not consistent formatting of functions)

@nivida nivida merged commit 7b6a389 into provider-related-improvements Jan 15, 2020
@nivida nivida deleted the pri/testing-gaps branch January 16, 2020 09:06
@snyk-bot snyk-bot mentioned this pull request Sep 21, 2022
Open
@adamlaska adamlaska mentioned this pull request Oct 2, 2022
Open
@Woodpile37 Woodpile37 mentioned this pull request Aug 19, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nivida nivida nivida approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@cgewecke @coveralls @nivida