Actually I should put the tests in a new class with a tearDown that
removes extra directories - it is called even if there is an
exception.
On 22 July 2013 16:31, James Tunnicliffe <email address hidden> wrote:
> On 22 July 2013 16:04, Milo Casagrande <email address hidden> wrote:
>> Hi James,
>>
>> overall it looks good, a couple of suggestion follows.
>>
>> On Wed, Jul 17, 2013 at 5:29 PM, James Tunnicliffe
>> <email address hidden> wrote:
>>>
>>> + def test_get_public_key_post_and_get_file(self):
>>> + response = self.client.get("http://testserver/api/request_key",
>>> + data={"key": settings.MASTER_API_KEY,
>>> + "public": ""})
>>> +
>>> + self.assertEqual(response.status_code, 200)
>>> + # Don't care what the key is, as long as it isn't blank
>>> + self.assertRegexpMatches(response.content, "\S+")
>>> + key = response.content
>>> +
>>> + # Now write a file so we can upload it
>>> + file_content = "test_get_key_post_and_get_file"
>>> + file_root = "/tmp"
>>> +
>>> + tmp_file_name = os.path.join(
>>> + file_root,
>>> + self.make_temporary_file(file_content))
>>> +
>>> + buildinfo_content = "\n".join([
>>> + "Format-Version: 0.1",
>>> + "Files-Pattern: *",
>>> + "Build-Name: test",
>>> + "License-Type: open"])
>>> + tmp_build_info = os.path.join(
>>> + file_root,
>>> + self.make_temporary_file(buildinfo_content))
>>> +
>>> + # Send the files
>>> + with open(tmp_file_name) as f:
>>> + response = self.client.post("http://testserver/pub/file_name",
>>> + data={"key": key, "file": f})
>>> + self.assertEqual(response.status_code, 200)
>>> +
>>> + with open(tmp_build_info) as f:
>>> + response = self.client.post("http://testserver/pub/BUILD-INFO.txt",
>>> + data={"key": key, "file": f})
>>> + self.assertEqual(response.status_code, 200)
>>> +
>>> + # Check the upload worked by reading the file back from its uploaded
>>> + # location
>>> + uploaded_file_path = os.path.join(settings.SERVED_PATHS[0],
>>> + "file_name")
>>> +
>>> + with open(uploaded_file_path) as f:
>>> + self.assertEqual(f.read(), file_content)
>>> +
>>> + # Test we can fetch the newly uploaded file
>>> + response = self.client.get("http://testserver/pub/file_name")
>>> + self.assertEqual(response.status_code, 200)
>>> +
>>> + # Delete the files generated by the test
>>> + shutil.rmtree(os.path.join(settings.SERVED_PATHS[0], "pub"))
>>
>>
>> Wouldn't it be better to have all the logic of this test method inside
>> a try-finally?
>> I'm concerned with an error that can happen, for whatever reason,
>> somewhere before the call to shutil.rmtree, leaving cruft on the file
>> system.
>
> Good point. Will clean that up.
>
>>> === modified file 'license_protected_downloads/uploads.py'
>>>
>>>
>>> @@ -41,11 +44,14 @@
>>> APIKeyStore.objects.filter(key=request.POST["key"])):
>>> return HttpResponseServerError("Invalid key")
>>>
>>> + api_key = APIKeyStore.objects.filter(key=request.POST["key"])
>>> +
>>
>> Does the call to APIKeyStore.objects.filter get cached in some way?
>> The problem is that the other solution I have in mind means to always
>> call APIKeyStore.objects.filter even if "key in request.POST" is
>> False.
>
> You couldn't make the call - if request.POST["key"] didn't exist, it
> would raise an exception. If you had no filter for the key value, the
> filter would match everything, which would be a bad thing.
>
> --
> James Tunnicliffe
Actually I should put the tests in a new class with a tearDown that
removes extra directories - it is called even if there is an
exception.
On 22 July 2013 16:31, James Tunnicliffe <email address hidden> wrote: public_ key_post_ and_get_ file(self) : testserver/ api/request_ key", MASTER_ API_KEY, l(response. status_ code, 200) xpMatches( response. content, "\S+") key_post_ and_get_ file" temporary_ file(file_ content) ) temporary_ file(buildinfo_ content) ) testserver/ pub/file_ name", l(response. status_ code, 200) build_info) as f: testserver/ pub/BUILD- INFO.txt", l(response. status_ code, 200) join(settings. SERVED_ PATHS[0] , file_path) as f: l(f.read( ), file_content) testserver/ pub/file_ name") l(response. status_ code, 200) rmtree( os.path. join(settings. SERVED_ PATHS[0] , "pub")) protected_ downloads/ uploads. py' objects. filter( key=request. POST["key" ])): verError( "Invalid key") objects. filter( key=request. POST["key" ]) objects. filter get cached in some way? objects. filter even if "key in request.POST" is
> On 22 July 2013 16:04, Milo Casagrande <email address hidden> wrote:
>> Hi James,
>>
>> overall it looks good, a couple of suggestion follows.
>>
>> On Wed, Jul 17, 2013 at 5:29 PM, James Tunnicliffe
>> <email address hidden> wrote:
>>>
>>> + def test_get_
>>> + response = self.client.get("http://
>>> + data={"key": settings.
>>> + "public": ""})
>>> +
>>> + self.assertEqua
>>> + # Don't care what the key is, as long as it isn't blank
>>> + self.assertRege
>>> + key = response.content
>>> +
>>> + # Now write a file so we can upload it
>>> + file_content = "test_get_
>>> + file_root = "/tmp"
>>> +
>>> + tmp_file_name = os.path.join(
>>> + file_root,
>>> + self.make_
>>> +
>>> + buildinfo_content = "\n".join([
>>> + "Format-Version: 0.1",
>>> + "Files-Pattern: *",
>>> + "Build-Name: test",
>>> + "License-Type: open"])
>>> + tmp_build_info = os.path.join(
>>> + file_root,
>>> + self.make_
>>> +
>>> + # Send the files
>>> + with open(tmp_file_name) as f:
>>> + response = self.client.post("http://
>>> + data={"key": key, "file": f})
>>> + self.assertEqua
>>> +
>>> + with open(tmp_
>>> + response = self.client.post("http://
>>> + data={"key": key, "file": f})
>>> + self.assertEqua
>>> +
>>> + # Check the upload worked by reading the file back from its uploaded
>>> + # location
>>> + uploaded_file_path = os.path.
>>> + "file_name")
>>> +
>>> + with open(uploaded_
>>> + self.assertEqua
>>> +
>>> + # Test we can fetch the newly uploaded file
>>> + response = self.client.get("http://
>>> + self.assertEqua
>>> +
>>> + # Delete the files generated by the test
>>> + shutil.
>>
>>
>> Wouldn't it be better to have all the logic of this test method inside
>> a try-finally?
>> I'm concerned with an error that can happen, for whatever reason,
>> somewhere before the call to shutil.rmtree, leaving cruft on the file
>> system.
>
> Good point. Will clean that up.
>
>>> === modified file 'license_
>>>
>>>
>>> @@ -41,11 +44,14 @@
>>> APIKeyStore.
>>> return HttpResponseSer
>>>
>>> + api_key = APIKeyStore.
>>> +
>>
>> Does the call to APIKeyStore.
>> The problem is that the other solution I have in mind means to always
>> call APIKeyStore.
>> False.
>
> You couldn't make the call - if request.POST["key"] didn't exist, it
> would raise an exception. If you had no filter for the key value, the
> filter would match everything, which would be a bad thing.
>
> --
> James Tunnicliffe
--
James Tunnicliffe