Skip to content

[codex] Isolate auth redirect URI test imports#7944

Closed
tianmind-studio wants to merge 1 commit into
BasedHardware:mainfrom
tianmind-studio:codex/windows-auth-redirect-jwt-stub
Closed

[codex] Isolate auth redirect URI test imports#7944
tianmind-studio wants to merge 1 commit into
BasedHardware:mainfrom
tianmind-studio:codex/windows-auth-redirect-jwt-stub

Conversation

@tianmind-studio

@tianmind-studio tianmind-studio commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Stub optional jwt / jwt.algorithms.RSAAlgorithm import surfaces in test_auth_redirect_uri.py so the auth redirect tests collect in lightweight Windows backend environments.
  • Add a temporary python_multipart stub while importing routers.auth, matching FastAPI's import-time Form route check without requiring the full multipart dependency for this test.
  • Restore the real utils package path and ensure the Redis auth helper names exist when another stub-heavy test leaves lightweight package modules in sys.modules.

Root cause

test_auth_redirect_uri.py exercises redirect URI validation and auth-code binding, but importing routers.auth also imports optional Apple JWT helpers and builds FastAPI Form endpoints. In this Windows lightweight backend venv, standalone collection failed before any assertions ran:

ModuleNotFoundError: No module named 'jwt'

After the JWT stub, FastAPI's route construction also required python-multipart. Running after test_action_item_date_validation.py exposed a separate order dependency where that test leaves utils / database.redis_db stubs that do not satisfy routers.auth imports.

Refresh

  • Rebased on main at 9ed12aea07.
  • Current head: 5486ea1a08.

Status

Superseded by #7983, which includes this Windows/lightweight-backend import isolation for backend/tests/unit/test_auth_redirect_uri.py plus the higher-priority PKCE auth-code fix for #7692. Closing this PR to keep the maintainer queue focused on #7983.

Validation

  • python -m pytest backend\tests\unit\test_auth_redirect_uri.py --collect-only -q --tb=short -> 46 tests collected
  • python -m pytest backend\tests\unit\test_auth_redirect_uri.py -q --tb=short -> 43 passed, 3 skipped
  • python -m pytest backend\tests\unit\test_action_item_date_validation.py backend\tests\unit\test_auth_redirect_uri.py -q --tb=short -> 69 passed, 3 skipped
  • python -m pytest backend\tests\unit\test_auth_redirect_uri.py backend\tests\unit\test_action_item_date_validation.py -q --tb=short -> 69 passed, 3 skipped
  • python -m py_compile backend\tests\unit\test_auth_redirect_uri.py
  • python -m black --line-length 120 --skip-string-normalization --check backend\tests\unit\test_auth_redirect_uri.py
  • git diff --check origin/main...HEAD
  • scripts/pre-commit

@kodjima33 kodjima33 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Test-only stabilization (import isolation / stubs); zero product risk, CI not failing.

Stub optional JWT and multipart import surfaces so auth redirect URI tests collect in lightweight Windows backend environments.

Restore the real utils package path and fill required Redis auth helpers when earlier tests leave lightweight package stubs in sys.modules.
@tianmind-studio tianmind-studio force-pushed the codex/windows-auth-redirect-jwt-stub branch from 836e23f to 5486ea1 Compare June 16, 2026 15:11

Copy link
Copy Markdown
Contributor Author

Closing this as superseded by #7983.

#7983 includes the same Windows/lightweight-backend import isolation for backend/tests/unit/test_auth_redirect_uri.py and also carries the higher-priority PKCE auth-code fix for #7692. Keeping #7983 as the single review target should avoid duplicate maintainer queue noise while preserving the test stabilization work.

@github-actions

Copy link
Copy Markdown
Contributor

Hey @tianmind-studio 👋

Thank you so much for taking the time to contribute to Omi! We truly appreciate you putting in the effort to submit this pull request.

After careful review, we've decided not to merge this particular PR. Please don't take this personally — we genuinely try to merge as many contributions as possible, but sometimes we have to make tough calls based on:

  • Project standards — Ensuring consistency across the codebase
  • User needs — Making sure changes align with what our users need
  • Code best practices — Maintaining code quality and maintainability
  • Project direction — Keeping aligned with our roadmap and vision

Your contribution is still valuable to us, and we'd love to see you contribute again in the future! If you'd like feedback on how to improve this PR or want to discuss alternative approaches, please don't hesitate to reach out.

Thank you for being part of the Omi community! 💜

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.

2 participants