Tests: Refactors test_urlrequest.py #6951
Conversation
AndreMiras
commented
Jun 17, 2020
AndreMiras
commented
- uses @pytest.mark.skipif() rather than return
- couple of DRY fixes using helper functions
|
|
||
| def __init__(self, queue): | ||
| super(UrlRequestQueue, self).__init__() |
There was a problem hiding this comment.
Removed because we don't inherit from anything
|
|
||
| queue = [] |
There was a problem hiding this comment.
The queue is already defined in the init, less is more. Having it defined here can lead to the misleading assumption we want it to be static attribute
| timed_out = False | ||
| while not request.is_finished and not timed_out: | ||
| Clock.tick() | ||
| sleep(.1) |
There was a problem hiding this comment.
reduced the sleep time from .5 to .1 because nobody likes to wait :)
|
A small test refactoring before addressing #6946, also refs https://discord.com/channels/423249981340778496/490493814281338890/722883388889104446 |
kivy/tests/test_urlrequest.py
Outdated
| from time import sleep | ||
| from base64 import b64encode | ||
| import os | ||
|
|
||
| if not os.environ.get('NONETWORK'): | ||
| from kivy.network.urlrequest import UrlRequest | ||
| from kivy.clock import Clock |
There was a problem hiding this comment.
Can you use the kivy_clock fixture and pass it in, instead of importing it?
Also, I don't like when we import kivy things globally in tests because then the kivy stuff is initialized before the tests start because kivy imports are so aggressive. It'd be better if you just do from kivy.network.urlrequest import UrlRequest in each test.
There was a problem hiding this comment.
I've updated the code as per your suggestion.
Out of curiosity, regarding kivy imports being aggressive. Dont't we have a deeper problem we want to fix one day? Or is it a feature that can't be fixed or that we don't want to fix?
There was a problem hiding this comment.
Yeah, we do have a very deep problem there that I hope will be fixed one day (the sooner the better). So this is just in preparation for that bright sunny morning :D
- uses @pytest.mark.skipif() rather than return - couple of DRY fixes using helper functions - uses the kivy_clock fixture
AndreMiras
force-pushed
the
feature/test_urlrequest_refactor
branch
from
c934516
to
cdb8956
Compare
|
Addressed all the comments, including:
It's commit emoji, https://gitmoji.carloscuesta.me/ I assume you didn't like it, so I removed it 😄 |
|
Oh, it didn't render as a ♻️ in the title so I was confused by what it was supposed to be. |
|
Yes, BTW, I just see the merge message picked the GitHub title rather than the actual commit title. So now we have a merge commit with the icon 🤦♂️ |
matham
changed the title
