From 2bb4d38f757b5ca3443fd89d15a0d81ea86caad9 Mon Sep 17 00:00:00 2001 From: Jannis Vajen Date: Tue, 19 May 2020 14:38:58 +0200 Subject: [PATCH 1/4] tests: Use realistic custom user model The previous test setup inherited from AbstractUser thus MyCustomUser still had a username field. The problems people are having when using the admin_client fixture in combination with a custom user model are due to the username field not being present. This change accounts for the more realistic scenario. See these tickets: https://github.com/pytest-dev/pytest-django/pull/246 https://github.com/pytest-dev/pytest-django/pull/484 https://github.com/pytest-dev/pytest-django/pull/748 --- tests/test_fixtures.py | 43 ++++++++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 12 deletions(-) diff --git a/tests/test_fixtures.py b/tests/test_fixtures.py index 2c718c54f..6f81474f1 100644 --- a/tests/test_fixtures.py +++ b/tests/test_fixtures.py @@ -503,15 +503,40 @@ def test_with_live_server(live_server): def test_custom_user_model(django_testdir, username_field): django_testdir.create_app_file( """ - from django.contrib.auth.models import AbstractUser + from django.contrib.auth.models import AbstractBaseUser, BaseUserManager, PermissionsMixin from django.db import models - class MyCustomUser(AbstractUser): + class MyCustomUserManager(BaseUserManager): + def create_user(self, {username_field}, password=None, **extra_fields): + extra_fields.setdefault('is_staff', False) + extra_fields.setdefault('is_superuser', False) + user = self.model({username_field}={username_field}, **extra_fields) + user.set_password(password) + user.save() + return user + + def create_superuser(self, {username_field}, password=None, **extra_fields): + extra_fields.setdefault('is_staff', True) + extra_fields.setdefault('is_superuser', True) + return self.create_user( + {username_field}={username_field}, + password=password, + **extra_fields + ) + + class MyCustomUser(AbstractBaseUser, PermissionsMixin): + email = models.EmailField(max_length=254, unique=True) identifier = models.CharField(unique=True, max_length=100) + is_staff = models.BooleanField( + 'staff status', + default=False, + help_text='Designates whether the user can log into this admin site.' + ) - USERNAME_FIELD = '%s' - """ - % (username_field), + objects = MyCustomUserManager() + + USERNAME_FIELD = '{username_field}' + """.format(username_field=username_field), "models.py", ) django_testdir.create_app_file( @@ -573,19 +598,13 @@ class Migration(migrations.Migration): ('password', models.CharField(max_length=128, verbose_name='password')), ('last_login', models.DateTimeField(null=True, verbose_name='last login', blank=True)), ('is_superuser', models.BooleanField(default=False, help_text='Designates that this user has all permissions without explicitly assigning them.', verbose_name='superuser status')), - ('username', models.CharField(error_messages={'unique': 'A user with that username already exists.'}, max_length=30, validators=[django.core.validators.RegexValidator(r'^[\\w.@+-]+$', 'Enter a valid username. This value may contain only letters, numbers and @/./+/-/_ characters.', 'invalid')], help_text='Required. 30 characters or fewer. Letters, digits and @/./+/-/_ only.', unique=True, verbose_name='username')), - ('first_name', models.CharField(max_length=30, verbose_name='first name', blank=True)), - ('last_name', models.CharField(max_length=30, verbose_name='last name', blank=True)), - ('email', models.EmailField(max_length=254, verbose_name='email address', blank=True)), + ('email', models.EmailField(error_messages={'unique': 'A user with that email address already exists.'}, max_length=254, unique=True, verbose_name='email address')), ('is_staff', models.BooleanField(default=False, help_text='Designates whether the user can log into this admin site.', verbose_name='staff status')), - ('is_active', models.BooleanField(default=True, help_text='Designates whether this user should be treated as active. Unselect this instead of deleting accounts.', verbose_name='active')), - ('date_joined', models.DateTimeField(default=django.utils.timezone.now, verbose_name='date joined')), ('identifier', models.CharField(unique=True, max_length=100)), ('groups', models.ManyToManyField(related_query_name='user', related_name='user_set', to='auth.Group', blank=True, help_text='The groups this user belongs to. A user will get all permissions granted to each of their groups.', verbose_name='groups')), ('user_permissions', models.ManyToManyField(related_query_name='user', related_name='user_set', to='auth.Permission', blank=True, help_text='Specific permissions for this user.', verbose_name='user permissions')), ], options={ - 'abstract': False, 'verbose_name': 'user', 'verbose_name_plural': 'users', }, From 94dcb4916c99e3febd3d60b9e1bbc39e067326d9 Mon Sep 17 00:00:00 2001 From: Jannis Vajen Date: Tue, 19 May 2020 14:47:23 +0200 Subject: [PATCH 2/4] tests: fix admin_client & admin_user for custom user model Relying on `User.USERNAME_FIELD` is enough for light customization and doesn't require `extra_arguments`. Esoteric user models probably need a specially tailored manager anyway and most likely a custom `django_user` fixture that goes with that. See this ticket: https://github.com/pytest-dev/pytest-django/issues/457 --- pytest_django/fixtures.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/pytest_django/fixtures.py b/pytest_django/fixtures.py index b2cc82580..7d1485674 100644 --- a/pytest_django/fixtures.py +++ b/pytest_django/fixtures.py @@ -291,11 +291,12 @@ def admin_user(db, django_user_model, django_username_field): try: user = UserModel._default_manager.get(**{username_field: username}) except UserModel.DoesNotExist: - extra_fields = {} - if username_field not in ("username", "email"): - extra_fields[username_field] = "admin" user = UserModel._default_manager.create_superuser( - username, "admin@example.com", "password", **extra_fields + **{ + username_field: username, + "email": "admin@example.com", + "password": "password", + } ) return user @@ -306,7 +307,7 @@ def admin_client(db, admin_user): from django.test.client import Client client = Client() - client.login(username=admin_user.username, password="password") + client.login(username=admin_user.get_username(), password="password") return client From ca2c2161a511d9bf204a284a6603bf1568c9a37d Mon Sep 17 00:00:00 2001 From: Jannis Vajen Date: Tue, 19 May 2020 23:15:16 +0200 Subject: [PATCH 3/4] Work around issue with pyflakes 2.2.0 --- pytest_django/plugin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pytest_django/plugin.py b/pytest_django/plugin.py index 5c59bd540..4d692de27 100644 --- a/pytest_django/plugin.py +++ b/pytest_django/plugin.py @@ -240,7 +240,7 @@ def _get_boolean_value(x, name, default=None): except KeyError: raise ValueError( "{} is not a valid value for {}. " - "It must be one of {}." % (x, name, ", ".join(possible_values.keys())) + "It must be one of {}.".format(x, name, ", ".join(possible_values.keys())) ) From 9f381099434681a69dd1b68abe61510076882e87 Mon Sep 17 00:00:00 2001 From: Jannis Vajen Date: Tue, 19 May 2020 23:35:11 +0200 Subject: [PATCH 4/4] Match passing test with warnings "1 passed, 1 warning" wasn't matched before when there was a deprecation warning. Newer Django versions warn about the removal of `django.conf.urls.url` in favor of `django.urls.re_path` but Django 1.11 is also tested against and doesn't support `re_path` yet. --- tests/test_fixtures.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_fixtures.py b/tests/test_fixtures.py index 6f81474f1..218fa9e28 100644 --- a/tests/test_fixtures.py +++ b/tests/test_fixtures.py @@ -616,7 +616,7 @@ class Migration(migrations.Migration): ) result = django_testdir.runpytest_subprocess("-s") - result.stdout.fnmatch_lines(["* 1 passed in*"]) + result.stdout.fnmatch_lines(["* 1 passed*"]) assert result.ret == 0