Fix empty file uploads #78
Conversation
tus-js-client and tusd support uploads of empty files. In these cases, only the initial POST request needs to be performed: tus/tus-js-client#106 This was supported by 0.2.5, but the refactoring inadvertently broke this by skipping by all HTTP requests entirely.
tests/test_async_uploader.py
Outdated
| def test_upload_empty(self): | ||
| with aioresponses() as resps: | ||
| resps.post( | ||
| "http://tusd.tusdemo.net/files/", status=200, |
There was a problem hiding this comment.
Could you use self.url here instead of tusd.tusdemo.net directly?
There was a problem hiding this comment.
Not self.url (it's hardcoded to an existing upload), but self.client.url contains an URL to a tus upload endpoint and we can use that instead.
Since we're practically mocking all the HTTP requests in this scenario, it would probably be better to use a fake URL we know that doesn't correspond to a real service (eg. mock-tus.localdomain), since we want the test to fail instead of contacting a real endpoint. But I'm using the same URL for consistency's sake here.
tests/test_async_uploader.py
Outdated
|
|
||
| # Set uploader's URL to None. This corresponds to a new upload; | ||
| # if the URL is set we're continuing an existing upload. | ||
| self.async_uploader.url = None |
There was a problem hiding this comment.
Is it necessary to set url = None here? I thought we are creating a new uploader for each test, so the url should not be initialized here. Or am I missing something?
There was a problem hiding this comment.
It's not actually necessary, and it's a better approach to create an uploader from the existing self.client fixture. I've refactored the tests to do so.
It's more reliable and concise to create an uploader from the existing client fixture instead of modifying an existing uploader to resemble what we want.
|
I've refactored the tests in addd94a per code review comments. If that looks OK, I can squash the two test commits to make the history a bit cleaner. |
tus-js-client and tusd support uploads of empty files. In
these cases, only the initial POST request needs to be performed:
tus/tus-js-client#106
This was supported by 0.2.5, but the refactoring inadvertently broke
this by skipping by all HTTP requests entirely.