Merge pull request #167 from opensafely-core/test-user-factory

Refactor how we create and use test users

airlock/users.py

@@ -9,19 +9,28 @@ class User:
     is a convenience for passing that information around.
     """
 
-    id: int
     username: str
     workspaces: dict = dataclasses.field(default_factory=dict)
     output_checker: bool = dataclasses.field(default=False)
 
+    def __post_init__(self):
+        assert isinstance(self.workspaces, dict)
+
     @classmethod
     def from_session(cls, session_data):
         user = session_data.get("user")
         if user is None:
             return
         workspaces = user.get("workspaces", dict())
         output_checker = user.get("output_checker", False)
-        return cls(user["id"], user["username"], workspaces, output_checker)
+        return cls(user["username"], workspaces, output_checker)
+
+    def to_dict(self):
+        return {
+            "username": self.username,
+            "workspaces": self.workspaces,
+            "output_checker": self.output_checker,
+        }
 
     def has_permission(self, workspace_name):
         return (

tests/factories.py

@@ -4,11 +4,21 @@
 from airlock.users import User
 
 
-default_user = User(1, "testuser")
-
-
-def get_user(*, username="testuser", workspaces=[], output_checker=False):
-    return User(1, username, workspaces, output_checker)
+def create_user(username="testuser", workspaces=None, output_checker=False):
+    """Factory to create a user.
+
+    For ease of use, workspaces can either be a list of workspaces, which is
+    converted into an appropriate dict. Or it can be an explicit dict.
+    """
+    if workspaces is None:
+        workspaces_dict = {}
+    elif isinstance(workspaces, dict):
+        workspaces_dict = workspaces
+    else:
+        workspaces_dict = {
+            workspace: {"project": "project"} for workspace in workspaces
+        }
+    return User(username, workspaces_dict, output_checker)
 
 
 def ensure_workspace(workspace_or_name):
@@ -38,7 +48,7 @@ def create_release_request(workspace, user=None, **kwargs):
 
     # create a default user with permission on workspace
     if user is None:
-        user = get_user(workspaces=[workspace.name])
+        user = create_user("author", workspaces=[workspace.name])
 
     release_request = bll._create_release_request(
         workspace=workspace.name, author=user.username, **kwargs
@@ -56,16 +66,15 @@ def write_request_file(request, group, path, contents="", user=None):
 
     # create a default user with permission on workspace
     if user is None:  # pragma: nocover
-        user = get_user(username=request.author, workspaces=[request.workspace])
+        user = create_user(request.author, workspaces=[workspace.name])
 
     bll.add_file_to_request(request, relpath=path, user=user, group_name=group)
 
 
 def create_filegroup(release_request, group_name, filepaths=None):
+    user = create_user(release_request.author, [release_request.workspace])
     for filepath in filepaths or []:  # pragma: nocover
-        bll.add_file_to_request(
-            release_request, filepath, User(1, release_request.author), group_name
-        )
+        bll.add_file_to_request(release_request, filepath, user, group_name)
     return bll._dal._get_or_create_filegroupmetadata(release_request.id, group_name)
 
 

tests/functional/conftest.py

@@ -6,6 +6,8 @@
 from django.conf import settings
 from django.contrib.sessions.models import Session
 
+from tests import factories
+
 
 @pytest.fixture(scope="session", autouse=True)
 def set_env():
@@ -38,8 +40,9 @@ def login_as_user(live_server, context, user_dict):
     Creates a session with relevant user data and
     sets the session cookie.
     """
+    user = factories.create_user(**user_dict)
     session_store = Session.get_session_store_class()()
-    session_store["user"] = user_dict
+    session_store["user"] = user.to_dict()
     session_store.save()
     context.add_cookies(
         [
@@ -58,7 +61,6 @@ def output_checker_user(live_server, context):
         live_server,
         context,
         {
-            "id": "test_output_checker",
             "username": "test_output_checker",
             "workspaces": {"test-dir2": {"project": "Project 2"}},
             "output_checker": True,
@@ -72,7 +74,6 @@ def researcher_user(live_server, context):
         live_server,
         context,
         {
-            "id": "test_researcher",
             "username": "test_researcher",
             "workspaces": {"test-dir1": {"project": "Project 1"}},
             "output_checker": False,

tests/functional/test_e2e.py

@@ -21,7 +21,7 @@ def dev_users(tmp_path, settings):
                         "fullname": "Output Checker",
                         "output_checker": True,
                         "staff": True,
-                        "workspaces": [],
+                        "workspaces": {},
                     },
                 },
                 "researcher": {
@@ -227,12 +227,12 @@ def test_e2e_reject_request(page, live_server, dev_users):
     find_and_click(page.get_by_test_id("nav-requests"))
 
     # View submitted request
-    find_and_click(page.get_by_role("link", name="test-workspace by testuser"))
+    find_and_click(page.get_by_role("link", name="test-workspace by author"))
 
     # Reject request
     find_and_click(page.locator("#reject-request-button"))
     # Page contains rejected message text
     expect(page.locator("body")).to_contain_text("Request has been rejected")
     # Requests view does not show rejected request
     find_and_click(page.get_by_test_id("nav-requests"))
-    expect(page.locator("body")).not_to_contain_text("test-workspace by testuser")
+    expect(page.locator("body")).not_to_contain_text("test-workspace by author")

tests/integration/conftest.py

@@ -1,22 +1,21 @@
 import pytest
+from django.test import Client
 
-from airlock.users import User
+from tests import factories
 
 
-@pytest.fixture
-def client_with_user(client):
-    def _client(session_user):
-        session_user = {"id": 1, "username": "test", **session_user}
-        session = client.session
-        session["user"] = session_user
-        session.save()
-        client.user = User.from_session(session)
-        return client
+class AirlockClient(Client):
+    def login(self, username="testuser", workspaces=None, output_checker=False):
+        user = factories.create_user(username, workspaces, output_checker)
+        self.login_with_user(user)
 
-    return _client
+    def login_with_user(self, user):
+        session = self.session
+        session["user"] = user.to_dict()
+        session.save()
+        self.user = user
 
 
 @pytest.fixture
-def client_with_permission(client_with_user):
-    output_checker = {"output_checker": True}
-    yield client_with_user(output_checker)
+def airlock_client():
+    return AirlockClient()