Skip to content

Conversation

LioriE
Copy link
Contributor

@LioriE LioriE commented Oct 5, 2025

Related Issues

https://github.com/descope/etc/issues/5922

Related PRs

branch PR
python-sdk: feat/async-methods (Link) PR #665

Description

Updated test cases to use IsolatedAsyncioTestCase and async test methods. Added async_mode support.
Created testing utils to unify the mocking and asserting between async and sync modes.

Must

  • Tests
  • Documentation (if applicable)

LioriE added 3 commits October 5, 2025 11:21
Updated management test cases to use IsolatedAsyncioTestCase and async test methods. Added async_mode support.
Created testing utils to unify the mocking and asserting between async and sync modes.
@LioriE LioriE mentioned this pull request Oct 5, 2025
2 tasks
@omercnet omercnet requested a review from Copilot October 5, 2025 12:11
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds async test support across the test suite by introducing an async execution mode, wrapping existing SDK calls with a futu_await helper, and parameterizing tox environments to run both sync and async variants. Centralizes HTTP mocking for sync/async parity and extends SSL / configuration related test coverage.

  • Introduces ASYNC_MODE handling and futu_await helper usage in tests
  • Adds unified mock_http_call utility and expands tox matrix (sync + async)
  • Broad refactor of tests to async style (IsolatedAsyncioTestCase) plus new SSL configuration tests

Reviewed Changes

Copilot reviewed 33 out of 34 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tox.ini Expands env matrix to sync/async variants; sets ASYNC_MODE via factor envs.
tests/testutils.py Adds mock_http_call plus async/sync HTTP mocking helpers.
tests/test_webauthn.py Refactors to async tests; uses futu_await and mock_http_call.
tests/test_totp.py Converts TOTP tests to async with unified mocking.
tests/test_sso.py Async migration and mock consolidation for SSO tests.
tests/test_saml.py Async migration and refactored HTTP mocking.
tests/test_password.py Async conversion; uses futu_await and mock_http_call.
tests/test_otp.py Large async refactor; introduces async_mode usage.
tests/test_oauth.py Async test conversion for OAuth flows.
tests/test_magiclink.py Async tests plus additional futu_await wrapping.
tests/test_flask.py Marks Flask integration tests async (decorator tests).
tests/test_enchantedlink.py Async migration; dual-mode JSON handling for async.
tests/test_descope_client.py Async variants of client lifecycle and validation tests.
tests/test_auth.py Async refactor and added SSL configuration tests.
tests/management/test_user.py Extensive async conversion for user management actions.
tests/management/test_tenant.py Async conversion for tenant management operations.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

)
with self.assertRaises(AuthException):
await futu_await(
await futu_await(webauthn.update_finish("t01", "response01"))
Copy link

Copilot AI Oct 5, 2025

Choose a reason for hiding this comment

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

Nested futu_await calls cause the inner awaited result (likely a non-awaitable dict/None) to be passed again to futu_await, which can mask the intended exception behavior or raise an unintended error. Remove the inner futu_await: await futu_await(webauthn.update_finish("t01", "response01")).

Suggested change
await futu_await(webauthn.update_finish("t01", "response01"))
webauthn.update_finish("t01", "response01")

Copilot uses AI. Check for mistakes.

)
self.assertIsNotNone(webauthn.sign_up_finish("t01", "response01"))
self.assertIsNotNone(
await futu_await(webauthn.sign_up_finish("t01", "response01"))
Copy link

Copilot AI Oct 5, 2025

Choose a reason for hiding this comment

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

In test_sign_in_finish you assert sign_up_finish instead of sign_in_finish, which is a copy-paste error and reduces the validity of the sign-in finish test. Replace sign_up_finish with sign_in_finish to validate the correct flow.

Suggested change
await futu_await(webauthn.sign_up_finish("t01", "response01"))
await futu_await(webauthn.sign_in_finish("t01", "response01"))

Copilot uses AI. Check for mistakes.

)
self.assertIsNotNone(webauthn.sign_up_finish("t01", "response01"))
self.assertIsNotNone(
await futu_await(webauthn.sign_up_finish("t01", "response01"))
Copy link

Copilot AI Oct 5, 2025

Choose a reason for hiding this comment

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

In test_update_finish you are calling sign_up_finish instead of update_finish, likely a copy-paste mistake that fails to assert the update finish behavior. Replace sign_up_finish with update_finish.

Suggested change
await futu_await(webauthn.sign_up_finish("t01", "response01"))
await futu_await(webauthn.update_finish("t01", "response01"))

Copilot uses AI. Check for mistakes.

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.

1 participant