Fixes flaky: test_guest_user_is_not_garbage_collected (#7609)

Author: pcrespov

.github/PULL_REQUEST_TEMPLATE.md

@@ -10,6 +10,7 @@
   ⬆️    Upgrade dependencies.
   📝    Add or update documentation.
   🔨    Add or update development scripts.
+  ✅    Add, update or pass tests.
   🔒️    Fix security issues.
   ⚠️    Changes in ops configuration etc. are required before deploying.
         [ Please add a link to the associated ops-issue or PR, such as in https://github.com/ITISFoundation/osparc-ops-environments or https://git.speag.com/oSparc/osparc-infra ]
@@ -28,32 +29,16 @@ or from https://gitmoji.dev/
 
 
 ## Related issue/s
-
-<!-- Link pull request to an issue
-  SEE https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue
-
-- resolves ITISFoundation/osparc-issues#428
-- fixes #26
--->
+<!-- LINK to other issues and add prefix `closes`, `fixes`, `resolves`-->
 
 
 ## How to test
 
 <!-- Give REVIEWERS some hits or code snippets on how could this be tested -->
 
-## Dev-ops checklist
-
-- [ ] No ENV changes or I properly updated ENV ([read the instruction](https://git.speag.com/oSparc/osparc-ops-deployment-configuration/-/blob/configs/README.md?ref_type=heads#how-to-update-env-variables))
-
-<!-- Some checks that might help your code run stable on production, and help devops assess criticality.
-Modified from https://oschvr.com/posts/what-id-like-as-sre/
+## Dev-ops
 
-- How can DevOps check the health of the service ?
-- How can DevOps safely and gracefully restart the service ?
-- How and why would this code fail ?
-- What kind of metrics are you exposing ?
-- Is there any documentation/design specification for the service ?
-- How (e.g. through which loglines) can DevOps detect unexpected situations that require escalation to human ?
-- What are the resource limitations (CPU, RAM) expected for this service ?
-- Are all relevant variables documented and adjustable via environment variables (i.e. no hardcoded magic numbers) ?
+<!--
+- No changes /updated ENV. SEE https://git.speag.com/oSparc/osparc-ops-deployment-configuration/-/blob/configs/README.md?ref_type=heads#how-to-update-env-variables)
+- SEE docs/devops-checklist.md
 -->

docs/devops-checklist.md

@@ -0,0 +1,15 @@
+# Devops checklist
+
+- No ENV changes or I properly updated ENV ([read the instruction](https://git.speag.com/oSparc/osparc-ops-deployment-configuration/-/blob/configs/README.md?ref_type=heads#how-to-update-env-variables))
+
+- Some checks that might help your code run stable on production, and help devops assess criticality.
+  - How can DevOps check the health of the service ?
+  - How can DevOps safely and gracefully restart the service ?
+  - How and why would this code fail ?
+  - What kind of metrics are you exposing ?
+  - Is there any documentation/design specification for the service ?
+  - How (e.g. through which loglines) can DevOps detect unexpected situations that require escalation to human ?
+  - What are the resource limitations (CPU, RAM) expected for this service ?
+  - Are all relevant variables documented and adjustable via environment variables (i.e. no hardcoded magic numbers) ?
+
+Ref: Modified from https://oschvr.com/posts/what-id-like-as-sre/

services/web/server/tests/unit/with_dbs/04/studies_dispatcher/test_studies_dispatcher_studies_access.py

@@ -234,9 +234,24 @@ async def _assert_redirected_to_study(
         "OSPARC-SIMCORE" in content
     ), f"Expected front-end rendering workbench's study, got {content!s}"
 
-    # Expects fragment to indicate client where to find newly created project
-    m = re.match(r"/study/([\d\w-]+)", response.real_url.fragment)
-    assert m, f"Expected /study/uuid, got {response.real_url.fragment}"
+    # First check if the fragment indicates an error
+    fragment = response.real_url.fragment
+    error_match = re.match(r"/error", fragment)
+    if error_match:
+        # Parse query parameters to extract error details
+        query_params = urllib.parse.parse_qs(
+            fragment.split("?", 1)[1] if "?" in fragment else ""
+        )
+        error_message = query_params.get("message", ["Unknown error"])[0]
+        error_status = query_params.get("status_code", ["Unknown"])[0]
+
+        pytest.fail(
+            f"Redirected to error page: Status={error_status}, Message={error_message}"
+        )
+
+    # Check for study path if not an error
+    m = re.match(r"/study/([\d\w-]+)", fragment)
+    assert m, f"Expected /study/uuid, got {fragment}"
 
     # Expects auth cookie for current user
     assert _is_user_authenticated(session)
@@ -435,7 +450,17 @@ async def enforce_garbage_collect_guest(uid):
 
 
 @pytest.mark.flaky(max_runs=3)
-@pytest.mark.parametrize("number_of_simultaneous_requests", [1, 2, 32])
+@pytest.mark.parametrize(
+    "number_of_simultaneous_requests",
+    [
+        1,
+        2,
+        16,
+        # NOTE: The number of simultaneous anonymous users is limited by system load.
+        # A GuestUsersLimitError is raised if user creation exceeds the MAX_DELAY_TO_CREATE_USER threshold.
+        # This test is flaky due to its dependency on app runtime conditions. Avoid increasing simultaneous requests.
+    ],
+)
 async def test_guest_user_is_not_garbage_collected(
     number_of_simultaneous_requests: int,
     web_server: TestServer,
@@ -454,9 +479,6 @@ async def test_guest_user_is_not_garbage_collected(
 
     async def _test_guest_user_workflow(request_index):
         print("request #", request_index, "-" * 10)
-
-        # TODO: heartbeat is missing here!
-        # TODO: reduce GC activation period to 0.1 secs
         # every guest uses different client to preserve it's own authorization/authentication cookies
         client: TestClient = await aiohttp_client(web_server)
         assert client.app
@@ -495,5 +517,4 @@ async def _test_guest_user_workflow(request_index):
     ]
 
     await asyncio.gather(*request_tasks)
-
     # and now the garbage collector shall delete our users since we are done...