-
Notifications
You must be signed in to change notification settings - Fork 3
feat(tests): add E2E test suite for authentication flows #1126
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
base: master
Are you sure you want to change the base?
Conversation
Add comprehensive E2E test suite for budapp authentication: - Login/logout flows with token validation - User registration with data verification - Token refresh mechanics - Password reset flows - Protected endpoint access control - User profile data verification after registration Infrastructure: - k3d/kind cluster setup scripts - Helm values for E2E environment - Dapr component configurations - pytest fixtures and helpers Also adds RATE_LIMIT_ENABLED config to budapp for disabling rate limiting in test environments. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Summary of ChangesHello @dittops, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new, comprehensive end-to-end (E2E) test suite for the budapp's authentication flows. The primary goal is to ensure the reliability and correctness of user authentication mechanisms across various deployment environments, including Kubernetes clusters and local Docker setups. This enhancement significantly improves the confidence in the core user access functionalities by validating complete user journeys and critical security aspects like token management and rate limiting. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a comprehensive and well-structured E2E test suite for authentication flows. The test infrastructure is robust, with support for k3d, Kind, and existing Kubernetes clusters, which greatly improves developer experience. The tests themselves are thorough, covering positive and negative paths, validation, and security aspects like rate limiting and privilege escalation. The use of helper classes and fixtures makes the tests clean and maintainable.
I've identified a few areas for improvement, mainly around configuration and dependency management to enhance stability and reproducibility. This includes pinning Docker image versions for Dapr, specifying the Kind node image version, fixing a placeholder for a crypto key, and pinning Python dependencies.
Overall, this is an excellent contribution that significantly improves the testing story for the project.
| asymmetricKey: | | ||
| -----BEGIN RSA PRIVATE KEY----- | ||
| MIIEpAIBAAKCAQEA0Z3VS5JJcds3xfn/ygWyF8PbnGy0AHB7MvGj2yDryPEut5hy | ||
| e2e-test-asymmetric-key-placeholder | ||
| -----END RSA PRIVATE KEY----- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| image: daprio/placement:edge | ||
| container_name: e2e-placement | ||
| command: ["./placement", "--port", "50006"] | ||
| ports: | ||
| - "50006:50006" | ||
| networks: | ||
| - e2e-network | ||
|
|
||
| e2e-scheduler: | ||
| image: daprio/dapr |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For better stability and reproducibility of the test environment, it's recommended to pin the Dapr images to specific versions instead of using :edge or an implicit :latest. Using floating tags can introduce unexpected changes and cause test flakiness. The same applies to the daprio/daprd:edge image on line 181.
For example:
e2e-placement:
image: daprio/placement:1.13.2
...
e2e-scheduler:
image: daprio/dapr:1.13.2
...And for the sidecar:
e2e-budapp-sidecar:
image: daprio/daprd:1.13.2
...Please use the Dapr version that matches the one being used in production or development.
| nodes: | ||
| - role: control-plane |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "redis_url": os.getenv( | ||
| "E2E_REDIS_URL", | ||
| "redis://default:e2e-redis-password@localhost:30379/2" | ||
| ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Redis password is hardcoded in the default redis_url. It's better to construct this URL from environment variables to keep configuration consistent and flexible, especially since other credentials are being loaded from the environment. For example, you could introduce E2E_REDIS_PASSWORD in your .env.e2e.sample and use it to build the default URL.
| # E2E Test Dependencies | ||
|
|
||
| # Core testing | ||
| pytest>=7.4.0 | ||
| pytest-asyncio>=0.21.0 | ||
| pytest-timeout>=2.1.0 | ||
| pytest-xdist>=3.3.0 | ||
| pytest-html>=4.0.0 | ||
| pytest-cov>=4.1.0 | ||
| pytest-mock>=3.11.0 | ||
|
|
||
| # HTTP clients | ||
| httpx>=0.24.0 | ||
| aiohttp>=3.8.0 | ||
|
|
||
| # Data generation | ||
| faker>=19.0.0 | ||
|
|
||
| # Database clients | ||
| asyncpg>=0.28.0 | ||
| psycopg2-binary>=2.9.0 | ||
| redis>=4.6.0 | ||
| clickhouse-driver>=0.2.6 | ||
|
|
||
| # Utilities | ||
| tenacity>=8.2.0 # Retry logic | ||
| python-dotenv>=1.0.0 # Environment variables | ||
| pydantic>=2.0.0 # Data validation | ||
| pyyaml>=6.0 # YAML parsing | ||
|
|
||
| # Dapr SDK | ||
| dapr>=1.12.0 | ||
|
|
||
| # Date/time utilities | ||
| python-dateutil>=2.8.0 | ||
|
|
||
| # JSON utilities | ||
| orjson>=3.9.0 | ||
|
|
||
| # Reporting | ||
| pytest-json-report>=1.5.0 | ||
|
|
||
| # Development | ||
| ipdb>=0.13.0 # Debugging |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dependencies in this file are not pinned to specific versions (e.g., pytest>=7.4.0). This can lead to non-reproducible builds and test environments, as pip install might pull newer, potentially breaking versions of these packages. It is a best practice to pin dependencies to exact versions (e.g., pytest==7.4.0). Consider using a tool like pip-compile from pip-tools to manage your dependencies and generate a fully pinned requirements.txt file.
There was a problem hiding this comment.
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.
ℹ️ 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".
| environment: | ||
| APP_NAME: budapp | ||
| NAMESPACE: e2e | ||
| LOG_LEVEL: DEBUG | ||
| APP_PORT: "9081" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docker-compose E2E env still enforces auth rate limits
The new RATE_LIMIT_ENABLED flag is only wired into the Helm E2E values; the Docker Compose E2E stack leaves the budapp container with rate limiting enabled. The auth suite registers and logs in many users (e.g., multiple uses of registered_user across the P0 auth tests), so the 3 registrations/10 minutes and 10 logins/minute decorators will start returning 429s long before the suite finishes when running against the compose setup. To keep the compose workflow usable for the advertised “lightweight” E2E run, the compose service needs RATE_LIMIT_ENABLED=false (or an equivalent override) the same way the Helm values do.
Useful? React with 👍 / 👎.
Add comprehensive E2E tests for model registry workflows: - Cloud model onboarding workflow (multi-step) - Local model onboarding workflow (async with polling) - Model edit operations (name, description, tags, URLs) - Model delete operations (soft delete) - Model listing and search - Provider listing and filtering Test infrastructure: - ModelHelper class with workflow management - Model fixtures and data generators - Proper handling of permission errors (MODEL_VIEW, MODEL_MANAGE) - Correct modality values (text_input/text_output) - Added 'models' marker for test filtering Test results: 89 passed, 35 skipped - Skipped tests due to permission requirements (expected for regular users) - Tests will pass with users having MODEL_MANAGE/MODEL_VIEW permissions 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Summary
RATE_LIMIT_ENABLEDconfigTest Coverage
Files Added/Modified
tests/e2e/- Complete E2E test infrastructureservices/budapp/budapp/commons/config.py- Addedrate_limit_enabledsettingservices/budapp/budapp/commons/rate_limiter.py- Support disabling rate limits for testsTest Plan
.gitignorefiles to exclude test artifacts🤖 Generated with Claude Code