Skip to content

Expand token-cookie missing-component coverage and clarify test variable naming#447

Open
srozen with Copilot wants to merge 4 commits into
mainfrom
copilot/add-test-for-missing-access-refresh-tokens
Open

Expand token-cookie missing-component coverage and clarify test variable naming#447
srozen with Copilot wants to merge 4 commits into
mainfrom
copilot/add-test-for-missing-access-refresh-tokens

Conversation

Copilot AI commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

test_token_cookies only covered the missing id_token case, leaving gaps for other required cookie components. This PR extends coverage so load_token_from_cookies is validated to return None when any required token cookie is absent, while also improving test variable clarity.

  • Coverage gap addressed

    • Added dedicated tests for missing access_token and missing refresh_token cookie scenarios.
    • Ensures symmetry with existing missing id_token behavior expectations.
  • Readability improvement

    • Renamed the ambiguous map variable in the existing missing-id_token test to explicitly reflect intent.
    • Aligned new test variable names with the same explicit naming pattern.
  • Representative change

    names = token_cookie_names(settings)
    cookie_map_without_access_token = dict(cookie_map)
    cookie_map_without_access_token.pop(names["access_token"])
    
    assert load_token_from_cookies(cookie_map_without_access_token, settings) is None
Original prompt
Please apply the following diffs and create a pull request.
Once the PR is ready, give it a title based on the messages of the fixes being applied.

[{"message":"The test only verifies behavior when the `id_token` cookie is missing. There is no coverage for missing `access_token` or `refresh_token` cookies. If those components are also required for a valid session, their absence should be tested as well to ensure `load_token_from_cookies` consistently returns `None` for any missing required component.","fixFiles":[{"filePath":"bff/tests/test_token_cookies.py","diff":"diff --git a/bff/tests/test_token_cookies.py b/bff/tests/test_token_cookies.py\n--- a/bff/tests/test_token_cookies.py\n+++ b/bff/tests/test_token_cookies.py\n@@ -68,6 +68,36 @@\n     assert load_token_from_cookies(missing_id, settings) is None\n \n \n+def test_token_cookie_missing_access_token_returns_none(app, build_token_payload):\n+    settings = app.extensions[\"bff_settings\"]\n+    response = app.response_class()\n+    token_payload = build_token_payload()\n+\n+    set_token_cookies(response, token_payload, settings)\n+    cookie_map = _extract_cookie_map(response)\n+\n+    names = token_cookie_names(settings)\n+    missing_access = dict(cookie_map)\n+    missing_access.pop(names[\"access_token\"])\n+\n+    assert load_token_from_cookies(missing_access, settings) is None\n+\n+\n+def test_token_cookie_missing_refresh_token_returns_none(app, build_token_payload):\n+    settings = app.extensions[\"bff_settings\"]\n+    response = app.response_class()\n+    token_payload = build_token_payload()\n+\n+    set_token_cookies(response, token_payload, settings)\n+    cookie_map = _extract_cookie_map(response)\n+\n+    names = token_cookie_names(settings)\n+    missing_refresh = dict(cookie_map)\n+    missing_refresh.pop(names[\"refresh_token\"])\n+\n+    assert load_token_from_cookies(missing_refresh, settings) is None\n+\n+\n def test_token_cookie_oversize_raises_error(app, build_token_payload):\n     settings = app.extensions[\"bff_settings\"]\n     response = app.response_class()\n"}]},{"message":"The variable name `missing_id` is ambiguous — it could refer to a missing ID in a general sense. A clearer name like `cookie_map_without_id_token` would better communicate the intent of the variable.","fixFiles":[{"filePath":"bff/tests/test_token_cookies.py","diff":"diff --git a/bff/tests/test_token_cookies.py b/bff/tests/test_token_cookies.py\n--- a/bff/tests/test_token_cookies.py\n+++ b/bff/tests/test_token_cookies.py\n@@ -62,10 +62,10 @@\n     cookie_map = _extract_cookie_map(response)\n \n     names = token_cookie_names(settings)\n-    missing_id = dict(cookie_map)\n-    missing_id.pop(names[\"id_token\"])\n+    cookie_map_without_id_token = dict(cookie_map)\n+    cookie_map_without_id_token.pop(names[\"id_token\"])\n \n-    assert load_token_from_cookies(missing_id, settings) is None\n+    assert load_token_from_cookies(cookie_map_without_id_token, settings) is None\n \n \n def test_token_cookie_oversize_raises_error(app, build_token_payload):\n"}]}]

Copilot AI changed the title [WIP] Add tests for missing access_token and refresh_token cookies Expand token-cookie missing-component coverage and clarify test variable naming Jul 3, 2026
Copilot AI requested a review from srozen July 3, 2026 13:16
Copilot finished work on behalf of srozen July 3, 2026 13:16
@srozen

srozen commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

@codex review

@srozen srozen marked this pull request as ready for review July 3, 2026 13:23
@srozen srozen requested a review from fsouris July 3, 2026 13:23

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 96ea1cac7c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@@ -0,0 +1,15 @@
Metadata-Version: 2.4

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Drop generated egg-info metadata

This adds setuptools-generated package metadata even though the BFF package metadata is already maintained in bff/pyproject.toml, and the repo guidance says generated artifacts should not be hand-edited unless explicitly allowed. Keeping this second copy means the next version, dependency, or file-list change can leave PKG-INFO, requires.txt, or SOURCES.txt stale, so local distribution metadata/package manifests may no longer match the actual project. Please remove the backend_for_frontend.egg-info files rather than committing them with this test-only change.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants