Small structural refactor, fix factory imports, update Docs

Makefile

@@ -111,7 +111,7 @@ functional-tests-down:  ## Stop the functional tests docker compose dependencies
 
 .PHONY: functional-tests-run
 functional-tests-run:  ## Run the functional tests
-	poetry run behave ./functional_tests
+	DJANGO_SETTINGS_MODULE=cms.settings.functional_test poetry run behave functional_tests
 
 .PHONY: functional-tests
 functional-tests: functional-tests-up functional-tests-run functional-tests-down  ## Run the functional tests with backing services (all in one)

functional_tests/README.md

@@ -2,28 +2,28 @@
 
 <!-- TOC -->
 
--   [Structure](#structure)
--   [Dependencies](#dependencies)
-    -   [App Instance For Test Development](#app-instance-for-test-development)
-    -   [Clearing and Initialising the Functional Test Development Database](#clearing-and-initialising-the-functional-test-development-database)
--   [Running the Tests](#running-the-tests)
-    -   [Playwright Options](#playwright-options)
--   [Viewing Failure Traces](#viewing-failure-traces)
-    -   [Viewing the Failure Trace from GitHub Actions](#viewing-the-failure-trace-from-github-actions)
--   [Test Code Standards and Style Guide](#test-code-standards-and-style-guide)
-    -   [Context Use](#context-use)
-    -   [Sharing Code Between Steps](#sharing-code-between-steps)
-    -   [Step wording](#step-wording)
-    -   [Assertions](#assertions)
-    -   [Step parameter types](#step-parameter-types)
--   [How the Tests Work](#how-the-tests-work)
-    -   [Django Test Runner and Test Case](#django-test-runner-and-test-case)
-    -   [Database Snapshot and Restore](#database-snapshot-and-restore)
-    -   [Playwright](#playwright)
--   [Why Aren't We Using Existing Django Testing Modules?](#why-arent-we-using-existing-django-testing-modules)
-    -   [Pytest-BDD](#pytest-bdd)
-    -   [Behave-Django](#behave-django)
-    <!-- TOC -->
+- [Structure](#structure)
+- [Dependencies](#dependencies)
+    - [App Instance For Test Development](#app-instance-for-test-development)
+    - [Clearing and Initialising the Functional Test Development Database](#clearing-and-initialising-the-functional-test-development-database)
+- [Running the Tests](#running-the-tests)
+    - [Playwright Options](#playwright-options)
+- [Viewing Failure Traces](#viewing-failure-traces)
+    - [Viewing the Failure Trace from GitHub Actions](#viewing-the-failure-trace-from-github-actions)
+- [Test Code Standards and Style Guide](#test-code-standards-and-style-guide)
+    - [Context Use](#context-use)
+    - [Sharing Code Between Steps](#sharing-code-between-steps)
+    - [Step wording](#step-wording)
+    - [Assertions](#assertions)
+    - [Step parameter types](#step-parameter-types)
+- [How the Tests Work](#how-the-tests-work)
+    - [Django Test Runner and Test Case](#django-test-runner-and-test-case)
+    - [Database Snapshot and Restore](#database-snapshot-and-restore)
+    - [Playwright](#playwright)
+- [Why Aren't We Using Existing Django Testing Modules?](#why-arent-we-using-existing-django-testing-modules)
+    - [Pytest-BDD](#pytest-bdd)
+    - [Behave-Django](#behave-django)
+  <!-- TOC -->
 
 ## Structure
 
@@ -105,12 +105,28 @@ poetry run dslr --url postgresql://ons:ons@localhost:15432/ons restore <SNAPSHOT
 See the [main README functional tests section](/README.md#run-the-functional-tests) for the basic commands for running
 the tests.
 
+### Other Methods of Running Tests
+
+If you wish to run the tests directly (e.g. running individual scenarios on the command line or from an IDE), then you
+first need to start the test dependencies in the background (see [Dependencies](#dependencies)).
+
+Then you need to ensure the Django settings environment variable is set correctly to `cms.settings.functional_test`
+where you are running the tests:
+
+```shell
+DJANGO_SETTINGS_MODULE=cms.settings.functional_test
+```
+
+For running in the command line you can export this variable, or prefix the command to run the tests with it.
+
+For running from an IDE, you may need to configure the run configuration to include this variable.
+
 ### Playwright Options
 
 Some Playwright configuration options can be passed in through environment variables
 
 | Variable   | Description                                                                                                                                                                    | Default                        |
-| ---------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------ |
+|------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|--------------------------------|
 | HEADLESS   | Toggle headless browser mode, set to "False" to show the browser window                                                                                                        | True                           |
 | SLOW_MO    | Sets the Playwright slow mo mode in milliseconds                                                                                                                               | 0                              |
 | BROWSER    | Set the browser for playwright to use, must be one of `chromium`, `firefox`, or `webkit`.<br/> NOTE: Currently only chromium is supported and tests may fail in other browsers | chromium                       |
@@ -132,13 +148,30 @@ Our GitHub Action is configured to save traces of any failed scenario and upload
     ```shell
     unzip <path_to_file>
     ```
-    (note that un-archiving using MacOS finder may not work as it recursively unzips the files inside, where we need the
-    files inside to remain zipped)
+   (note that un-archiving using MacOS finder may not work as it recursively unzips the files inside, where we need the
+   files inside to remain zipped)
 1. This should leave you with a zip file for each failed scenario
 1. Open the traces zip files one at a time using the [Playwright Trace Viewer](https://playwright.dev/docs/trace-viewer)
 
 You should then be able to step through the failed tests and get a better idea of the state and cause of the failure.
 
+## Test Data Setup
+
+Some tests may require objects to be set up in the database, such as a user or set of pages that the feature relies
+upon. For this, we can use [Factory Boy](https://factoryboy.readthedocs.io/en/stable/orms.html#django) to see data
+directly into the database.
+
+### Importing Factories
+
+Factories that use Django models rely on Django being initialised. This causes issues if they are imported at a module
+level in the tests, as those imports then get run before Django has initialised, so can fail with a error like:
+
+```
+Models aren't loaded yet.
+```
+
+To work around this, the Factory classes must be imported within the step functions, so that
+
 ## Test Code Standards and Style Guide
 
 ### Context Use
@@ -170,7 +203,10 @@ def step_to_do_a_thing(context):
 ### Sharing Code Between Steps
 
 Step files should not import code from other step files, where code can be shared between steps they should either be in
-the same file, or the shared code should be factored out into the utilities module.
+the same file, or the shared code should be factored out into the [step_helpers](step_helpers) module.
+
+This is to avoid potential circular imports and make it clear which code is specific to certain steps, and which is
+reusable across any steps.
 
 ### Step wording
 

functional_tests/environment.py

@@ -1,22 +1,27 @@
 import os
 from pathlib import Path
 
+import django
 from behave import use_fixture
 from behave.model import Scenario
 from behave.model_core import Status
 from behave.runner import Context
 from playwright.sync_api import BrowserContext, Page, Playwright, sync_playwright
 
 from functional_tests.behave_fixtures import django_test_case, django_test_runner
+from functional_tests.step_helpers.utilities import str_to_bool
+
+# The factory classes require Django to have been set up at their import time.
+# To ensure Django set up happens before that point, we call setup at the module level here.
+# This will get called again during the test runner setup in the before_all hook,
+# but that happens too late to solve import time issues.
+django.setup()
 
 
 def before_all(context: Context):
     """Runs once before all tests.
     Sets up playwright browser and context to be used in all scenarios.
     """
-    # Ensure Django uses the functional test settings
-    os.environ["DJANGO_SETTINGS_MODULE"] = "cms.settings.functional_test"
-
     # This is required for Django to run within a Poetry shell
     os.environ["DJANGO_ALLOW_ASYNC_UNSAFE"] = "True"
 
@@ -94,22 +99,11 @@ def after_scenario(context: Context, scenario: Scenario):
     if context.playwright_trace and scenario.status == Status.failed:
         # If the scenario failed, write the trace chunk out to a file, which will be prefixed with the scenario name
         context.browser_context.tracing.stop_chunk(
-            path=context.playwright_traces_dir.joinpath(f"{scenario.name}_failure_trace.zip")
+            path=context.playwright_traces_dir.joinpath(f"{scenario.name.replace(' ', '_')}_failure_trace.zip")
         )
 
     elif context.playwright_trace:
         # Else end the trace chunk without saving to a file
         context.browser_context.tracing.stop_chunk()
 
     context.page.close()
-
-
-def str_to_bool(bool_string: str) -> bool:
-    """Takes a string argument which indicates a boolean, and returns the corresponding boolean value.
-    raises ValueError if input string is not one of the recognized boolean like values.
-    """
-    if bool_string.lower() in ("yes", "true", "t", "y", "1"):
-        return True
-    if bool_string.lower() in ("no", "false", "f", "n", "0"):
-        return False
-    raise ValueError(f"Invalid input: {bool_string}")

functional_tests/factories.py → functional_tests/step_helpers/factories.py

@@ -1,7 +1,7 @@
 from django.contrib.auth.hashers import make_password
 from factory.django import DjangoModelFactory
 
-DEFAULT_TEST_PASSWORD = "default_test_password"  # pragma: allowlist secret # noqa: S105
+TEST_USER_PASSWORD = "test_user_password"  # pragma: allowlist secret # noqa: S105
 
 
 class UserFactory(DjangoModelFactory):
@@ -11,7 +11,9 @@ class Meta:
         model = "users.User"
 
     username = "test_user"
-    password = make_password(DEFAULT_TEST_PASSWORD)
+    # Use make_password to hash the password since this being stored directly in the database
+    password = make_password(TEST_USER_PASSWORD)
+
     is_active = True
     is_staff = True
     is_superuser = True  # This is currently required to log into admin site

functional_tests/step_helpers/users.py

@@ -0,0 +1,7 @@
+from functional_tests.step_helpers.factories import TEST_USER_PASSWORD, UserFactory
+
+
+def create_cms_admin_user() -> tuple[str, str]:
+    """Creates a CMS admin user using a factory, returns the username and password."""
+    user = UserFactory()
+    return user.username, TEST_USER_PASSWORD

functional_tests/step_helpers/utilities.py

@@ -0,0 +1,9 @@
+def str_to_bool(bool_string: str) -> bool:
+    """Takes a string argument which indicates a boolean, and returns the corresponding boolean value.
+    raises ValueError if input string is not one of the recognized boolean like values.
+    """
+    if bool_string.lower() in ("yes", "true", "t", "y", "1"):
+        return True
+    if bool_string.lower() in ("no", "false", "f", "n", "0"):
+        return False
+    raise ValueError(f"Invalid input: {bool_string}")

functional_tests/steps/cms_login.py

@@ -2,6 +2,8 @@
 from behave.runner import Context
 from playwright.sync_api import expect
 
+from functional_tests.step_helpers.users import create_cms_admin_user
+
 
 @given("the user is a CMS admin")  # pylint: disable=E1102
 def user_is_cms_admin(context: Context) -> None:
@@ -18,7 +20,6 @@ def cms_admin_navigates_to_beta_homepage(context: Context) -> None:
 @when("they enter a their valid username and password and click login")  # pylint: disable=E1102
 def enter_a_valid_username_and_password_and_sign_in(context: Context) -> None:
     """Enter the username and password and click login."""
-    expect(context.page).to_have_url(f"{context.base_url}/admin/login/")
     context.page.get_by_placeholder("Enter your username").fill(context.username)
     context.page.get_by_placeholder("Enter password").fill(context.password)
     context.page.get_by_role("button", name="Sign in").click()
@@ -37,18 +38,6 @@ def user_logs_into_the_admin_site(context: Context) -> None:
     """Creates a user and logs into the admin site."""
     context.username, context.password = create_cms_admin_user()
     context.page.goto(f"{context.base_url}/admin/login/")
-    expect(context.page).to_have_url(f"{context.base_url}/admin/login/")
     context.page.get_by_placeholder("Enter your username").fill(context.username)
     context.page.get_by_placeholder("Enter password").fill(context.password)
     context.page.get_by_role("button", name="Sign in").click()
-
-
-def create_cms_admin_user() -> tuple[str, str]:
-    """Creates a CMS admin user using a factory, returns the username and password."""
-    # TODO this import fails at the top level with error:  pylint: disable=W0511
-    #  "django.core.exceptions.AppRegistryNotReady: Models aren't loaded yet."
-    #   Find a better solution
-    from functional_tests.factories import DEFAULT_TEST_PASSWORD, UserFactory  # pylint: disable=C0415
-
-    user = UserFactory()
-    return user.username, DEFAULT_TEST_PASSWORD

functional_tests/steps/contact_details_snippet.py

@@ -1,13 +1,10 @@
 from behave import given  # pylint: disable=E0611
 from behave.runner import Context
 
+from functional_tests.step_helpers.factories import ContactDetailsFactory
+
 
 @given("a contact details snippet exists")
 def create_contact_details_snippet(context: Context):
     """Create a contact details snippet."""
-    # TODO this import fails at the top level with error:  pylint: disable=W0511
-    #  "django.core.exceptions.AppRegistryNotReady: Models aren't loaded yet."
-    #   Find a better solution
-    from functional_tests.factories import ContactDetailsFactory  # pylint: disable=C0415
-
     context.contact_details_snippet = ContactDetailsFactory()