From ddc76511ddef4c77a9f7d59b201146282e238424 Mon Sep 17 00:00:00 2001 From: bloodearnest Date: Fri, 14 Feb 2025 09:16:04 +0000 Subject: [PATCH] Use the new create_api_workspace function in tests This replaces the use of literal dicts throughout test setup to use the new factory functions. This improves error detection (linting catch more errors), and will make it easier to change how user data is created. --- tests/functional/conftest.py | 76 +++++++++-------------- tests/functional/test_request_pages.py | 56 ++++------------- tests/functional/test_workspace_pages.py | 33 ++++------ tests/integration/test_auth_views.py | 17 ++--- tests/integration/test_middleware.py | 7 +-- tests/integration/views/test_workspace.py | 56 ++++++----------- tests/unit/test_business_logic.py | 29 +++------ tests/unit/test_login_api.py | 28 +++------ 8 files changed, 100 insertions(+), 202 deletions(-) diff --git a/tests/functional/conftest.py b/tests/functional/conftest.py index 037c74448..6db582fd9 100644 --- a/tests/functional/conftest.py +++ b/tests/functional/conftest.py @@ -66,16 +66,13 @@ def output_checker_user(live_server, context): login_as_user( live_server, context, - { - "username": "test_output_checker", - "workspaces": { - "test-dir2": { - "project_details": {"name": "Project 2", "ongoing": True}, - "archived": False, - } + factories.create_api_user( + username="test_output_checker", + workspaces={ + "test-dir2": factories.create_api_workspace(project="Project 2") }, - "output_checker": True, - }, + output_checker=True, + ), ) @@ -84,16 +81,13 @@ def researcher_user(live_server, context): login_as_user( live_server, context, - { - "username": "test_researcher", - "workspaces": { - "test-dir1": { - "project_details": {"name": "Test Project", "ongoing": True}, - "archived": False, - } + factories.create_api_user( + username="test_researcher", + workspaces={ + "test-dir1": factories.create_api_workspace(project="Test Project") }, - "output_checker": False, - }, + output_checker=False, + ), ) @@ -105,41 +99,31 @@ def dev_users(tmp_path, settings): { "output_checker": { "token": "output_checker", - "details": { - "username": "output_checker", - "fullname": "Output Checker", - "output_checker": True, - "staff": True, - "workspaces": {}, - }, + "details": factories.create_api_user( + username="output_checker", + output_checker=True, + workspaces={}, + ), }, "output_checker_1": { "token": "output_checker_1", - "details": { - "username": "output_checker_1", - "fullname": "Output Checker 1", - "output_checker": True, - "staff": True, - "workspaces": {}, - }, + "details": factories.create_api_user( + username="output_checker_1", + output_checker=True, + workspaces={}, + ), }, "researcher": { "token": "researcher", - "details": { - "username": "researcher", - "fullname": "Researcher", - "output_checker": False, - "staff": False, - "workspaces": { - "test-workspace": { - "project_details": { - "name": "Test Project", - "ongoing": True, - }, - "archived": False, - } + "details": factories.create_api_user( + username="researcher", + output_checker=False, + workspaces={ + "test-workspace": factories.create_api_workspace( + project="Test Project" + ) }, - }, + ), }, } ) diff --git a/tests/functional/test_request_pages.py b/tests/functional/test_request_pages.py index 1826cade0..81309b949 100644 --- a/tests/functional/test_request_pages.py +++ b/tests/functional/test_request_pages.py @@ -12,16 +12,7 @@ def test_request_file_withdraw(live_server, context, page, bll): author = login_as_user( live_server, context, - user_dict={ - "username": "author", - "workspaces": { - "workspace": { - "project_details": {"name": "Project 2", "ongoing": True}, - "archived": False, - } - }, - "output_checker": False, - }, + user_dict=factories.create_api_user(username="author"), ) release_request = factories.create_request_at_status( @@ -56,10 +47,7 @@ def test_request_file_group_context_modal(live_server, context, page): author = login_as_user( live_server, context, - user_dict={ - "username": "author", - "workspaces": ["workspace"], - }, + user_dict=factories.create_api_user(username="author"), ) release_request = factories.create_request_at_status( @@ -93,20 +81,13 @@ def test_request_group_edit_comment_for_author(live_server, context, page, bll): author = login_as_user( live_server, context, - user_dict={ - "username": "author", - "workspaces": { - "workspace": { - "project_details": {"name": "Project 2", "ongoing": True}, - "archived": False, - }, - "pending": { - "project_details": {"name": "Project 2", "ongoing": True}, - "archived": False, - }, + user_dict=factories.create_api_user( + username="author", + workspaces={ + "workspace": factories.create_api_workspace(), + "pending": factories.create_api_workspace(), }, - "output_checker": False, - }, + ), ) pending_release_request = factories.create_request_at_status( @@ -170,11 +151,7 @@ def test_request_group_edit_comment_for_checker( login_as_user( live_server, context, - user_dict={ - "username": "checker", - "workspaces": {}, - "output_checker": True, - }, + user_dict=factories.create_api_user(username="checker", output_checker=True), ) submitted_release_request = factories.create_request_at_status( @@ -221,11 +198,7 @@ def test_request_group_comment_visibility_public_for_checker( login_as_user( live_server, context, - user_dict={ - "username": "checker", - "workspaces": {}, - "output_checker": True, - }, + user_dict=factories.create_api_user(username="checker", output_checker=True), ) checker = factories.create_airlock_user("checker", [], True) @@ -250,10 +223,7 @@ def test_request_group_comment_visibility_public_for_checker( def _workspace_dict(): return { - "workspace": { - "project_details": {"name": "Project 2", "ongoing": True}, - "archived": False, - } + "workspace": factories.create_api_workspace(project="Project 2"), } @@ -291,10 +261,10 @@ def test_request_buttons( file_review_buttons_visible, ): user_data = { - "researcher": dict( + "researcher": factories.create_api_user( username="researcher", workspaces=_workspace_dict(), output_checker=False ), - "checker": dict( + "checker": factories.create_api_user( username="checker", workspaces=_workspace_dict(), output_checker=True ), } diff --git a/tests/functional/test_workspace_pages.py b/tests/functional/test_workspace_pages.py index 21416987d..182e9237b 100644 --- a/tests/functional/test_workspace_pages.py +++ b/tests/functional/test_workspace_pages.py @@ -78,14 +78,15 @@ def test_content_buttons( "researcher": dict( username="researcher", workspaces={ - "workspace": { - "project_details": {"name": "Project 1", "ongoing": ongoing}, - "archived": archived, - } + "workspace": factories.create_api_workspace( + project="Project 1", ongoing=ongoing, archived=archived + ) }, output_checker=False, ), - "checker": dict(username="checker", workspaces={}, output_checker=True), + "checker": factories.create_api_user( + username="checker", workspaces={}, output_checker=True + ), } user = login_as_user(live_server, context, user_data[login_as]) workspace = factories.create_workspace("workspace", user) @@ -223,14 +224,9 @@ def test_file_content_buttons( is_enabled, tooltip, ): - user_data = dict( + user_data = factories.create_api_user( username="author", - workspaces={ - "workspace": { - "project_details": {"name": "Project 1", "ongoing": True}, - "archived": False, - } - }, + workspaces={"workspace": factories.create_api_workspace(project="Project 1")}, output_checker=False, ) user = login_as_user(live_server, context, user_data) @@ -338,15 +334,12 @@ def test_csv_filtering(live_server, page, context, bll): login_as_user( live_server, context, - user_dict={ - "username": "author", - "workspaces": { - "my-workspace": { - "project_details": {"name": "Project 2", "ongoing": True}, - "archived": False, - }, + user_dict=factories.create_api_user( + username="author", + workspaces={ + "my-workspace": factories.create_api_workspace(project="Project 2"), }, - }, + ), ) page.goto( diff --git a/tests/integration/test_auth_views.py b/tests/integration/test_auth_views.py index 2a9c8d49d..c7f0b7798 100644 --- a/tests/integration/test_auth_views.py +++ b/tests/integration/test_auth_views.py @@ -2,6 +2,8 @@ import pytest +from tests import factories + pytestmark = pytest.mark.django_db @@ -28,16 +30,7 @@ def test_login(requests_post, client, settings): api_response = requests_post.return_value api_response.status_code = 200 - api_response.json.return_value = { - "username": "test_user", - "output_checker": False, - "workspaces": { - "workspace": { - "project_details": {"name": "project1", "ongoing": True}, - "archived": False, - }, - }, - } + api_response.json.return_value = factories.create_api_user() assert "user" not in client.session @@ -52,11 +45,11 @@ def test_login(requests_post, client, settings): json={"user": "test_user", "token": "foo bar baz"}, ) - assert client.session["user"]["username"] == "test_user" + assert client.session["user"]["username"] == "testuser" assert client.session["user"]["output_checker"] is False assert client.session["user"]["workspaces"] == { "workspace": { - "project_details": {"name": "project1", "ongoing": True}, + "project_details": {"name": "project", "ongoing": True}, "archived": False, }, } diff --git a/tests/integration/test_middleware.py b/tests/integration/test_middleware.py index 90d101578..e8feaf0ca 100644 --- a/tests/integration/test_middleware.py +++ b/tests/integration/test_middleware.py @@ -23,10 +23,9 @@ def test_middleware_expired_user(airlock_client, responses): session.save() new_workspaces = user.workspaces.copy() - new_workspaces["new_workspace"] = { - "project_details": {"name": "other_project", "ongoing": True}, - "archived": False, - } + new_workspaces["new_workspace"] = factories.create_api_workspace( + project="other_project" + ) responses.post( f"{settings.AIRLOCK_API_ENDPOINT}/releases/authorise", diff --git a/tests/integration/views/test_workspace.py b/tests/integration/views/test_workspace.py index f7385fe70..95a9a2d42 100644 --- a/tests/integration/views/test_workspace.py +++ b/tests/integration/views/test_workspace.py @@ -23,13 +23,7 @@ def test_home_redirects(airlock_client): def test_workspace_view_summary(airlock_client): user = factories.create_airlock_user( - username="testuser", - workspaces={ - "workspace": { - "project_details": {"name": "TESTPROJECT", "ongoing": True}, - "archived": False, - } - }, + workspaces={"workspace": factories.create_api_workspace(project="TESTPROJECT")} ) airlock_client.login_with_user(user) @@ -44,13 +38,13 @@ def test_workspace_view_summary(airlock_client): def test_workspace_view_archived_inactive(airlock_client): user = factories.create_airlock_user( - username="testuser", workspaces={ - "workspace-abc": { - "project_details": {"name": "TESTPROJECT", "ongoing": False}, - "archived": True, - } - }, + "workspace-abc": factories.create_api_workspace( + project="TESTPROJECT", + archived=True, + ongoing=False, + ) + } ) airlock_client.login_with_user(user) @@ -488,30 +482,18 @@ def test_workspaces_index_user_permitted_workspaces(airlock_client): user = factories.create_airlock_user( username="testuser", workspaces={ - "test1a": { - "project_details": {"name": "Project 1", "ongoing": True}, - "archived": True, - }, - "test1b": { - "project_details": {"name": "Project 1", "ongoing": True}, - "archived": False, - }, - "test1c": { - "project_details": {"name": "Project 1", "ongoing": True}, - "archived": False, - }, - "test2b": { - "project_details": {"name": "Project 2", "ongoing": False}, - "archived": False, - }, - "test2a": { - "project_details": {"name": "Project 2", "ongoing": False}, - "archived": False, - }, - "test3": { - "project_details": {"name": "Project 3", "ongoing": True}, - "archived": False, - }, + "test1a": factories.create_api_workspace( + project="Project 1", archived=True + ), + "test1b": factories.create_api_workspace(project="Project 1"), + "test1c": factories.create_api_workspace(project="Project 1"), + "test2b": factories.create_api_workspace( + project="Project 2", ongoing=False + ), + "test2a": factories.create_api_workspace( + project="Project 2", ongoing=False + ), + "test3": factories.create_api_workspace(project="Project 3"), }, ) diff --git a/tests/unit/test_business_logic.py b/tests/unit/test_business_logic.py index 166e74e29..b622e8e7a 100644 --- a/tests/unit/test_business_logic.py +++ b/tests/unit/test_business_logic.py @@ -69,18 +69,9 @@ def test_provider_get_workspaces_for_user(bll, output_checker): factories.create_workspace("bar") factories.create_workspace("not-allowed") workspaces = { - "foo": { - "project_details": {"name": "project 1", "ongoing": True}, - "archived": False, - }, - "bar": { - "project_details": {"name": "project 2", "ongoing": True}, - "archived": True, - }, - "not-exists": { - "project_details": {"name": "project 2", "ongoing": True}, - "archived": False, - }, + "foo": factories.create_api_workspace(project="project 1", archived=False), + "bar": factories.create_api_workspace(project="project 2", archived=True), + "not-exists": factories.create_api_workspace(project="project 3"), } user = factories.create_airlock_user( username="testuser", workspaces=workspaces, output_checker=output_checker @@ -1308,20 +1299,18 @@ def test_add_file_to_request_not_author(bll): # workspace archived ( { - "workspace": { - "project_details": {"name": "p1", "ongoing": True}, - "archived": True, - } + "workspace": factories.create_api_workspace( + project="p1", archived=True + ), }, exceptions.RequestPermissionDenied, ), # project inactive ( { - "workspace": { - "project_details": {"name": "p1", "ongoing": False}, - "archived": False, - } + "workspace": factories.create_api_workspace( + project="p1", ongoing=False + ), }, exceptions.RequestPermissionDenied, ), diff --git a/tests/unit/test_login_api.py b/tests/unit/test_login_api.py index b468ee225..12aa4c95d 100644 --- a/tests/unit/test_login_api.py +++ b/tests/unit/test_login_api.py @@ -3,6 +3,7 @@ import pytest from airlock import login_api +from tests import factories def test_get_user_data_with_dev_users(settings, tmp_path): @@ -14,20 +15,14 @@ def test_get_user_data_with_dev_users(settings, tmp_path): { "test_user": { "token": "foo bar baz", - "details": { - "output_checker": True, - "workspaces": { - "test1": { - "project_details": { - "name": "project1", - "ongoing": True, - }, - "archived": False, - }, + "details": factories.create_api_user( + output_checker=True, + workspaces={ + "test1": factories.create_api_workspace(project="project1") }, - }, + ), }, - } + }, ) ) @@ -50,14 +45,7 @@ def test_get_user_data_with_dev_users_invalid(settings, tmp_path): json.dumps( { "test_user": {"token": "foo bar baz"}, - "details": { - "workspaces": { - "test1": { - "project_details": {"name": "project1", "ongoing": True}, - "archived": False, - }, - } - }, + "details": factories.create_api_user(), } ) )