Expand Views testing for backend

[andrewtavis]

Terms

• [x] I have searched open and closed feature requests
• [x] I agree to follow activist's Code of Conduct

Description

In #1121 and #1125 we added in a coverage report to the backend PR workflow. One thing that became apparent from that is that our testing for Views in the backend is quite low. Here's the current output for the View files in the backend:

backend/authentication/views.py                       90     13    86%   84-97, 194-204
backend/communities/groups/views.py                   92     65    29%   29-31, 34-39, 42-50, 53-74, 77-101, 104-126, 136-169
backend/communities/organizations/views.py            97     68    30%   36-38, 41-47, 50-61, 64-87, 90-113, 116-142, 152-185
backend/communities/views.py                           8      0   100%
backend/content/views.py                             153    113    26%   27-35, 41-52, 55-65, 71-81, 84-95, 101-110, 119-126, 132-155, 158-167, 170-181, 184-195, 198-207, 220-228, 234-245, 248-258, 261-272, 275-286, 289-299
backend/events/views.py                               96     69    28%   28-30, 33-38, 41-49, 52-73, 78-98, 103-122, 132-165

I'm expecting that the tests to expand the coverage of these files will be fairly similar, hence putting it all into a single issue. We can also break this up into file based PRs if that would be easier for the contributing :)

Contribution

Happy to discuss the implementation here and review a PR once it's opened! I can also pick this up eventually if need be 😊

[sh-ran]

Just wanted to ask if I had to write the remaining test cases in order to get a 100% coverage for each of the file.

[andrewtavis]

Ideally 100%, but if you want to expand out the tests a bit with an initial PR that would be totally fine, @sh-ran! Feel free to send something along for your first try and we'll go from there :)

[mattburnett-repo]

Hi there @sh-ran , @andrewtavis 👋

Just wanted to mention that there are already some model, serializer and view tests for the content app (image upload and social media links) in the backend/content/tests folder. More test coverage is always better, though, so please feel free to add to the coverage for these things if you wish!

[sh-ran]

@mattburnett-repo I had a doubt with this.

How does partial update differ from update? Just wanted to make sure if both have different usages since both the methods have the same lines of code. If they don't differ much can i just use the test cases I have used for update method.

Thank you!

[mattburnett-repo]

Hi @sh-ran ! Which files are these code examples in? I want to be certain that I see the same code / problem that you do :)

To directly answer your question,

update: Replaces all fields in a record that already exists. It is like the PUT method in http. I think of it like a POST method, except for a record that already exists.

partial_update: Only replaces fields that are in the request. It is like the PATCH method in http.

Let me know the files you are looking at, and I can give you more answers.

[sh-ran]

Hello! @mattburnett-repo

The file I am talking about is in

backend/communities/groups/views.py

There under GroupViewsClass you can find both update and partial_update methods.

I had my desktop switched off. I hope this is ok. I can reach out to you tomorrow. I apologise for the inconveniences.

And thank you for clarifying my doubt!!

[mattburnett-repo]

You're very welcome, @sh-ran ! No inconvenience at all. Please ask questions; in the activist community we try to be supportive and helpful, as much as we are able.

Do you know about our video meeting (dev sync) tomorrow? There is information about this meeting in the Element chat, in the 'Development' room, posted by activist-bot. We have this meeting every other Saturday, and it is a great place to meet others on the team, and ask questions.

About your question: Knowing now, that the 'update' and 'partial_update' are in the same view file, I would suggest that in the '../group/views.py' test file you write tests that check both the PUT (update) and PATCH (partial_update) methods, one test for PUT and one test for PATCH. I will give you room to decide how to best do that, but of course if you have questions feel free to ask.

[sh-ran]

Thank you for clarifying my doubt @mattburnett-repo

I have been writing the tests for each method in the tests folder within the path you mentioned with separate files for better readability. As for the meeting I am looking forward to it. I'll see you guys tonight!

[sh-ran]

Hello! @andrewtavis @mattburnett-repo

I have added a few more tests for communities/groups/views.py and created a PR. I will be working on the tests for delete and patch part in the coming days.

I would also like to know if this the functions in the attached pic below needs completion so that I can write tests for that as well.

Thank you!

[mattburnett-repo]

For the GroupTextViewSet class, I would suggest building out a simple test for this, maybe a test that simply checks if the class / viewset exists. This would help with the coverage, and in the future when this class is more built we could also build out the test.

That's just one thought about this. What do you think @andrewtavis ?

[andrewtavis]

I'd agree with @mattburnett-repo on this :) Thanks for asking, @sh-ran! 😊

[sh-ran]

Hello! @andrewtavis @mattburnett-repo

I am raising this issue regarding my previous accepted PR. While working on other tests I came across my mistake I had made in the old PR. The if-blocks. While checking the coverage of the tests I found that the test to delete user was never covered and showed up as missing. Either the if block was never executed or was not included in the coverage report or both.

So the for the past few days I have been digging through the codebase to fix this. And here is everything I found.

1. Looking at the swagger UI you will find that it accepts no parameters but in views.py file you will see that it accepts a paramater pk.

2. So to fix this I checked the serializers.py file and found there's no relevant code to handle to the validation of the data. I created a DeleteUserSerializer class and updated it with code by using what others had written in other classes. And also updated the views.py files as needed.

3. After all this the swagger UI still doesnt reflect the changes and also I cant seem to authenticate the user. I tried using authorization headers but it throws a 415 error.

This is the current test case I have written:

Any help regarding the authentication part would be really helpful.

[andrewtavis]

Hey @sh-ran 👋 Very sorry for the slow reply here. Last few days have been very very busy on my end. Do you want to open a PR with the current changes and then we can try it out locally?

CC @to-sta here as well for the backend discussion :)

[to-sta]

For authentication you need to add an authentication header with the users token to the request. There should be a logged_in_user pytest fixture example in the organisation test_api.py file.

[sh-ran]

@andrewtavis All cool :) haha. I will work on the authentication part after once going through what @to-sta mentioned and if I still cant figure it out I will raise a PR for help.

PS: @to-sta I could not find the file you were talking about. If I am not wrong the file you mentioned should be in backend/communities/organizations/tests/test_api.py. Could not find it using search as well in VS code.

And as for the token part, if you mean something like this: response = client.delete( path="/v1/auth/delete/", data={"pk" : UUID}, headers={"Authentication" : "Token " + insert_token_from_user})

I did try this as well. But I get an error code 415 saying Unsupported Media Type. I did not mess around with content type and hence never went deep into this error.

[to-sta]

@sh-ran Ah yeah 😅 you are right. The PR #1159 is not merged yet.

Take a look at the PR; I think there’s something like that in there:

@pytest.fixture
def logged_in_user():
       ...

client.credentials(...)

[sh-ran]

Ah makes sense. I will go through it now and start working! Thank you @to-sta

Cheers!

[andrewtavis]

Thanks for the support here, @to-sta! I'm hoping to merge the orgs, events and hopefully groups at the same time, but will get to orgs and events soon if the groups aren't along yet :)

[sh-ran]

Hello @andrewtavis !

I have pushed a PR with new tests and also attached a coverage report so I don't mess up like last time 😆. I have yet to push a create_group test. Its all done just a few bugs to squash thats all. I have been unable to to set the topics. As soon as I am done with it I will be pushing it as well.

And thank you for all the help in the past couple of days! Those were really helpful.

[andrewtavis]

Hey @sh-ran! Sorry for the wait on reviewing your PR. It's now at the top of my list, so I should be able to get to it sometime today 😊

[sh-ran]

Hello! @andrewtavis

Sorry for getting back to you so late. Just saw that my recent PR has been merged. Thank you for that!

As for what to work on, I was planning on continuing writing the tests to improve coverage and also I had a doubt regarding delete user serializer.

Just wanted to know if this is all or if its incomplete. If its incomplete maybe I can look into it and complete that as well.

Or if you had some other issue that needs any attention I could work on that too. I am go either way.

[andrewtavis]

Hi @sh-ran 👋 No worries on the reply! The serializer does seem incomplete to me, but then @to-sta would know best on that. I'd say that we can definitely continue here though! Feel free to work on other tests and we'll finalize the decision on the serializer above :)

[to-sta]

A status code and a simple message are enough for now.

[sh-ran]

Hello! @andrewtavis @to-sta ,

just wanted to clarify if this is all for this code (i am guessing its not)

My org delete test was failing and in order to make it work i had to comment out this line in organization/views.py

I can either leave that commented out while I submit the PR or I can comment out the test I have written and you guys can uncomment it later on when the StatusType class is more developed.

Also I was encountering this error: While trying to create a new org I cant assign org.created_by = user where org is an instance created using OrganizationFactory and user is an instance created by UserFactory. I cannot understand why I cannot assign to user when that instance is a super to UserModel. Am I doing something wrong? Everywhere else I was able to set it without any hiccups.

relevant code to this problem:

Thank you!

[to-sta]

Let's start with the test_org_create, I will try to get back to you on other issues you encountered later on.

Also I was encountering this error: While trying to create a new org I cant assign org.created_by = user where org is an instance created using OrganizationFactory and user is an instance created by UserFactory. I cannot understand why I cannot assign to user when that instance is a super to UserModel. Am I doing something wrong? Everywhere else I was able to set it without any hiccups.

relevant code to this problem:

Thank you!

The error msg says you aren't able to assign an AnonymousUser, that is because you are making the POST request, without being authenticated.

Essentially you need to create a user and login. When you login you are getting a token back that you need to add to your request with a Authorisation header.

It is not necessary to use the OrganisationFactory, because you want to create an Organisation via the API, right?

The view will retrieve the user based on the token you are sending with your request and add it to the creation of the Organisation.

org = serializer.save(created_by=request.user)

Hope that helps.

[sh-ran]

Let's start with the test_org_create, I will try to get back to you on other issues you encountered later on.

Also I was encountering this error: While trying to create a new org I cant assign org.created_by = user where org is an instance created using OrganizationFactory and user is an instance created by UserFactory. I cannot understand why I cannot assign to user when that instance is a super to UserModel. Am I doing something wrong? Everywhere else I was able to set it without any hiccups. relevant code to this problem: Thank you!

The error msg says you aren't able to assign an AnonymousUser, that is because you are making the POST request, without being authenticated.

Essentially you need to create a user and login. When you login you are getting a token back that you need to add to your request with a Authorisation header.

It is not necessary to use the OrganisationFactory, because you want to create an Organisation via the API, right?

The view will retrieve the user based on the token you are sending with your request and add it to the creation of the Organisation.

org = serializer.save(created_by=request.user)

Hope that helps.

Well this is embarrassing of me. I have written that part so many times by now that I should practically have it by heart in fact I have finished all the test cases and messed up this one up haha. But I still forgot to get the auth headers face palm moment

I apologize for my stupidity and wasting your time, my bad.

[to-sta]

No worries @sh-ran and thanks for your contribution.

[andrewtavis]

No need to apologize, @sh-ran, and not stupid at all. Happens to everyone 😊

[sh-ran]

Hello! @andrewtavis

While working on tests for contents/views.py I noticed that there was no DiscussionFactory Class. Tried writing it but unfortunately couldn't get it to work. If that factory class is developed I could write some more tests for the contents app.

[andrewtavis]

Hey @sh-ran 👋 Thanks for all the detail you're putting into this! Do you want to send along your best attempt at the factory and we'll finish it up, and then you can write the further tests in another PR? :)