-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Fix incorrect asserts in test and ensure connections are closed #3004
Fix incorrect asserts in test and ensure connections are closed #3004
Conversation
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #3004 +/- ##
==========================================
- Coverage 91.84% 91.83% -0.01%
==========================================
Files 128 128
Lines 33232 33283 +51
==========================================
+ Hits 30523 30567 +44
- Misses 2709 2716 +7 ☔ View full report in Codecov by Sentry. |
a44028d
to
4cc5559
Compare
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.
Just to my understanding - why is this the correct way to test it?
4cc5559
to
f62e35b
Compare
f62e35b
to
d928c9c
Compare
@kristjanvalur I think you missed my question here, if you can, I would like to understand it. |
I did indeed, sorry :) with pytest.raises(ConnectionError) as e:
sslclient.ping()
assert "SSL: CERTIFICATE_VERIFY_FAILED" in str(e) The above assert will never be hit. sslclient.ping() raises a connection error and the see https://docs.pytest.org/en/7.1.x/reference/reference.html?highlight=raises#pytest.raises |
@kristjanvalur Sorry, I missed this pr. can you merge master in so I'll can merge it? |
seems unit tests runners are shaky again. |
Pull Request check-list
Please make sure to review and check all of these items:
NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.
Description of change
Various error message assertions were incorrectly placed and were not testing anything.
In addition, sockets were being leaked in the tests.