-
Notifications
You must be signed in to change notification settings - Fork 213
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Added comprehensive test coverage for Group-related models #1020
base: main
Are you sure you want to change the base?
Changes from 7 commits
4c0efb5
d29d144
6f2b7e5
493be86
0b126a0
cf46c3c
88acfb9
e9b2389
d960f41
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,3 +41,5 @@ dist | |
|
||
# Node dependencies | ||
node_modules | ||
/.vscode | ||
.vscode/settings.json |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,10 @@ | ||
{ | ||
"eslint.validate": ["javascript", "typescript", "vue"], | ||
"eslint.useFlatConfig": true, | ||
"typescript.tsdk": "./frontend/node_modules/typescript/lib" | ||
"typescript.tsdk": "./frontend/node_modules/typescript/lib", | ||
"python.testing.pytestArgs": [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you give me a quick explanation of why these lines are needed, @zeus2611? Thank you! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. VS Code automatically added these lines while I was working on the tests. It seems they were included as part of the editor's configuration updates. Let me know if you’d like me to investigate further! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's remove these then :) Can you do another commit to do this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah you already did :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you! |
||
"backend" | ||
], | ||
"python.testing.unittestEnabled": false, | ||
"python.testing.pytestEnabled": true | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,235 @@ | ||
""" | ||
Testing for the Group model. | ||
""" | ||
|
||
# mypy: ignore-errors | ||
from datetime import datetime | ||
from uuid import UUID | ||
|
||
import pytest | ||
from django.core.exceptions import ValidationError | ||
from faker import Faker | ||
|
||
from authentication.factories import UserFactory | ||
from entities.factories import ( | ||
GroupFactory, | ||
OrganizationFactory, | ||
) | ||
from entities.models import Group | ||
|
||
pytestmark = pytest.mark.django_db | ||
|
||
|
||
def test_group_creation() -> None: | ||
"""Test complete group creation with all fields.""" | ||
user = UserFactory() | ||
org = OrganizationFactory(created_by=user) | ||
fake = Faker() | ||
|
||
group = Group.objects.create( | ||
org_id=org, | ||
created_by=user, | ||
group_name=fake.company(), | ||
name=fake.company(), | ||
tagline=fake.catch_phrase(), | ||
location=fake.city(), | ||
category=fake.word(), | ||
get_involved_url=fake.url(), | ||
terms_checked=True, | ||
) | ||
|
||
assert isinstance(group.id, UUID) | ||
assert group.org_id == org | ||
assert group.created_by == user | ||
assert isinstance(group.group_name, str) | ||
assert isinstance(group.creation_date, datetime) | ||
assert group.terms_checked is True | ||
|
||
|
||
def test_required_fields() -> None: | ||
"""Test that required fields raise validation error when missing.""" | ||
user = UserFactory() | ||
org = OrganizationFactory(created_by=user) | ||
|
||
# Test missing group_name | ||
with pytest.raises(ValidationError): | ||
group = Group( | ||
org_id=org, | ||
created_by=user, | ||
name="Test Name", | ||
location="Test Location", | ||
category="Test Category", | ||
) | ||
group.full_clean() | ||
|
||
# Test missing location | ||
with pytest.raises(ValidationError): | ||
group = Group( | ||
org_id=org, | ||
created_by=user, | ||
group_name="Test Group", | ||
name="Test Name", | ||
category="Test Category", | ||
) | ||
group.full_clean() | ||
|
||
|
||
def test_optional_fields() -> None: | ||
"""Test that optional fields can be blank or null.""" | ||
user = UserFactory() | ||
org = OrganizationFactory(created_by=user) | ||
|
||
group = Group.objects.create( | ||
org_id=org, | ||
created_by=user, | ||
group_name="Test Group", | ||
name="Test Name", | ||
location="Test Location", | ||
category="Test Category", | ||
) | ||
|
||
# Should not raise ValidationError | ||
group.full_clean() | ||
assert group.tagline == "" | ||
assert group.get_involved_url == "" | ||
assert group.terms_checked is False | ||
|
||
|
||
def test_field_max_lengths() -> None: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @zeus2611, I reviewed your changes, and they generally look great! 😃 One suggestion before we merge: please remove the tests related to Django, such as those for max_length or auto_fields. We don't need these tests because Django’s ORM and model field types are already well-tested by the framework itself. We rely on Django's built-in tests to ensure that things like max_length and auto_fields are functioning correctly. Additionally, during the code review process, we check for the correct usage of these features. If there were any issues, we would catch them at that stage. By removing these tests, we keep the test suite focused on more application-specific logic, avoiding redundancy. Thanks! |
||
"""Test that fields have correct max lengths.""" | ||
user = UserFactory() | ||
org = OrganizationFactory(created_by=user) | ||
fake = Faker() | ||
|
||
group = Group.objects.create( | ||
org_id=org, | ||
created_by=user, | ||
group_name=fake.company(), | ||
name=fake.company(), | ||
tagline=fake.catch_phrase(), | ||
location=fake.city(), | ||
category=fake.word(), | ||
get_involved_url=fake.url(), | ||
terms_checked=True, | ||
) | ||
|
||
assert len(group.group_name) <= 100 | ||
assert len(group.name) <= 100 | ||
assert len(group.tagline) <= 200 | ||
assert len(group.location) <= 100 | ||
assert len(group.category) <= 100 | ||
assert len(group.get_involved_url) <= 200 | ||
|
||
|
||
def test_cascade_delete() -> None: | ||
"""Test that deleting an organization deletes all associated groups.""" | ||
user = UserFactory() | ||
org = OrganizationFactory(created_by=user) | ||
|
||
GroupFactory(org_id=org, created_by=user) | ||
|
||
assert Group.objects.count() == 1 | ||
org.delete() | ||
assert Group.objects.count() == 0 | ||
|
||
org = OrganizationFactory(created_by=user) | ||
GroupFactory(org_id=org, created_by=user) | ||
|
||
assert Group.objects.count() == 1 | ||
user.delete() | ||
assert Group.objects.count() == 0 | ||
|
||
|
||
def test_url_validations() -> None: | ||
"""Test that get_involved_url field is a valid URL.""" | ||
user = UserFactory() | ||
org = OrganizationFactory(created_by=user) | ||
fake = Faker() | ||
|
||
# Test invalid URL | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor thing that we do for comments is that if they're their own line as is the case here, then it'd be best if they were fully punctuated as this is Python standard :) If it's an inline comment, then no need for capitalization or a period at the end, but if it's on its own line then including both would be best :) I can also do this if you'd like me to! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, @andrewtavis! That makes sense. I'll update the comments to follow the Python standard, ensuring punctuation and capitalization where needed for standalone comments. Appreciate the heads-up! 😊 |
||
with pytest.raises(ValidationError): | ||
group = Group( | ||
org_id=org, | ||
created_by=user, | ||
group_name=fake.company(), | ||
name=fake.company(), | ||
location=fake.city(), | ||
category=fake.word(), | ||
get_involved_url="not a url", | ||
terms_checked=True, | ||
) | ||
group.full_clean() | ||
|
||
# Test valid URL | ||
group = Group.objects.create( | ||
org_id=org, | ||
created_by=user, | ||
group_name=fake.company(), | ||
name=fake.company(), | ||
location=fake.city(), | ||
category=fake.word(), | ||
get_involved_url=fake.url(), | ||
terms_checked=True, | ||
) | ||
|
||
group.full_clean() | ||
|
||
|
||
def test_auto_fields() -> None: | ||
"""Test that auto fields are set correctly.""" | ||
user = UserFactory() | ||
org = OrganizationFactory(created_by=user) | ||
fake = Faker() | ||
|
||
group = Group.objects.create( | ||
org_id=org, | ||
created_by=user, | ||
group_name=fake.company(), | ||
name=fake.company(), | ||
location=fake.city(), | ||
category=fake.word(), | ||
get_involved_url=fake.url(), | ||
terms_checked=True, | ||
) | ||
|
||
assert group.creation_date is not None | ||
assert isinstance(group.creation_date, datetime) | ||
assert group.id is not None | ||
assert isinstance(group.id, UUID) | ||
|
||
|
||
def test_multiple_groups_per_org() -> None: | ||
"""Test that multiple groups can be created per organization.""" | ||
user = UserFactory() | ||
org = OrganizationFactory(created_by=user) | ||
fake = Faker() | ||
|
||
group1 = Group.objects.create( | ||
org_id=org, | ||
created_by=user, | ||
group_name=fake.company(), | ||
name=fake.company(), | ||
location=fake.city(), | ||
category=fake.word(), | ||
get_involved_url=fake.url(), | ||
terms_checked=True, | ||
) | ||
|
||
group2 = Group.objects.create( | ||
org_id=org, | ||
created_by=user, | ||
group_name=fake.company(), | ||
name=fake.company(), | ||
location=fake.city(), | ||
category=fake.word(), | ||
get_involved_url=fake.url(), | ||
terms_checked=True, | ||
) | ||
|
||
assert Group.objects.count() == 2 | ||
assert group1.org_id == org | ||
assert group2.org_id == org | ||
|
||
org_groups = Group.objects.filter(org_id=org) | ||
assert group1 in org_groups | ||
assert group2 in org_groups |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
""" | ||
Test cases for the GroupEvents entity. | ||
""" | ||
|
||
import pytest | ||
|
||
from entities.factories import GroupEventFactory, GroupFactory | ||
|
||
pytestmark = pytest.mark.django_db | ||
|
||
|
||
def test_group_event_str() -> None: | ||
"""Test string representation of GroupEvent model""" | ||
group_event = GroupEventFactory.build() | ||
assert str(group_event) == f"{group_event.id}" | ||
|
||
|
||
def test_multiple_events_per_group() -> None: | ||
"""Test multiple events for a single group""" | ||
group = GroupFactory() | ||
events = [GroupEventFactory(group_id=group) for _ in range(3)] | ||
|
||
assert len(events) == 3 | ||
for event in events: | ||
assert event.group_id == group |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
""" | ||
Test cases for the GroupMember model. | ||
""" | ||
|
||
import pytest | ||
|
||
from authentication.factories import UserFactory | ||
from entities.factories import GroupFactory, GroupMemberFactory | ||
|
||
pytestmark = pytest.mark.django_db | ||
|
||
|
||
def test_group_member_str() -> None: | ||
"""Test string representation of GroupMember model""" | ||
group_member = GroupMemberFactory.build() | ||
assert str(group_member) == f"{group_member.id}" | ||
|
||
|
||
def test_group_member_roles() -> None: | ||
"""Test the different roles a group member can have""" | ||
user = UserFactory() | ||
group = GroupFactory() | ||
|
||
# Test owner role | ||
owner = GroupMemberFactory( | ||
group_id=group, user_id=user, is_owner=True, is_admin=False, is_comms=False | ||
) | ||
assert owner.is_owner is True | ||
assert owner.is_admin is False | ||
assert owner.is_comms is False | ||
|
||
# Test admin role | ||
admin = GroupMemberFactory( | ||
group_id=group, user_id=user, is_owner=False, is_admin=True, is_comms=False | ||
) | ||
assert admin.is_owner is False | ||
assert admin.is_admin is True | ||
assert admin.is_comms is False | ||
|
||
# Test comms role | ||
comms = GroupMemberFactory( | ||
group_id=group, user_id=user, is_owner=False, is_admin=False, is_comms=True | ||
) | ||
assert comms.is_owner is False | ||
assert comms.is_admin is False | ||
assert comms.is_comms is True | ||
|
||
|
||
def test_multiple_members_per_group() -> None: | ||
"""Test multiple members in a single group""" | ||
group = GroupFactory() | ||
members = [GroupMemberFactory(group_id=group) for _ in range(3)] | ||
|
||
assert len(members) == 3 | ||
|
||
for member in members: | ||
assert member.group_id == group |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checking in on these being added in @zeus2611 :) We do have
settings.json
andextensions.json
version controlled, so should we maybe take these lines out?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I initially thought this folder was automatically modified and added to version control. However, since these files are already version-controlled, I’ll update the
.gitignore
file to exclude them. Thanks for pointing that out!