Skip to content

✅ Fixes flaky: test_guest_user_is_not_garbage_collected #7609

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 6 additions & 21 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 ]
Expand All @@ -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
-->
15 changes: 15 additions & 0 deletions docs/devops-checklist.md
Original file line number Diff line number Diff line change
@@ -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/
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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...
Loading