-
-
Notifications
You must be signed in to change notification settings - Fork 24
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
Add unit tests for User model and configure dedicated test database #493
base: main
Are you sure you want to change the base?
Conversation
Implemented tests for creating users, superusers, token management, user activation/deactivation, and group properties in the users app. Signed-off-by: MissTipo <[email protected]>
- Modified database settings to use a dedicated test database when running tests to avoid conflicts with production data. Signed-off-by: MissTipo <[email protected]>
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.
Some changes are still necessary so that all the checks can pass.
backend/users/tests.py
Outdated
with self.assertRaisesMessage(ValueError, "Superuser must have is_staff=True."): | ||
User.objects.create_superuser(email="[email protected]", password="pass", is_staff=False) | ||
# is_superuser must be True | ||
with self.assertRaisesMessage(ValueError, "Superuser must have is_superuser=True."): | ||
User.objects.create_superuser(email="[email protected]", password="pass", is_superuser=False) |
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.
If you're checking these errors, it's good to take into account i18n.
In this case, instead of checking for "Superuser must have is_staff=True."
, test for _("Superuser must have is_staff=True.")
, where _
is from django.utils.translation import gettext as _
.
backend/users/tests.py
Outdated
import uuid | ||
from django.test import TestCase, override_settings | ||
from django.urls import reverse | ||
from django.utils import timezone | ||
from django.contrib.auth.models import Group | ||
|
||
from users.models import User, CustomUserManager | ||
from users.groups_management import MAIN_ADMIN, NGO_ADMIN, NGO_MEMBER, RESTRICTED_ADMIN |
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.
Ruff is going to complain on unused imports, so remove what you didn't make use of.
In the backend/
folder, run ruff check .
and it will tell you what issues you have.
if "test" in sys.argv: | ||
DATABASES["default"].update({ | ||
"NAME": env("TEST_DATABASE_NAME", default="redirectioneaza_test"), | ||
"HOST": env("TEST_DATABASE_HOST", default="localhost"), | ||
"USER": env("TEST_DATABASE_USER", default=env("DATABASE_USER")), | ||
"PASSWORD": env("TEST_DATABASE_PASSWORD", default=env("DATABASE_PASSWORD")), | ||
}) |
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.
Black is going to complain regarding formatting on this file but also tests.py
.
To check it on your own system, either run black ./ --check
in the backend/
folder, or black ./backend --check
in the root.
You can also run it without the --check
flag, but beware that if you're not targeting a folder in the root of which you have a pyproject.toml
, it will not apply the correct rules and might end up trying to over-format code.
Here's a sample of the differences with a lot of lines redacted for readability
❯ black ./backend --check
would reformat ~/redirectioneaza/backend/redirectioneaza/settings/database.py
Oh no! 💥 💔 💥
1 file would be reformatted, 141 files would be left unchanged.
❯ black ./ --check
would reformat ~/redirectioneaza/backend/donations/common/models_hashing.py
[...] — 91 more lines
would reformat ~/redirectioneaza/backend/users/models.py
Oh no! 💥 💔 💥
93 files would be reformatted, 108 files would be left unchanged.
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.
Ok. Working on it.
… issues. Signed-off-by: MissTipo <[email protected]>
This PR introduces a suite of unit tests for the User model (and its custom manager) in the users app, and updates the database settings so that tests run against a dedicated test database instead of the production/Postgres host. These changes improve confidence in user‑related functionality and ensure an isolated environment for running tests.
What’s been changed
users/tests.py
Added tests for
CustomUserManager.create_use
r andcreate_superuser
, including invalid‑flag error cases.Covered token lifecycle methods (
refresh_token,
verify_token
,clear_token
).Verified activate/deactivate behavior.
Tested
create_admin_login_ur
l URL generation.Exercised group‑based properties (
is_admin
,is_ngo_admin
,is_ngo_member
) against all relevant roles.redirectioneaza/settings/database.py
Imported sys and detect "test" in sys.argv.
When running tests, override
DATABASES['default']
to point at a localredirectioneaza_test
database (hosted on localhost) to avoid conflicts with production data.How to verify
createdb redirectioneaza_test
).python manage.py test users
and confirm all 8 tests pass.This lays the groundwork for broader test coverage across the project and ensures our CI/test runs are safe and repeatable.