[PATCH v6 06/10] REST: Add Patches to the API

[Andy Doan]

On 06/19/2016 01:19 PM, Finucane, Stephen wrote:
> On 16 Jun 16:13, Andy Doan wrote:
>> This exposes patches via the REST API.
>>
>> Security Constraints:
>>   * Anyone (logged in or not) can read all objects.
>>   * No one can create/delete objects.
>>   * Project maintainers are allowed to update (ie "patch"
>>     attributes)
>>
>> NOTE: Patch.save was overridden incorrectly and had to be
>> fixed to work with DRF.
>>
>> Signed-off-by: Andy Doan <andy.doan at linaro.org>
>
> There's an issue with one of the tests below when I run the entire
> test suite as one...
>
>> +    def test_detail_tags(self):
>> +        patches = create_patches()
>> +        patches[0].content = 'Reviewed-by: Test User <test at example.com>\n'
>> +        patches[0].save()
>> +        patches[0].refresh_tag_counts()
>
> ^ I don't think this is needed (it's called in 'save'), but more
> importantly...
>
>> +        resp = self.client.get(self.api_url(patches[0].id))
>> +        tags = resp.data['tags']
>> +        self.assertEqual(1, len(tags))
>
> The above assertion fails when I run the entire test suite, like so:
>
>      tox -e py27
>
> This doesn't seem to be storing tags correctly.
>
>      ======================================================================
>      FAIL: test_detail_tags (patchwork.tests.test_rest_api.TestPatchAPI)
>      ----------------------------------------------------------------------
>      Traceback (most recent call last):
>        File "/home/sfinucan/innovation/patchwork/patchwork/patchwork/tests/test_rest_api.py", line 274, in test_detail_tags
>          self.assertEqual(1, len(tags))
>      AssertionError: 1 != 0
>
> I'm guessing it's due to some side effect, but I haven't figured out
> what's causing this yet. Mind taking a look?

Weird issue, and I'm not positive you'll like this workaround:

      def test_detail_tags(self):
+        # defaults.project is remembered between TestCases and .save()
+        # is called which just updates the object. This leaves the
+        # potential for the @cached_property project.tags to be
+        # invalid, so we have to invalidate this cached value before
+        # doing tag operations:
+        del defaults.project.tags
          patches = create_patches()

So the workaround is really just addressing a symptom of the test-suite: 
We don't actually delete/re-create the project between runs, so it can 
cache bad stuff. I'm fine with adding this workaround to the patch. I 
think doing a "real fix" would be much more systemic and beyond the 
scope of this work.