-
Notifications
You must be signed in to change notification settings - Fork 154
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.
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
chore: Bump socket.io/socket.io-client from ^2 to ^4 #487
base: main
Are you sure you want to change the base?
chore: Bump socket.io/socket.io-client from ^2 to ^4 #487
Conversation
New, updated, and removed dependencies detected. Learn more about Socket for GitHub ↗︎
Have feedback? Participate in our User Experience Survey 📊 |
jest.config.js
Outdated
functions: 96.25, | ||
lines: 90.85, | ||
lines: 90.0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is worrisome.
Which lines are we no longer testing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@styfle
I was under the impression that the socket.io/socket.io-client version 4 has a smaller footprint and hence the value for lines had to be adapted - it could be the case that a treshold between 90.0 and 90.85 is the correct value
So it now fails with:
Jest: "global" coverage threshold for lines (90.85%) not met: 90.32%
So 90.32 could be the new "correct" value - or?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The coverage is supposed to be for the src
directory. Changing the dependencies shouldn't change coverage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No - but seems to have had an impact - nevertheless
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found this faulty path - 'socket.io/lib/index.js' which now is 'socket.io/dist/index.js'
But still the values had to be adapted - maybe the case is special-cases.ts must be adapted - nevertheless with the new path some values still had to bed adapted
Resolves #486