Rewrite application Tests using Factory-Boy #116

chris48s · 2020-03-25T20:47:14Z

Closes #48

Done so far:

Rewritten the tests in the userauth app to use the UserFactory (the user app tests generated by cookiecutter already had a UserFactory class) instead of fixtures
Written a CustomTagFactory class and sketched out what the TaggedItemsFactory needs to look like

The remaining tasks are:

Write a ResourceFactory class and finish off TaggedItemsFactory
Rewrite the tests for the resources app to use the tag/resource factories instead of fixtures.

We might also look at generating init data (#115) in this PR, or we could leave it for another PR.

@BethanyG - I'll mark up the diff with some notes explaining what I've done so far a bit more. I'll leave it with you to have a look at this PR, work on the ResourceFactory and resources tests and push some more commits to this branch.

We'll use our factories to seed the DB with fake data as well as testing lets declare our factories in the app dir instead of under /tests

chris48s

I can't leave a comment inine for this becuase there's no diff for /users/factories.py, but we don't need to define factory fields on fields that already have a default. Our class

class UserFactory(DjangoModelFactory):
    username = Faker("user_name")
    email = Faker("email")
    name = Faker("name")

    @post_generation
    def password(self, create: bool, extracted: Sequence[Any], **kwargs):
        # bla

is functionally the same as

class UserFactory(DjangoModelFactory):
    username = Faker("user_name")
    email = Faker("email")
    name = Faker("name")

    @post_generation
    def password(self, create: bool, extracted: Sequence[Any], **kwargs):
        # bla

    is_staff = False
    is_active = True
    date_joined = datetime.datetime.now()
    last_login = None

because of the model defaults on is_staff, etc. We can still explicitly overide them if we need to create an admin user, or fake a user who has already logged in, or whatever..

project/userauth/tests.py

project/tagging/factories.py

mayela · 2020-03-27T05:05:37Z

Hi @chris48s and @BethanyG. I'm Mar. I added the ResourceFactory, hope you can review it. Thanks in advance

chris48s

Thanks for working on this. I've left a couple of comments inline

project/resources/factories.py

codecov · 2020-03-30T05:02:35Z

Codecov Report

Merging #116 into master will decrease coverage by 2.86%.
The diff coverage is 51.35%.

@@            Coverage Diff             @@
##           master     #116      +/-   ##
==========================================
- Coverage   78.88%   76.02%   -2.87%     
==========================================
  Files          25       28       +3     
  Lines         341      392      +51     
==========================================
+ Hits          269      298      +29     
- Misses         72       94      +22

Impacted Files	Coverage Δ
project/tagging/factories.py	`0.00% <0.00%> (ø)`
project/users/factories.py	`100.00% <ø> (ø)`
project/resources/factories.py	`100.00% <100.00%> (ø)`
project/tagging/serializers.py	`83.33% <0.00%> (-8.34%)`	⬇️
project/resources/views.py	`68.18% <0.00%> (-4.55%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 22b14e3...8cbfdac. Read the comment docs.

mayela · 2020-03-30T17:46:17Z

Thanks for your comments @chris48s. Today I'm going to review each one and resolve it. Nice day! 😊

chris48s · 2020-04-02T19:16:02Z

There's one remaining task to finish off this PR, which is to rewrite the resources tests to use the ResourceFactory added by @mayela . Are you still planning to pick this up @BethanyG ?

BethanyG · 2020-04-02T19:45:29Z

@chris48s - I am. I have just been snowed under. Should be able to get to it before the weekend.

chris48s · 2020-04-02T19:47:24Z

No pressure :)

mayela · 2020-04-02T23:58:04Z

@BethanyG I want to help with tests! How we can work together?

mayela · 2020-04-03T17:41:03Z

@chris48s Sorry, I didn't notice I closed the PR! 😓

BethanyG · 2020-04-05T01:52:02Z

@mayela - We could always use more tests and more testing coverage!! I'll open an issue, and we can go from there.... also -- you can always open issues here in the repo for things you think we need to put in place, and if it's work you want to work on..just assign the issue to yourself. We can always discuss details/use cases as we go. 😄

I went ahead and re-wrote the existing (and sorta pathetic) resources tests so we can get this PR closed...but we can (and need to) add more tests and scenarios around user auth, resources, tagging, and of course, integration tests. You can also touch base with @chris48s and see if there are things on his mind you could help out with as well.

BethanyG · 2020-04-05T02:00:03Z

@lpatmo - adding you here as a reviewer because I've just done a push..so need additional eyes on this. @chris48s - I cannot seem to add you as a reviewer, or I would have. 😉 .

lpatmo · 2020-04-05T02:21:36Z

Thanks for tagging me; I will take a look!

lpatmo

Thanks so much, @chris48s -- I admittedly had to look up https://factoryboy.readthedocs.io/en/latest/, but this LGTM!

Props to @BethanyG for filing #125.

chris48s

Nearly there on this one - team effort :)

It looks like your build is failing with AssertionError: '' != 'WEB'

Is that due to #125 ?
If so and you don't want to address the bug in this PR, how about we comment the assertion or use the same technique as I did in 49eca65 and keep the test but mark it as a skip() or expectedFailure() for now?

chris48s · 2020-04-05T13:55:07Z

project/resources/tests.py

        response = self.client.patch(url, data, format='json')
        self.assertEqual(response.status_code, status.HTTP_200_OK)
        self.assertEqual(response.data['description'], data['description'])
-        self.assertEqual(response.data['tags'][0]["name"], data['tags'][0])


What happened to the assertions about tags here? (not necessarily wrong to bin them - its just not explained anywhere, I don't think)

Yup. That's due to #125 😄 . I didn't even think to do a skip -- I'll add that in, I like how you did it with the issue ref in 49eca65 .

as for binning the tags..I was solving an issue with the ordering being different coming back in data .. and then decided I didn't want to. But I should do at least one test case with tags, so I can add it back in.

chris48s · 2020-04-05T13:56:35Z

project/resources/tests.py

-        url = '/api/v1/resources/96eec6e2-59f4-11ea-9149-dca9047779fe/'
+        records = create_batch(ResourceFactory, 10)
+        new_resource = create(ResourceFactory, guid='96f4dd16-59f4-11ea-9149-dca9047779fe')
+        url = f'/api/v1/resources/{new_resource.guid}/'


Because you're using new_resource.guid, we don't actually need to hard-code the guid. This could just be:

new_resource = create(ResourceFactory) url = f'/api/v1/resources/{new_resource.guid}/'

Also, same comment for test_view_one_resource()

Good point. Dunno what I was thinking there...I did that elsewhere, but brain farted here. Fixed in the new push. Thank for catching!

chris48s · 2020-04-05T13:59:18Z

I cannot seem to add you as a reviewer,

I think its because I opened the PR: I can't approve my own PR (even though 3 of us have now pushed to the branch)

…ses, added tag test back into patch_one_resource.

BethanyG · 2020-04-06T00:11:22Z

@chris48s - do you wanna have one last look and then push the big, green button? Thank you so much for your patience and work on this!. 🌟

chris48s · 2020-04-06T13:33:33Z

LGTM 👍
We can move on to #115 now :)

chris48s added 5 commits March 25, 2020 19:57

move existing UserFactory

db251ca

We'll use our factories to seed the DB with fake data as well as testing lets declare our factories in the app dir instead of under /tests

rewrite userauth tests using UserFactory instead of fixtures

79703b8

add CustomTagFactory

fd9d67b

add a TODO comment for TaggedItemsFactory

a946cfd

resource factories will go here

92735fd

chris48s commented Mar 25, 2020

View reviewed changes

project/userauth/tests.py Show resolved Hide resolved

project/tagging/factories.py Show resolved Hide resolved

project/tagging/factories.py Outdated Show resolved Hide resolved

project/tagging/factories.py Show resolved Hide resolved

mayela added 2 commits March 26, 2020 17:51

Add ResourceFactory

5aa9bd7

Add TaggedItemsFactory

9a500a0

chris48s commented Mar 27, 2020

View reviewed changes

project/resources/factories.py Outdated Show resolved Hide resolved

project/resources/factories.py Outdated Show resolved Hide resolved

project/resources/factories.py Show resolved Hide resolved

chris48s mentioned this pull request Mar 29, 2020

add tests for tag slugging #121

Merged

Merge branch 'master' into issue48

9cf9536

mayela added 2 commits March 30, 2020 16:23

Fix typo

7868387

Refactor RESOURCE_TYPES on factory file

b96bb2d

mayela closed this Apr 2, 2020

chris48s reopened this Apr 3, 2020

BethanyG added 2 commits April 4, 2020 18:33

Re-wrote the existing Resources tests to use resources.factories.

4cb2b84

Removed unused import statements.

2d55b54

BethanyG requested a review from lpatmo April 5, 2020 01:59

lpatmo approved these changes Apr 5, 2020

View reviewed changes

chris48s commented Apr 5, 2020

View reviewed changes

chris48s changed the title ~~WIP: Rewrite application Tests using Factory-Boy~~ Rewrite application Tests using Factory-Boy Apr 5, 2020

chris48s marked this pull request as ready for review April 5, 2020 13:59

Added skp for creat_one_resource, removed hard coded guids in test ca…

8cbfdac

…ses, added tag test back into patch_one_resource.

chris48s merged commit 1ad65a9 into master Apr 6, 2020

chris48s mentioned this pull request Apr 6, 2020

[Testing] [WIP] Use Factory-Boy to Create Seed Data for Dev and Staging DBs #115

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrite application Tests using Factory-Boy #116

Rewrite application Tests using Factory-Boy #116

chris48s commented Mar 25, 2020 •

edited by BethanyG

Loading

chris48s left a comment

mayela commented Mar 27, 2020

chris48s left a comment

codecov bot commented Mar 30, 2020 •

edited

Loading

mayela commented Mar 30, 2020

chris48s commented Apr 2, 2020

BethanyG commented Apr 2, 2020

chris48s commented Apr 2, 2020

mayela commented Apr 2, 2020

mayela commented Apr 3, 2020

BethanyG commented Apr 5, 2020

BethanyG commented Apr 5, 2020

lpatmo commented Apr 5, 2020

lpatmo left a comment

chris48s left a comment •

edited

Loading

chris48s Apr 5, 2020

BethanyG Apr 5, 2020

BethanyG Apr 5, 2020

chris48s Apr 5, 2020

BethanyG Apr 5, 2020

chris48s commented Apr 5, 2020

BethanyG commented Apr 6, 2020

chris48s commented Apr 6, 2020

Rewrite application Tests using Factory-Boy #116

Rewrite application Tests using Factory-Boy #116

Conversation

chris48s commented Mar 25, 2020 • edited by BethanyG Loading

chris48s left a comment

Choose a reason for hiding this comment

mayela commented Mar 27, 2020

chris48s left a comment

Choose a reason for hiding this comment

codecov bot commented Mar 30, 2020 • edited Loading

Codecov Report

mayela commented Mar 30, 2020

chris48s commented Apr 2, 2020

BethanyG commented Apr 2, 2020

chris48s commented Apr 2, 2020

mayela commented Apr 2, 2020

mayela commented Apr 3, 2020

BethanyG commented Apr 5, 2020

BethanyG commented Apr 5, 2020

lpatmo commented Apr 5, 2020

lpatmo left a comment

Choose a reason for hiding this comment

chris48s left a comment • edited Loading

Choose a reason for hiding this comment

chris48s Apr 5, 2020

Choose a reason for hiding this comment

BethanyG Apr 5, 2020

Choose a reason for hiding this comment

BethanyG Apr 5, 2020

Choose a reason for hiding this comment

chris48s Apr 5, 2020

Choose a reason for hiding this comment

BethanyG Apr 5, 2020

Choose a reason for hiding this comment

chris48s commented Apr 5, 2020

BethanyG commented Apr 6, 2020

chris48s commented Apr 6, 2020

chris48s commented Mar 25, 2020 •

edited by BethanyG

Loading

codecov bot commented Mar 30, 2020 •

edited

Loading

chris48s left a comment •

edited

Loading