codebuddies / backend

CodeBuddies back-end
https://codebuddies.org
GNU General Public License v3.0
20 stars 25 forks source link

[API Projects] "Model" tests for /projects endpoint #179

Closed BethanyG closed 3 years ago

BethanyG commented 3 years ago

Issues #80 and #81 (closed PRs #170 and #169) were approved and merged without tests being written for the code. This was not supposed to happen. Tests now need to be written for those two issues/PRs before going further with this endpoint.

Adding these links, in case they're helpful:

lpatmo commented 3 years ago

@watchtheblur @BethanyG Test makes sense for me re: API endpoints, but I don't fully understand what we would be testing for #169 and #170, which is why I didn't ask @watchtheblur to write any (I thought they weren't necessary).

169 involves a single startapp command that creates a bunch of files that are standard to Django, so am not sure what we would test.

170 creates a model, and from my reading of this Obey the Testing Goat chapter on database layer validation I understood that it makes sense to test that validation works as intended, but the model fields that we introduce are pretty simple.

... OK, in an effort to figure out what I'm missing about testing models, I skimmed the earlier chapters of the book again, and found this example:

from lists.models import Item
[...]

class ItemModelTest(TestCase):

    def test_saving_and_retrieving_items(self):
        first_item = Item()
        first_item.text = 'The first (ever) list item'
        first_item.save()

        second_item = Item()
        second_item.text = 'Item the second'
        second_item.save()

        saved_items = Item.objects.all()
        self.assertEqual(saved_items.count(), 2)

        first_saved_item = saved_items[0]
        second_saved_item = saved_items[1]
        self.assertEqual(first_saved_item.text, 'The first (ever) list item')
        self.assertEqual(second_saved_item.text, 'Item the second')

... and that makes a lot of sense to me! So I imagine we could write an OSProject model test that looks something like:

//projects/tests.py

from .models import OSProjects

class OSProjectsModelTest(TestCase):

    def test_saving_and_retrieving_items(self):
        first_project = OSProjects()
        first_project.title = 'The first (ever) project'
        first_project.save()

        second_project = OSProjects()
        second_project.title = 'Project the second'
        second_project.save()

        saved_projects = OSProjects.objects.all()
        self.assertEqual(saved_projects.count(), 2)

        first_saved_project = saved_projects[0]
        second_saved_project = saved_projects[1]
        self.assertEqual(first_saved_project.text, 'The first (ever) project')
        self.assertEqual(second_saved_project.title, 'Project the second')

... but with a few more fields included, and maybe a deletion as well.

BethanyG commented 3 years ago

@lpatmo -- agree that #169 is not really worth writing a test for. For some reason, I was thinking that one included adding in the app to settings and or registering it with the framework. But it doesn't -- it is only creating boilerplate. So yeah -- no tests needed.

On the other hand (as you can see from issue #98 assigned to me) I do think model tests are useful. And I suspect they'll be even more useful when we get into situations where we are making changes to a model (because then the tests should alert us a change has been made by breaking). I don't think they need to be elaborate -- but we should do some. I am toying with trying out the django-test-migrations with the pytest plugin, but I want to get the registration flows to a point where they are cleaner and we can decide on a direction first. But we could maybe collaborate on trying it out if you'd like.

lpatmo commented 3 years ago

👀 Ohhh, you put some great links into #98! That first one there (https://medium.com/@dwernychukjosh/testing-models-with-django-using-pytest-and-factory-boy-a2985adce7b3) makes a lot of sense, thanks.

I feel like I need to finish up GH Actions deployment first and get a handle on writing tests and serializers for the projects API endpoint before looking into https://pypi.org/project/django-test-migrations/, but I can see it being useful when we get to the stage of doing a bunch more design/API iteration.

lpatmo commented 3 years ago

@watchtheblur Partly inspired by that testing models article, I started writing out some factories tests for our models.py file in this branch: issue-179-model-tests

The diff so far: https://github.com/codebuddies/backend/compare/issue-179-model-tests?expand=1

Some todos:

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

lpatmo commented 3 years ago

Forgot to close this issue after merging in https://github.com/codebuddies/backend/pull/184