Skip to content

Conversation

Joe-Heffer-Shef
Copy link
Collaborator

@Joe-Heffer-Shef Joe-Heffer-Shef commented Mar 14, 2025

Changes:

  • Add tests to GitHub Actions workflow
  • Refactor test suite
  • Add unittest.TestCase base classes for views and services
  • Implement mock object factories using Factory Boy
  • Added unit tests for:
    • Organisation Service
    • Organisation views
    • Project Service
    • Project views
    • User views (home, login, logout)
    • Survey service
  • Added coverage report
  • Run tests in parallel (2 cores on a GHA runner)

The invite tests are currently disabled.

@Joe-Heffer-Shef Joe-Heffer-Shef linked an issue Mar 14, 2025 that may be closed by this pull request
8 tasks
@Joe-Heffer-Shef Joe-Heffer-Shef mentioned this pull request Mar 14, 2025
8 tasks
@Joe-Heffer-Shef Joe-Heffer-Shef self-assigned this Mar 14, 2025
@Joe-Heffer-Shef Joe-Heffer-Shef added the enhancement New feature or request label Mar 14, 2025
Copy link
Contributor

@twinkarma twinkarma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Joe-Heffer-Shef, I've also added all the base permission tests which has now been merged into the dev branch. Also added factoryboy for creating test objects. Can you integrate them into this PR?

@twinkarma
Copy link
Contributor

Also let's use the dev branch as the as the one we just for active development. Once we're happy with the state of the dev branch we can merge the changes in to main.

@Joe-Heffer-Shef Joe-Heffer-Shef changed the base branch from main to dev March 14, 2025 11:44
@Joe-Heffer-Shef Joe-Heffer-Shef marked this pull request as draft March 14, 2025 12:02
@Joe-Heffer-Shef
Copy link
Collaborator Author

Converted to draft while I merged in Twin's unit tests with what I've written.

@Joe-Heffer-Shef Joe-Heffer-Shef marked this pull request as ready for review March 19, 2025 14:37
@Joe-Heffer-Shef
Copy link
Collaborator Author

Joe-Heffer-Shef commented Mar 19, 2025

Is this some issue with resolving the URLs? Is it getting confused between invite/<pk>/ and invite/success/?

def test_success_invitation(self):
self.get("success_invitation")

django.urls.exceptions.NoReverseMatch: Reverse for 'invite' with no arguments not found. 1 pattern(s) tried: ['invite/(?P<pk>[0-9]+)\\Z']

SORT/survey/urls.py

Lines 56 to 61 in a6efc2b

path("invite/<int:pk>", views.InvitationView.as_view(), name="invite"),
path(
"invite/success/",
views.SuccessInvitationView.as_view(),
name="success_invitation",
),

@twinkarma
Copy link
Contributor

@Joe-Heffer-Shef I think everything related to email invitations need a proper review altogether. It was something implemented very early on but so much has changed that something probably broke along the way. I made an issue #89 for this exact reason. I'd be tempted to just leave the invitation tests to be done in that issue.

@Joe-Heffer-Shef Joe-Heffer-Shef removed the enhancement New feature or request label Mar 20, 2025
@Joe-Heffer-Shef
Copy link
Collaborator Author

@Joe-Heffer-Shef I think everything related to email invitations need a proper review altogether. It was something implemented very early on but so much has changed that something probably broke along the way. I made an issue #89 for this exact reason. I'd be tempted to just leave the invitation tests to be done in that issue.

Okay, I've disabled the invite tests for now, which can be implemented separately with issue #89.

These tests all pass now. There might be some details that I've overlooked but the code coverage should be alright for now! (We could measure this.)

Joe-Heffer-Shef and others added 8 commits April 15, 2025 11:19
# Conflicts:
#	README.md
#	SORT/settings.py
#	assets/sort-survey-configurator/README.md
#	assets/sort-survey-configurator/package-lock.json
#	home/templatetags/vite_integration.py
#	survey/services/survey.py
@Joe-Heffer-Shef Joe-Heffer-Shef removed the request for review from f-allian April 15, 2025 14:49
Copy link
Contributor

@twinkarma twinkarma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few minor changes need for the test class names but the rest is looking good. Let's get this merged in!


def test_add_user_to_organisation(self):
"""
Check that an organisation administrator can add another user to that organisation.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A manager won't be adding a user directly. They'll need to send an invite that generates the code, the user can then add themselves to the organisation if they have the correct code.

from survey.services import SurveyService


class SurveyServiceTestCase(SORT.test.test_case.ViewTestCase):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename to SurveyViewsTestcase?

from survey.services import SurveyService


class SurveyServiceTestCase(SORT.test.test_case.ViewTestCase):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename to InvitationViewTestCase?

# Conflicts:
#	SORT/settings.py
#	home/tests/model_factory.py
#	home/tests/test_services.py
#	requirements.txt
#	survey/services/survey.py
#	survey/tests/test_models.py
@Joe-Heffer-Shef Joe-Heffer-Shef merged commit e377616 into dev Apr 29, 2025
4 checks passed
@Joe-Heffer-Shef Joe-Heffer-Shef deleted the test/backend branch April 29, 2025 15:37
@Joe-Heffer-Shef
Copy link
Collaborator Author

I accidentally merged this without resolving the issues above, sorry @twinkarma! I'll sort out the issues (no pun intended) next week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add unit testing to backend
2 participants