activist
activist copied to clipboard
activist-org
Expand Serializers testing for backend
Terms
- [x] I have searched open and closed feature requests
- [x] I agree to follow activist's Code of Conduct
Description
In https://github.com/activist-org/activist/issues/1121 and https://github.com/activist-org/activist/pull/1125 we added in a coverage report to the backend PR workflow. One thing that became apparent from that is that our testing for Serializers in the backend is quite low. Here's the current output for the Serializer files in the backend:
backend/authentication/serializers.py 59 2 97% 111, 114
backend/communities/groups/serializers.py 46 7 85% 59-64, 67-72
backend/communities/organizations/serializers.py 50 7 86% 60-65, 68-73
backend/communities/serializers.py 10 0 100%
backend/content/serializers.py 57 20 65% 45-71, 92-106
backend/events/serializers.py 49 12 76% 58-71, 74-79, 88-90
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 🚀
Hi @andrewtavis
I'd like to work on this issue! I'm looking to start contributing to open source projects, and improving test coverage seems like a great way to get familiar with the codebase. I have experience with backend development and writing tests, though I'm new to this specific project.
I understand this involves writing tests to improve coverage for the serializer files listed in the description. I'd like to start with one file (perhaps backend/authentication/serializers.py since it already has high coverage and might be a gentler introduction) and then continue with the others if that approach works well.
Could you provide guidance on,
The preferred testing approach for serializers in this project ? Any specific testing patterns I should follow ? Whether you'd prefer I tackle one file per PR or group them ?
I'll set up the development environment and familiarize myself with the existing tests. Thanks for the opportunity to contribute!
Hey @narmadha-raghu 👋 Welcome to the community! Great to have you here 😊
I think you're plan of starting with just backend/authentication/serializers.py makes total sense :) Thanks for sharing your thinking on how to tackle this issue!
Answers to your questions:
The preferred testing approach for serializes in this project? Any specific testing patterns I should follow?
- I was looking for some resources to direct you to, and this blog post seemed to be a sensible approach with some very logical patterns that we could bring over here
- Looking into this and also bench marking the current test patterns against them would be great!
Whether you'd prefer I tackle one file per PR or group them?
- Whatever works for you, but maybe we can do one file for the first one?
Thanks for your willingness to contribute! Looking forward to this 🚀 Please let us know if you have further questions!
CC @to-sta for if anything comes to mind with how we write serializer tests :)
Hi @andrewtavis,
I've added tests to bring backend/authentication/serializers.py to 100% coverage.
I followed the resource you shared and specifically added tests for PasswordResetSerializer. The rest of the serializers are already covered through endpoint testing.
I initially tried to fit this within the existing test patterns without creating a new test class, but since serializer testing requires direct validation on the serializer object, endpoint testing alone wouldn't suffice. However, adding serializer-specific tests only for PasswordResetSerializer feels inconsistent, given that other serializers rely solely on endpoint testing.
Would you recommend also applying the DRF serializer testing pattern to DeleteUserResponseSerializer, SignupSerializer, and LoginSerializer for consistency?
Looking forward to your feedback! 🚀
Yes let's definitely work on the other Serializers and get them consistent given the changes once we merge in #1151! Thanks for the work here! I'll try to give it a check in a few days :)
Would you have interest in moving on to another serializer testing file at this point, @narmadha-raghu? Hope so! 😊
@andrewtavis Of course, I will take it up. I'll give one PR to fix all the serializer tests this time. Would that work?
Current coverage for the serializers is:
Name Stmts Miss Cover Missing
------------------------------------------------------------------------
authentication/serializers.py 59 0 100%
communities/groups/serializers.py 54 5 91% 117, 137-142
communities/organizations/serializers.py 50 1 98% 86
communities/serializers.py 10 0 100%
content/serializers.py 78 14 82% 86-91, 113-115, 148, 261-275
events/serializers.py 57 8 86% 130, 138, 158-163, 197-199
------------------------------------------------------------------------
Do you want to raise a PR to finish the organizations serializer testa and maybe groups in the same one, @narmadha-raghu? The tests for the organization file should be directly applicable to the groups 😊
@andrewtavis I'll take this up this weekend, I'll provide the communities serializer in one PR.
So far so good here, @MuhammadNizamani! As mentioned int he review of #1475, do we want to do organization tests now? Should be very similar to those for groups. If we want to start reorganizing directories to split the files, then that would also make sense to me :)
Closed by the above PRs 🚀 Really thanks so much for a great first effort in the community, @MuhammadNizamani! Let us know if another issue is of interest, and also please feel free to open issues if you're seeing things that you'd suggest improving 😊