Merge pull request rdmorganiser#1053 from afuetterer/1052-pt

chore: enable flake8-pytest-style (pt) rule in ruff config

conftest.py

@@ -34,7 +34,7 @@ def fixtures():
 
 
 @pytest.fixture(scope='session')
-def django_db_setup(django_db_setup, django_db_blocker, fixtures):
+def django_db_setup(django_db_setup, django_db_blocker, fixtures): # noqa: PT004 - pytest-django requires this name "django_db_setup"
     """Populate database with test data from fixtures directories."""
     with django_db_blocker.unblock():
         call_command('loaddata', *fixtures)

pyproject.toml

@@ -134,6 +134,7 @@ select = [
   "F",    # pyflakes
   "I",    # isort
   "PGH",  # pygrep-hooks
+  "PT",   # flake8-pytest-style
   "RUF",  # ruff
   "UP",   # pyupgrade
   "W",    # pycodestyle
@@ -186,6 +187,10 @@ rest_framework = ["rest_framework"]
   "F821", # undefined-names
 ]
 
+[tool.ruff.lint.flake8-pytest-style]
+fixture-parentheses = false
+parametrize-names-type = "csv"
+
 [tool.pytest.ini_options]
 DJANGO_SETTINGS_MODULE = "config.settings"
 testpaths = ["rdmo/*/tests"]

rdmo/accounts/tests/test_views.py

@@ -38,7 +38,7 @@
 
 
 @pytest.fixture(autouse=True, scope='module')
-def reload_urls_at_teardown():
+def _reload_urls_at_teardown():
     '''Clear the url cache after the test function.'''
     yield
     reload_urls('accounts')

rdmo/core/tests/test_swagger.py

@@ -7,7 +7,7 @@
 )
 
 
-@pytest.mark.parametrize("username,password", users)
+@pytest.mark.parametrize('username,password', users)
 def test_swagger(db, client, username, password):
     client.login(username=username, password=password)
 

rdmo/core/tests/test_utils.py

@@ -24,7 +24,7 @@
 )
 
 
-@pytest.mark.parametrize("url,sanitized_url", urls)
+@pytest.mark.parametrize('url,sanitized_url', urls)
 def test_sanitize_url(url, sanitized_url):
     assert sanitize_url(url) == sanitized_url
 
@@ -33,6 +33,6 @@ def test_join_url():
     assert join_url('https://example.com//', '/terms', 'foo') == 'https://example.com/terms/foo'
 
 
-@pytest.mark.parametrize("human,bytes", human2bytes_test_values)
+@pytest.mark.parametrize('human,bytes', human2bytes_test_values)
 def test_human2bytes(human: Optional[str], bytes: float):
     assert human2bytes(human) == bytes

rdmo/core/tests/test_views.py

@@ -17,7 +17,7 @@ def test_home_anonymous(db, client):
     assert response.status_code == 200
 
 
-@pytest.mark.parametrize("username,password", users)
+@pytest.mark.parametrize('username,password', users)
 def test_home_user(db, client, username, password):
     client.login(username=username, password=password)
     response = client.get(reverse('home'))
@@ -30,7 +30,7 @@ def test_about_anonymous(db, client):
     assert response.status_code == 302
 
 
-@pytest.mark.parametrize("username,password", users)
+@pytest.mark.parametrize('username,password', users)
 def test_about_user(db, client, username, password):
     client.login(username=username, password=password)
     response = client.get(reverse('about'))
@@ -55,7 +55,7 @@ def test_i18n_switcher(db, client):
     assert 'en' in response['Content-Language']
 
 
-@pytest.mark.parametrize("username,password", users)
+@pytest.mark.parametrize('username,password', users)
 def test_can_view_management(db, client, username, password):
     client.login(username=username, password=password)
     response = client.get(reverse('management'))

rdmo/management/tests/conftest.py

@@ -8,8 +8,8 @@
 from rdmo.accounts.utils import set_group_permissions
 
 
-@pytest.fixture(scope="function")
-def e2e_tests_django_db_setup(django_db_setup, django_db_blocker, fixtures):
+@pytest.fixture
+def _e2e_tests_django_db_setup(django_db_setup, django_db_blocker, fixtures):
     """Set up database and populate with fixtures, that get restored for every test case."""
     with django_db_blocker.unblock():
         call_command("loaddata", *fixtures)
@@ -41,8 +41,8 @@ def logout_user(page: Page):
     expect(page).to_have_url('/')
     return page
 
-@pytest.fixture(scope="function")
-def logged_in_user(e2e_tests_django_db_setup, base_url_page, username:str, password: str) -> Page:
+@pytest.fixture
+def logged_in_user(_e2e_tests_django_db_setup, base_url_page, username:str, password: str) -> Page:
     """Log in as admin user through Django login UI, returns logged in page for e2e tests."""
     page = login_user(base_url_page, username, password)
     yield page

rdmo/management/tests/test_commands.py

@@ -9,7 +9,7 @@
 from rdmo.management.tests.helpers_xml import xml_test_files
 
 
-@pytest.mark.parametrize("xml_file_path, error_message", xml_test_files.items())
+@pytest.mark.parametrize("xml_file_path,error_message", xml_test_files.items())
 def test_import(db, settings, xml_file_path, error_message):
     xml_file = Path(settings.BASE_DIR).joinpath(xml_file_path)
     stdout, stderr = io.StringIO(), io.StringIO()

rdmo/management/tests/test_frontend_import_options.py

@@ -28,7 +28,7 @@
 # Defined in filterCheckBoxText in rdmo/management/assets/js/components/import/common/ImportFilters.js
 IMPORT_FILTER_LABEL_TEXT = "Show only new and changes"
 
-@pytest.mark.parametrize("username, password", test_users)  # consumed by fixture
+@pytest.mark.parametrize("username,password", test_users)  # consumed by fixture
 def test_import_and_update_optionsets_in_management(logged_in_user: Page) -> None:
     """Test that each content type is available through the navigation."""
     delete_all_objects([OptionSet, Option])

rdmo/management/tests/test_frontend_import_questions.py

@@ -19,7 +19,7 @@
 
 test_users = [('editor', 'editor')]
 
-@pytest.mark.parametrize("username, password", test_users)  # consumed by fixture
+@pytest.mark.parametrize("username,password", test_users)  # consumed by fixture
 def test_import_catalogs_in_management(logged_in_user: Page) -> None:
     """Test that the catalogs.xml can be imported correctly."""
     delete_all_objects([Catalog, Section, PageModel, QuestionSet, Question])

rdmo/management/tests/test_frontend_management_elements.py

@@ -20,7 +20,7 @@
 
 test_users = [('editor', 'editor')]
 
-@pytest.mark.parametrize("username, password", test_users)
+@pytest.mark.parametrize("username,password", test_users)
 @pytest.mark.parametrize("helper", model_helpers)
 def test_management_navigation(logged_in_user: Page, helper: ModelHelper, username: str, password: str) -> None:
     """Test that each content type is available through the navigation."""
@@ -44,7 +44,7 @@ def test_management_navigation(logged_in_user: Page, helper: ModelHelper, userna
 
 
 
-@pytest.mark.parametrize("username, password", test_users)
+@pytest.mark.parametrize("username,password", test_users)
 @pytest.mark.parametrize("helper", model_helpers)
 def test_management_has_items(logged_in_user: Page, helper: ModelHelper) -> None:
     """Test all items in database are visible in management UI."""
@@ -55,7 +55,7 @@ def test_management_has_items(logged_in_user: Page, helper: ModelHelper) -> None
     expect(items_in_ui).to_have_count(num_items_in_database)
 
 
-@pytest.mark.parametrize("username, password", test_users)
+@pytest.mark.parametrize("username,password", test_users)
 @pytest.mark.parametrize("helper", model_helpers)
 def test_management_nested_view(
     logged_in_user: Page, helper: ModelHelper
@@ -70,7 +70,7 @@ def test_management_nested_view(
         expect(page.locator(".panel-default > .panel-body").first).to_be_visible()
 
 
-@pytest.mark.parametrize("username, password", test_users)
+@pytest.mark.parametrize("username,password", test_users)
 @pytest.mark.parametrize("helper", model_helpers)
 def test_management_create_model(
     logged_in_user: Page, helper: ModelHelper
@@ -107,7 +107,7 @@ def test_management_create_model(
     assert helper.model.objects.get(**query)
 
 
-@pytest.mark.parametrize("username, password", test_users)
+@pytest.mark.parametrize("username,password", test_users)
 @pytest.mark.parametrize("helper", model_helpers)
 def test_management_edit_model(logged_in_user: Page, helper: ModelHelper) -> None:
     page = logged_in_user

rdmo/management/tests/test_merge_attributes.py

@@ -113,7 +113,7 @@ def get_related_affected_instances(attribute) -> list:
 
 
 @pytest.fixture
-def create_new_merge_attributes_and_views(db, settings):
+def _create_new_merge_attributes_and_views(db, settings):
     """ Creates model instances for merge attributes tests """
     example_attributes = Attribute.objects.filter(uri_prefix=EXAMPLE_URI_PREFIX).all()
     create_copies_of_attributes_with_new_uri_prefix(example_attributes, NEW_MERGE_URI_PREFIXES)
@@ -122,7 +122,8 @@ def create_new_merge_attributes_and_views(db, settings):
 
 
 @pytest.mark.parametrize('uri_prefix', NEW_MERGE_URI_PREFIXES)
-def test_that_the_freshly_created_merge_attributes_are_present(db, create_new_merge_attributes_and_views, uri_prefix):
+@pytest.mark.usefixtures("_create_new_merge_attributes_and_views")
+def test_that_the_freshly_created_merge_attributes_are_present(db, uri_prefix):
     merge_attributes = Attribute.objects.filter(uri_prefix=uri_prefix).all()
     assert len(merge_attributes) > 2
     unique_uri_prefixes = set(Attribute.objects.values_list("uri_prefix", flat=True))
@@ -154,8 +155,8 @@ def test_that_the_freshly_created_merge_attributes_are_present(db, create_new_me
 @pytest.mark.parametrize('save', [False, True])
 @pytest.mark.parametrize('delete', [False, True])
 @pytest.mark.parametrize('view', [False, True])
-def test_command_merge_attributes(db, settings, create_new_merge_attributes_and_views,
-                                  source_uri_prefix, save, delete, view):
+@pytest.mark.usefixtures("_create_new_merge_attributes_and_views")
+def test_command_merge_attributes(db, settings, source_uri_prefix, save, delete, view):
     source_attributes = Attribute.objects.filter(uri_prefix=source_uri_prefix).all()
     assert len(source_attributes) > 2
     unique_uri_prefixes = set(Attribute.objects.values_list("uri_prefix", flat=True))
@@ -212,8 +213,8 @@ def test_command_merge_attributes(db, settings, create_new_merge_attributes_and_
 @pytest.mark.parametrize('save', [False, True])
 @pytest.mark.parametrize('delete', [False, True])
 @pytest.mark.parametrize('view', [False, True])
-def test_command_merge_attributes_for_views(db, settings, create_new_merge_attributes_and_views,
-                                            source_uri_prefix, save, delete, view):
+@pytest.mark.usefixtures("_create_new_merge_attributes_and_views")
+def test_command_merge_attributes_for_views(db, settings, source_uri_prefix, save, delete, view):
     source_attributes = Attribute.objects.filter(uri_prefix=source_uri_prefix).all()
     assert len(source_attributes) > 2
     unique_uri_prefixes = set(Attribute.objects.values_list("uri_prefix", flat=True))

rdmo/management/tests/test_viewset_upload.py

@@ -110,7 +110,7 @@ def test_create_empty(db, client, username, password):
 
 
 @pytest.mark.parametrize('username,password', users)
-@pytest.mark.parametrize('xml_file_path, error_message', xml_error_files.items())
+@pytest.mark.parametrize('xml_file_path,error_message', xml_error_files.items())
 def test_create_error(db, client, username, password, xml_file_path, error_message):
     client.login(username=username, password=password)
 

rdmo/projects/tests/test_view_membership_multisite.py

@@ -32,17 +32,18 @@
 sites_domains = ('example.com', 'foo.com', 'bar.com')
 
 
-@pytest.fixture()
-def multisite(settings):
+@pytest.fixture
+def _multisite(settings):
     settings.MULTISITE = True
 
 
 @pytest.mark.parametrize('username,password', users)
 @pytest.mark.parametrize('project_id', projects)
 @pytest.mark.parametrize('membership_role', membership_roles)
 @pytest.mark.parametrize('site_domain', sites_domains)
+@pytest.mark.usefixtures("_multisite")
 def test_get_invite_email_project_path_function(db, client, username, password, project_id,
-                                                membership_role, site_domain, multisite):
+                                                membership_role, site_domain):
     client.login(username=username, password=password)
 
     current_site = Site.objects.get_current()
@@ -69,8 +70,9 @@ def test_get_invite_email_project_path_function(db, client, username, password,
 @pytest.mark.parametrize('project_id', projects)
 @pytest.mark.parametrize('membership_role', membership_roles)
 @pytest.mark.parametrize('site_domain', sites_domains)
+@pytest.mark.usefixtures("_multisite")
 def test_invite_email_project_path_email_body(db, client, username, password, project_id,
-                                              membership_role, site_domain, multisite):
+                                              membership_role, site_domain):
     client.login(username=username, password=password)
 
     current_site = Site.objects.get_current()

rdmo/projects/tests/test_view_project_join.py

@@ -8,8 +8,8 @@
 membership_roles = ('owner', 'manager', 'author', 'guest')
 
 
-@pytest.fixture()
-def use_project_invite_timeout(settings):
+@pytest.fixture
+def _use_project_invite_timeout(settings):
     settings.PROJECT_INVITE_TIMEOUT = -1
 
 
@@ -85,7 +85,8 @@ def test_project_join_error(db, client, membership_role):
 
 
 @pytest.mark.parametrize('membership_role', membership_roles)
-def test_project_join_timeout_error(db, client, membership_role, use_project_invite_timeout):
+@pytest.mark.usefixtures("_use_project_invite_timeout")
+def test_project_join_timeout_error(db, client, membership_role):
     client.login(username='user', password='user')
 
     project = Project.objects.get(id=1)

rdmo/projects/tests/test_viewset_project_value.py

@@ -232,7 +232,7 @@ def test_delete(db, client, username, password, project_id, value_id):
 
 @pytest.mark.parametrize('username,password', users)
 @pytest.mark.parametrize('project_id', projects)
-@pytest.mark.parametrize('value_id, set_values_count', set_values)
+@pytest.mark.parametrize('value_id,set_values_count', set_values)
 def test_set(db, client, username, password, project_id, value_id, set_values_count):
     client.login(username=username, password=password)
     value_exists = Value.objects.filter(project_id=project_id, snapshot=None, id=value_id).exists()

rdmo/questions/tests/test_viewset_catalog_multisite.py

@@ -213,7 +213,7 @@ def test_detail_export(db, client, username, password, export_format):
 
 
 @pytest.mark.parametrize('username,password', users)
-@pytest.mark.parametrize('add_or_remove, has_current_site_check', [('add', True), ('remove', False)])
+@pytest.mark.parametrize('add_or_remove,has_current_site_check', [('add', True), ('remove', False)])
 @pytest.mark.parametrize('locked', [True, False])
 def test_update_catalog_toggle_site(db, client, username, password, add_or_remove, has_current_site_check, locked):
     client.login(username=username, password=password)

rdmo/tasks/tests/test_viewset_task_multisite.py

@@ -140,7 +140,7 @@ def test_detail_export(db, client, username, password, export_format):
 
 
 @pytest.mark.parametrize('username,password', users)
-@pytest.mark.parametrize('add_or_remove, has_current_site_check', [('add', True), ('remove', False)])
+@pytest.mark.parametrize('add_or_remove,has_current_site_check', [('add', True), ('remove', False)])
 @pytest.mark.parametrize('locked', [True, False])
 def test_update_task_toggle_site(db, client, username, password, add_or_remove, has_current_site_check, locked):
     client.login(username=username, password=password)

rdmo/views/tests/test_viewset_view_multisite.py

@@ -130,7 +130,7 @@ def test_detail_export(db, client, username, password, export_format):
 
 
 @pytest.mark.parametrize('username,password', users)
-@pytest.mark.parametrize('add_or_remove, has_current_site_check', [('add', True), ('remove', False)])
+@pytest.mark.parametrize('add_or_remove,has_current_site_check', [('add', True), ('remove', False)])
 @pytest.mark.parametrize('locked', [True, False])
 def test_update_view_toggle_site(db, client, username, password, add_or_remove, has_current_site_check, locked):
     client.login(username=username, password=password)