From 3d9e86b9edc98214994067809ac80226e789412f Mon Sep 17 00:00:00 2001 From: Claire Peters Date: Fri, 7 Jul 2023 20:27:28 -0700 Subject: [PATCH] refactor tests --- coldfront/core/allocation/tests.py | 19 +- coldfront/core/project/test_views.py | 69 +++---- coldfront/core/test_helpers/factories.py | 195 +++++++++--------- .../core/test_helpers/fasrc_factories.py | 15 +- coldfront/core/test_helpers/utils.py | 13 +- 5 files changed, 141 insertions(+), 170 deletions(-) diff --git a/coldfront/core/allocation/tests.py b/coldfront/core/allocation/tests.py index 49b5229fa..824a318f5 100644 --- a/coldfront/core/allocation/tests.py +++ b/coldfront/core/allocation/tests.py @@ -12,11 +12,10 @@ AllocationChangeRequest, ) from coldfront.core.test_helpers.factories import ( - fake, setup_models, - ResourceFactory, UserFactory, ProjectFactory, + ResourceFactory, AllocationFactory, AllocationChangeRequestFactory, ) @@ -72,19 +71,11 @@ def setUpTestData(cls): """Set up users and project for testing""" super(AllocationListViewTest, cls).setUpTestData() cls.additional_allocations = [ - AllocationFactory( - project=ProjectFactory( - title=fake.unique.project_title(), - pi=UserFactory(username=fake.unique.username()), - ) - ) - for i in list(range(100)) + AllocationFactory() for i in list(range(50)) ] for allocation in cls.additional_allocations: allocation.resources.add(ResourceFactory(name='holylfs09/tier1', id=2)) - cls.nonproj_nonallocation_user = UserFactory( - username='rdrake', is_staff=False, is_superuser=False - ) + cls.nonproj_nonallocation_user = UserFactory(username='rdrake') def test_allocation_list_access_admin(self): """Confirm that AllocationList access control works for admin""" @@ -92,7 +83,8 @@ def test_allocation_list_access_admin(self): # confirm that show_all_allocations=on enables admin to view all allocations response = self.client.get("/allocation/?show_all_allocations=on") - self.assertEqual(len(response.context['item_list']), 101) + + self.assertEqual(len(response.context['item_list']), Allocation.objects.all().count()) def test_allocation_list_access_pi(self): """Confirm that AllocationList access control works for pi @@ -198,7 +190,6 @@ def setUp(self): def test_allocationchangeview_access(self): """Test get request""" - kwargs = {'pk': 1} self.allocation_access_tstbase(self.url) utils.test_user_can_access(self, self.pi_user, self.url) # Manager can access utils.test_user_cannot_access(self, self.proj_allocation_user, self.url) # user can't access diff --git a/coldfront/core/project/test_views.py b/coldfront/core/project/test_views.py index 1a0b7fe5a..b60fab006 100644 --- a/coldfront/core/project/test_views.py +++ b/coldfront/core/project/test_views.py @@ -4,18 +4,16 @@ from coldfront.core.test_helpers import utils from coldfront.core.test_helpers.factories import ( + setup_models, UserFactory, ProjectFactory, ProjectUserFactory, PAttributeTypeFactory, ProjectAttributeFactory, ProjectStatusChoiceFactory, - setup_models, ProjectAttributeTypeFactory, - ProjectUserRoleChoiceFactory, - fake ) -from coldfront.core.project.models import ProjectUserStatusChoice +from coldfront.core.project.models import ProjectUserStatusChoice, Project logging.disable(logging.CRITICAL) @@ -37,8 +35,6 @@ def setUpTestData(cls): cls.nonproject_user = cls.nonproj_allocation_user # add pi_user and project_user to project_user - cls.normal_projuser = ProjectUserFactory(project=cls.project, user=cls.project_user) - attributetype = PAttributeTypeFactory(name='string') cls.projectattributetype = ProjectAttributeTypeFactory(attribute_type=attributetype)# ProjectAttributeType.objects.get(pk=1) @@ -90,13 +86,6 @@ def setUpTestData(cls): cls.projectattribute = ProjectAttributeFactory(value=36238, proj_attr_type=cls.projectattributetype, project=cls.project) cls.url = f'/project/{cls.project.pk}/' - cls.no_allocation_project = ProjectFactory(title=fake.unique.project_title(), - pi=UserFactory(username=fake.unique.user_name())) - - def test_projectdetail_render(self): - # test rendering for project with no allocation - no_allocation_proj_url = f'/project/{self.no_allocation_project.pk}/' - utils.test_user_can_access(self, self.admin_user, no_allocation_proj_url) def test_projectdetail_access(self): """Test project detail page access""" @@ -108,8 +97,6 @@ def test_projectdetail_access(self): # user not belonging to project cannot access utils.test_user_cannot_access(self, self.nonproject_user, self.url) - - def test_projectdetail_permissions(self): """Test project detail page access permissions""" # admin has is_allowed_to_update_project set to True @@ -219,12 +206,14 @@ def test_project_attribute_create_post_required_values(self): self.client.force_login(self.admin_user, backend='django.contrib.auth.backends.ModelBackend') # missing project - response = self.client.post(self.url, data={'proj_attr_type': self.projectattributetype.pk, - 'value': 'test_value'}) + response = self.client.post(self.url, data={ + 'proj_attr_type': self.projectattributetype.pk, 'value': 'test_value' + }) self.assertFormError(response, 'form', 'project', 'This field is required.') # missing value - response = self.client.post(self.url, data={'proj_attr_type': self.projectattributetype.pk, - 'project': self.project.pk}) + response = self.client.post(self.url, data={ + 'proj_attr_type': self.projectattributetype.pk, 'project': self.project.pk + }) self.assertFormError(response, 'form', 'value', 'This field is required.') @@ -234,9 +223,11 @@ def test_project_attribute_create_value_type_match(self): self.client.force_login(self.admin_user, backend='django.contrib.auth.backends.ModelBackend') # test that value must be numeric if proj_attr_type is string - response = self.client.post(self.url, data={'proj_attr_type': self.int_projectattributetype.pk, - 'value': True, - 'project': self.project.pk}) + response = self.client.post(self.url, data={ + 'proj_attr_type': self.int_projectattributetype.pk, + 'value': True, + 'project': self.project.pk + }) self.assertFormError(response, 'form', '', 'Invalid Value True. Value must be an int.') @@ -247,7 +238,9 @@ class ProjectAttributeUpdateTest(ProjectViewTestBase): def setUpTestData(cls): """Set up users and project for testing""" super(ProjectAttributeUpdateTest, cls).setUpTestData() - cls.projectattribute = ProjectAttributeFactory(value=36238, proj_attr_type=cls.projectattributetype, project=cls.project) + cls.projectattribute = ProjectAttributeFactory( + value=36238, proj_attr_type=cls.projectattributetype, project=cls.project + ) cls.url = f'/project/{cls.project.pk}/project-attribute-update/{cls.projectattribute.pk}' @@ -292,11 +285,7 @@ def setUpTestData(cls): """Set up users and project for testing""" super(ProjectListViewTest, cls).setUpTestData() # add 100 projects to test pagination, permissions, search functionality - cls.additional_projects = [ - ProjectFactory(title=fake.unique.project_title(), - pi=UserFactory(username=fake.unique.user_name())) - for i in list(range(100)) - ] + cls.additional_projects = [ProjectFactory() for i in list(range(100))] cls.url = '/project/' ### ProjectListView access tests ### @@ -316,31 +305,27 @@ def test_project_list_access(self): def test_project_list_display_members(self): """Test that project list displays only projects that user is an active member of.""" # deactivated projectuser won't see project on their page - self.normal_projuser.status, _ = ProjectUserStatusChoice.objects.get_or_create(name='Removed') - self.normal_projuser.save() - self.client.force_login(self.normal_projuser.user, backend='django.contrib.auth.backends.ModelBackend') - response = self.client.get(self.url) + self.npu.status, _ = ProjectUserStatusChoice.objects.get_or_create(name='Removed') + self.npu.save() + response = utils.login_and_get_page(self.client, self.normal_projuser, self.url) self.assertEqual(len(response.context['object_list']), 0) def test_project_list_displayall_permission_admin(self): """Test that the projectlist displayall option displays all projects to admin""" url = self.url + '?show_all_projects=on' - self.client.force_login(self.admin_user, backend='django.contrib.auth.backends.ModelBackend') - response = self.client.get(url) - self.assertGreaterEqual(101, len(response.context['object_list'])) + response = utils.login_and_get_page(self.client, self.admin_user, url) + self.assertEqual(len(response.context['object_list']), Project.objects.all().count()) def test_project_list_displayall_permission_pi(self): """Test that the projectlist displayall option displays only the pi's projects to the pi""" url = self.url + '?show_all_projects=on' - self.client.force_login(self.pi_user, backend='django.contrib.auth.backends.ModelBackend') - response = self.client.get(url) + response = utils.login_and_get_page(self.client, self.pi_user, url) self.assertEqual(len(response.context['object_list']), 1) def test_project_list_displayall_permission_project_user(self): """Test that the projectlist displayall option displays only the project user's projects to the project user""" url = self.url + '?show_all_projects=on' - self.client.force_login(self.project_user, backend='django.contrib.auth.backends.ModelBackend') - response = self.client.get(url) + response = utils.login_and_get_page(self.client, self.project_user, url) self.assertEqual(len(response.context['object_list']), 1) @@ -349,18 +334,16 @@ def test_project_list_displayall_permission_project_user(self): def test_project_list_search(self): """Test that project list search works.""" url_base = self.url + '?show_all_projects=on' - self.client.force_login(self.admin_user, backend='django.contrib.auth.backends.ModelBackend') # search by project project_title url = url_base + '&title=' + self.project.title - response = self.client.get(url) + response = utils.login_and_get_page(self.client, self.admin_user, url) self.assertEqual(len(response.context['object_list']), 1) def test_project_list_search_pagination(self): """confirm that navigation to next page of search works as expected""" url = self.url + '?show_all_projects=on' - self.client.force_login(self.admin_user, backend='django.contrib.auth.backends.ModelBackend') - response = self.client.get(url) + response = utils.login_and_get_page(self.client, self.admin_user, url) diff --git a/coldfront/core/test_helpers/factories.py b/coldfront/core/test_helpers/factories.py index 185a0660a..ac4bcc6c0 100644 --- a/coldfront/core/test_helpers/factories.py +++ b/coldfront/core/test_helpers/factories.py @@ -1,41 +1,48 @@ -from django.contrib.auth import get_user_model -from django.contrib.auth.models import User - import factory +from django.contrib.auth.models import User +from django.contrib.auth import get_user_model from factory import SubFactory +from factory.fuzzy import FuzzyChoice +from factory.django import DjangoModelFactory from faker import Faker from faker.providers import BaseProvider, DynamicProvider -from factory.django import DjangoModelFactory from coldfront.core.field_of_science.models import FieldOfScience from coldfront.core.resource.models import ResourceType, Resource -# from coldfront.core.department.models import Department -from coldfront.core.project.models import (Project, - ProjectUser, - ProjectAttribute, - ProjectAttributeType, - ProjectUserRoleChoice, - ProjectUserStatusChoice, - ProjectStatusChoice, - AttributeType as PAttributeType - ) -from coldfront.core.allocation.models import (Allocation, - AllocationUser, - AllocationUserNote, - AllocationAttribute, - AllocationStatusChoice, - AllocationAttributeType, - AllocationChangeRequest, - AllocationChangeStatusChoice, - AllocationAttributeUsage, - AllocationUserStatusChoice, - AllocationAttributeChangeRequest, - AttributeType as AAttributeType - ) +from coldfront.core.project.models import ( + Project, + ProjectUser, + ProjectAttribute, + ProjectAttributeType, + ProjectUserRoleChoice, + ProjectUserStatusChoice, + ProjectStatusChoice, + AttributeType as PAttributeType, +) +from coldfront.core.allocation.models import ( + Allocation, + AllocationUser, + AllocationUserNote, + AllocationAttribute, + AllocationStatusChoice, + AllocationAttributeType, + AllocationChangeRequest, + AllocationChangeStatusChoice, + AllocationAttributeUsage, + AllocationUserStatusChoice, + AllocationAttributeChangeRequest, + AttributeType as AAttributeType, +) from coldfront.core.grant.models import GrantFundingAgency, GrantStatusChoice from coldfront.core.publication.models import PublicationSource +### Default values and Faker provider setup ### + +project_status_choice_names = ['New', 'Active', 'Archived'] +project_user_role_choice_names = ['User', 'Manager'] +field_of_science_names = ['Physics', 'Chemistry', 'Economics', 'Biology', 'Sociology'] +attr_types = ['Date', 'Int', 'Float', 'Text', 'Boolean'] fake = Faker() @@ -52,12 +59,13 @@ def username(self): return f'{first_name}{last_name}'.lower() field_of_science_provider = DynamicProvider( - provider_name="fieldofscience", - elements=['Chemistry', 'Physics', 'Economics', 'Biology', 'Statistics', 'Astrophysics'], + provider_name="fieldofscience", elements=field_of_science_names, ) +attr_type_provider = DynamicProvider(provider_name="attr_types", elements=attr_types) + +for provider in [ColdfrontProvider, field_of_science_provider, attr_type_provider]: + factory.Faker.add_provider(provider) -fake.add_provider(ColdfrontProvider) -fake.add_provider(field_of_science_provider) ### User factories ### @@ -66,7 +74,9 @@ class UserFactory(DjangoModelFactory): class Meta: model = get_user_model() django_get_or_create = ('username',) - username = fake.unique.username() + first_name = factory.Faker('first_name') + last_name = factory.Faker('last_name') + username = factory.LazyAttribute(lambda o: f'{o.first_name}{o.last_name}') email = factory.LazyAttribute(lambda obj: '%s@example.com' % obj.username) is_staff = False is_active = True @@ -80,15 +90,7 @@ class Meta: model = FieldOfScience django_get_or_create = ('description',) - description = fake.fieldofscience() - - -### Department factories ### - -# class DepartmentFactory(DjangoModelFactory): -# class Meta: -# model = Department - + description = factory.Faker('fieldofscience') @@ -104,12 +106,16 @@ class Meta: model = GrantStatusChoice + ### Project factories ### class ProjectStatusChoiceFactory(DjangoModelFactory): class Meta: model = ProjectStatusChoice + # ensure that names are unique django_get_or_create = ('name',) + # randomly generate names from list of default values + # name = FuzzyChoice(project_status_choice_names) name = 'Active' @@ -119,14 +125,13 @@ class Meta: django_get_or_create = ('title',) pi = SubFactory(UserFactory) - title = fake.unique.project_title() - description = fake.sentence() + title = factory.Faker('project_title') + description = factory.Faker('sentence') field_of_science = SubFactory(FieldOfScienceFactory) status = SubFactory(ProjectStatusChoiceFactory) force_review = False requires_review = False - # force_review = fake.boolean() - # requires_review = fake.boolean() + class ProjectUserRoleChoiceFactory(DjangoModelFactory): class Meta: @@ -134,12 +139,14 @@ class Meta: django_get_or_create = ('name',) name = 'User' + class ProjectUserStatusChoiceFactory(DjangoModelFactory): class Meta: model = ProjectUserStatusChoice django_get_or_create = ('name',) name = 'Active' + class ProjectUserFactory(DjangoModelFactory): class Meta: model = ProjectUser @@ -158,7 +165,8 @@ class PAttributeTypeFactory(DjangoModelFactory): class Meta: model = PAttributeType # django_get_or_create = ('name',) - name='Text' + name = factory.Faker('attr_type') + class ProjectAttributeTypeFactory(DjangoModelFactory): class Meta: @@ -176,8 +184,8 @@ class Meta: + ### Publication factories ### -### Publication factories ### class PublicationSourceFactory(DjangoModelFactory): class Meta: model = PublicationSource @@ -198,18 +206,15 @@ class ResourceFactory(DjangoModelFactory): class Meta: model = Resource django_get_or_create = ('name',) - name = fake.unique.resource_name() + name = factory.Faker('resource_name') - description = fake.sentence() + description = factory.Faker('sentence') resource_type = SubFactory(ResourceTypeFactory) - - ### Allocation factories ### - class AllocationStatusChoiceFactory(DjangoModelFactory): class Meta: model = AllocationStatusChoice @@ -221,20 +226,11 @@ class AllocationFactory(DjangoModelFactory): class Meta: model = Allocation django_get_or_create = ('project',) - justification = fake.sentence() + justification = factory.Faker('sentence') status = SubFactory(AllocationStatusChoiceFactory) project = SubFactory(ProjectFactory) - # definition of the many-to-many "resources" field using the ResourceFactory - # to automatically generate one or more of the required Resource objects - # @post_generation - # def resources(self, create, extracted, **kwargs): - # if not create: - # return - # if extracted: - # for resource in extracted: - # self.resources.add(resource) - # else: - # self.resources.add(ResourceFactory()) + is_changeable = True + ### Allocation Attribute factories ### @@ -245,12 +241,15 @@ class Meta: django_get_or_create = ('name',) name='Int' + class AllocationAttributeTypeFactory(DjangoModelFactory): class Meta: model = AllocationAttributeType django_get_or_create = ('name',) name = 'Test attribute type' attribute_type = SubFactory(AAttributeTypeFactory) + is_changeable=True + class AllocationAttributeFactory(DjangoModelFactory): class Meta: @@ -283,13 +282,15 @@ class Meta: django_get_or_create = ('name',) name = 'Pending' + class AllocationChangeRequestFactory(DjangoModelFactory): class Meta: model = AllocationChangeRequest allocation = SubFactory(AllocationFactory) status = SubFactory(AllocationChangeStatusChoiceFactory) - justification = fake.sentence() + justification = factory.Faker('sentence') + class AllocationAttributeChangeRequestFactory(DjangoModelFactory): class Meta: @@ -299,6 +300,8 @@ class Meta: allocation_attribute = SubFactory(AllocationAttributeFactory) new_value = 1000 + + ### Allocation User factories ### class AllocationUserStatusChoiceFactory(DjangoModelFactory): @@ -326,7 +329,7 @@ class Meta: django_get_or_create = ('allocation') allocation = SubFactory(AllocationFactory) author = SubFactory(AllocationUserFactory) - note = fake.sentence() + note = factory.Faker('sentence') @@ -341,45 +344,49 @@ def setup_models(test_case): AAttributeTypeFactory(name=attribute_type) for status in ['Pending', 'Approved', 'Denied']: AllocationChangeStatusChoiceFactory(name=status) - for alloc_attr_type in ['Storage Quota (TB)']: - AllocationAttributeTypeFactory(name=alloc_attr_type, is_private=False, is_changeable=True) + + quota_tb_type = AllocationAttributeTypeFactory(name='Storage Quota (TB)') # users - test_case.admin_user = UserFactory(username='gvanrossum', is_staff=True, is_superuser=True) + test_case.admin_user = UserFactory( + username='gvanrossum', is_staff=True, is_superuser=True + ) # pi is a project admin but not an AllocationUser. - test_case.pi_user = UserFactory(username='sdpoisson', - is_staff=False, is_superuser=False) - test_case.proj_allocation_user = UserFactory(username='ljbortkiewicz', - is_staff=False, is_superuser=False) - test_case.proj_nonallocation_user = UserFactory(username='wkohn', - is_staff=False, is_superuser=False) - test_case.nonproj_allocation_user = UserFactory(username='jsaul', - is_staff=False, is_superuser=False) + test_case.pi_user = UserFactory(username='sdpoisson') + test_case.proj_allocation_user = UserFactory(username='ljbortkiewicz') + test_case.proj_nonallocation_user = UserFactory(username='wkohn') + test_case.nonproj_allocation_user = UserFactory(username='jsaul') test_case.project = ProjectFactory(pi=test_case.pi_user, title="poisson_lab") # allocations - test_case.proj_allocation = AllocationFactory( - project=test_case.project, - is_changeable=True, - ) - test_case.proj_allocation.resources.add(ResourceFactory(name='holylfs10/tier1', id=1)) + test_case.proj_allocation = AllocationFactory(project=test_case.project) + resource = ResourceFactory(name='holylfs10/tier1', id=1) + test_case.proj_allocation.resources.add(resource) # make a quota_bytes allocation attribute - allocation_quota = AllocationQuotaFactory(allocation=test_case.proj_allocation, value=109951162777600) - AllocationAttributeUsageFactory(allocation_attribute=allocation_quota, value=10995116277760) + allocation_quota = AllocationQuotaFactory( + allocation=test_case.proj_allocation, value=109951162777600 + ) + AllocationAttributeUsageFactory( + allocation_attribute=allocation_quota, value=10995116277760 + ) # make a quota TB allocation attribute - allocation_quota_tb = AllocationAttributeFactory(allocation=test_case.proj_allocation, + allocation_quota_tb = AllocationAttributeFactory( + allocation=test_case.proj_allocation, value = 100, - allocation_attribute_type=AllocationAttributeTypeFactory(name='Storage Quota (TB)'), - ) + allocation_attribute_type=quota_tb_type, + ) AllocationAttributeUsageFactory(allocation_attribute=allocation_quota_tb, value=10) # relationships - AllocationUserFactory(user=test_case.proj_allocation_user, allocation=test_case.proj_allocation) - AllocationUserFactory(user=test_case.nonproj_allocation_user, allocation=test_case.proj_allocation) - + for user in [test_case.proj_allocation_user, test_case.nonproj_allocation_user]: + AllocationUserFactory(user=user, allocation=test_case.proj_allocation) manager_role = ProjectUserRoleChoiceFactory(name='Manager') - ProjectUserFactory(user=test_case.pi_user, project=test_case.project, role=manager_role) - test_case.normal_projuser = ProjectUserFactory(user=test_case.proj_allocation_user, - project=test_case.project) - ProjectUserFactory(user=test_case.proj_nonallocation_user, project=test_case.project) + ProjectUserFactory( + user=test_case.pi_user, project=test_case.project, role=manager_role + ) + test_case.npu = ProjectUserFactory(user=test_case.proj_allocation_user, project=test_case.project) + test_case.normal_projuser = test_case.npu.user + ProjectUserFactory( + user=test_case.proj_nonallocation_user, project=test_case.project + ) diff --git a/coldfront/core/test_helpers/fasrc_factories.py b/coldfront/core/test_helpers/fasrc_factories.py index 1ceb0218a..f424e20b8 100644 --- a/coldfront/core/test_helpers/fasrc_factories.py +++ b/coldfront/core/test_helpers/fasrc_factories.py @@ -6,11 +6,12 @@ from coldfront.core.project.models import Project from coldfront.plugins.ifx.models import ProductResource, ProjectOrganization -from coldfront.core.test_helpers.factories import (fake, - ResourceFactory, - UserFactory, - ProjectFactory, - ProjectUserFactory, +from coldfront.core.test_helpers.factories import ( + factory as cf_factory, + UserFactory, + ProjectFactory, + ResourceFactory, + ProjectUserFactory, ) class FacilityFactory(DjangoModelFactory): @@ -108,7 +109,7 @@ def setup_departments(test_case): last_name='Doe', full_name='John Doe', ) - ProjectFactory(title=fake.unique.project_title()) + ProjectFactory() for project in Project.objects.all(): project_title = project.title org = OrganizationFactory( @@ -118,7 +119,7 @@ def setup_departments(test_case): OrgRelationFactory(parent=test_case.school, child=org) - dept2_proj = ProjectFactory(title=fake.unique.project_title(), pi=test_case.dept_member_user) + dept2_proj = ProjectFactory(pi=test_case.dept_member_user) dept2_proj_org = OrganizationFactory( name=dept2_proj.title, rank='lab', org_tree='Harvard' ) diff --git a/coldfront/core/test_helpers/utils.py b/coldfront/core/test_helpers/utils.py index ffcf91d80..901b77d6c 100644 --- a/coldfront/core/test_helpers/utils.py +++ b/coldfront/core/test_helpers/utils.py @@ -1,13 +1,4 @@ -"""Functions for testing the same basic principle across different applications""" -from django.test import Client - - -### Functions for in-situ error checks from command line ### -def login_return_client(username, password): - """return a logged-in client object""" - client = Client() - client.login(username=username, password=password) - return client +"""utility functions for unit and integration testing""" def page_contains_for_user(test_case, user, url, text): """Check that page contains text for user""" @@ -31,8 +22,6 @@ def collect_all_ids_in_listpage(client, listpage): obj_ids.extend([o.id for o in response.context_data['object_list']]) return obj_ids - -### Functions for testing library ### def login_and_get_page(client, user, page): """force login and return get response for page""" client.force_login(user, backend="django.contrib.auth.backends.ModelBackend")