Repository navigation
Prefer pytest-style tests (issue #861) - #1862
Conversation
mfonism
changed the title
Codecov Report
@@ Coverage Diff @@
## master #1862 +/- ##
==========================================
- Coverage 64.44% 64.44% -0.01%
==========================================
Files 200 200
Lines 16033 15962 -71
==========================================
- Hits 10333 10287 -46
+ Misses 5700 5675 -25
Continue to review full report at Codecov.
|
mfonism
changed the title
|
Thank you @mfonism. Overall looks good, but this will need careful review which would take a bit of a time. |
ceb8
left a comment
ceb8
left a comment
There was a problem hiding this comment.
Looks good to me. I had one suggestion about cleaning up strings but all the functionality looks good.
| hostUrl = host + ":" + str(connPort) + "/" + serverContext + "/" \ | ||
| + tapContext + "/" |
There was a problem hiding this comment.
While you're updating these tests, let's also switch these to formatted strings. I.e. "{}:{}/{}/{}".format(host, connPort, serverContext, tapContext).
This goes for all the other similar stings in this file.
There was a problem hiding this comment.
I would even suggest to use fstrings, those are even easier to read.
There was a problem hiding this comment.
Oooh, yes! I forgot our earliest supported python version has those.
F-strings are the best: f"{host}:{connPort}/{serverContext}/{tapContext}"
bsipocz
added
hacktoberfest-accepted
bsipocz
left a comment
bsipocz
left a comment
There was a problem hiding this comment.
I've reviewed the last commit about the strings, and will use the previous approval for the test refactor for merging.
|
Oh, wow, you changed all the strings 🎉 |
|
So, this is a massive diff, I gave a review for the string changes and didn't spot any issues. And @ceb8 has already approved the test refactors. So this is ready to go now. |
|
Massive thank you @mfonism! |
This PR converts xunit-style style tests to pytest-style tests as per issue #861