Skip to content

✅ Add tests for OAuth2AuthorizationCodeBearer with auto_error=False - #15859

Open
Eikentech wants to merge 4 commits into
fastapi:masterfrom
Eikentech:pr-oauth2-authcode-optional
Open

Eikentech wants to merge 4 commits into
fastapi:masterfrom
Eikentech:pr-oauth2-authcode-optional

Conversation

@Eikentech

@Eikentech Eikentech commented Jun 28, 2026

Copy link
Copy Markdown

Summary

This PR adds the missing optional-mode test (mirroring the existing test_security_oauth2_password_bearer_optional.py) and removes the now-unnecessary # pragma: nocover.

The fix

  1. New test tests/test_security_oauth2_authorization_code_bearer_optional.pyauto_error=False, covering:
    • no Authorization header → returns None (handler returns the "create an account" message),
    • a valid Bearer token → returns the token,
    • a non-Bearer scheme → returns None,
    • the OpenAPI schema (unchanged from the existing auth-code test).
  2. Remove the # pragma: nocover on the return None line, since the branch is now genuinely covered.

Notes

This is a coverage-consistency fix, not a behaviour change — fastapi/security/oauth2.py runtime behaviour is
unchanged; the PR only adds the missing test for an already-documented branch and drops the coverage-exclusion
pragma it was hidden behind.

@codspeed

codspeed Bot commented Jun 28, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 20 untouched benchmarks


Comparing Eikentech:pr-oauth2-authcode-optional (6235e0a) with master (4f15548)1

Open in CodSpeed

Footnotes

  1. No successful run was found on master (a64dfbb) during the generation of this report, so 4f15548 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@YuriiMotov YuriiMotov changed the title test: cover OAuth2AuthorizationCodeBearer auto_error=False branch ✅ Add tests for OAuth2AuthorizationCodeBearer with auto_error=False Jun 29, 2026
@YuriiMotov

Copy link
Copy Markdown
Member

@Eikentech, could you please review the PR description and make it shorter?
For now it looks like AI-generated and not properly reviewed by human

@YuriiMotov YuriiMotov added internal Internal changes waiting labels Jun 29, 2026
@Eikentech

Copy link
Copy Markdown
Author

@YuriiMotov , I have updated the PR description, pls check. Thanks

@github-actions github-actions Bot removed the waiting label Jul 3, 2026

@YuriiMotov YuriiMotov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

Implementation is as in test_security_oauth2_password_bearer_optional.py.
See the diff in details

Details
--- a/tests/test_security_oauth2_password_bearer_optional.py
+++ b/tests/test_security_oauth2_password_bearer_optional.py
@@ -1,11 +1,13 @@
 from fastapi import FastAPI, Security
-from fastapi.security import OAuth2PasswordBearer
+from fastapi.security import OAuth2AuthorizationCodeBearer
 from fastapi.testclient import TestClient
 from inline_snapshot import snapshot
 
 app = FastAPI()
 
-oauth2_scheme = OAuth2PasswordBearer(tokenUrl="/token", auto_error=False)
+oauth2_scheme = OAuth2AuthorizationCodeBearer(
+    authorizationUrl="authorize", tokenUrl="token", auto_error=False
+)
 
 
 @app.get("/items/")
@@ -31,7 +33,7 @@ def test_token():
 
 
 def test_incorrect_token():
-    response = client.get("/items", headers={"Authorization": "Notexistent testtoken"})
+    response = client.get("/items", headers={"Authorization": "Non-existent testtoken"})
     assert response.status_code == 200, response.text
     assert response.json() == {"msg": "Create an account first"}
 
@@ -54,15 +56,21 @@ def test_openapi_schema():
                         },
                         "summary": "Read Items",
                         "operationId": "read_items_items__get",
-                        "security": [{"OAuth2PasswordBearer": []}],
+                        "security": [{"OAuth2AuthorizationCodeBearer": []}],
                     }
                 }
             },
             "components": {
                 "securitySchemes": {
-                    "OAuth2PasswordBearer": {
+                    "OAuth2AuthorizationCodeBearer": {
                         "type": "oauth2",
-                        "flows": {"password": {"scopes": {}, "tokenUrl": "/token"}},
+                        "flows": {
+                            "authorizationCode": {
+                                "authorizationUrl": "authorize",
+                                "tokenUrl": "token",
+                                "scopes": {},
+                            }
+                        },
                     }
                 }
             },

@Eikentech

Copy link
Copy Markdown
Author

@YuriiMotov , how this PR can be merged ?

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

Labels

internal Internal changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants