codeant-ai-for-open-source[bot] commented on code in PR #42803:
URL: https://github.com/apache/superset/pull/42803#discussion_r3749245951
##########
tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py:
##########
@@ -40,6 +40,7 @@
from superset.mcp_service.dashboard.tool.generate_dashboard import (
_generate_title_from_charts,
)
+from superset.models.dashboard import Dashboard as _RealDashboard
Review Comment:
**Suggestion:** The module-level import eagerly loads the Dashboard/Slice
model graph during pytest collection, before the unit-test app fixture runs
`encrypted_field_factory.init_app()`. This can raise “App not initialized yet.
Please call init_app first” and prevent the entire test module from being
collected. Import the real Dashboard lazily inside the helper after the app
context has been established. [import error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Dashboard-generation test module can fail during pytest collection.
- ⚠️ CI cannot execute this unit-test module.
- ⚠️ Failures occur before any test or fixture setup runs.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=78033cf8278c4c3c99dde4fda96325d7&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=78033cf8278c4c3c99dde4fda96325d7&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py
**Line:** 43:43
**Comment:**
*Import Error: The module-level import eagerly loads the
Dashboard/Slice model graph during pytest collection, before the unit-test app
fixture runs `encrypted_field_factory.init_app()`. This can raise “App not
initialized yet. Please call init_app first” and prevent the entire test module
from being collected. Import the real Dashboard lazily inside the helper after
the app context has been established.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42803&comment_hash=9a32cbc1dff8ec3465cc56e1e4b8748015ccfdd43ec51f46fb9521187d50a00d&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42803&comment_hash=9a32cbc1dff8ec3465cc56e1e4b8748015ccfdd43ec51f46fb9521187d50a00d&reaction=dislike'>👎</a>
##########
tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py:
##########
@@ -167,6 +168,24 @@ def _setup_generate_dashboard_mocks(
mock_dashboard_cls.return_value = dashboard
mock_find_by_id.return_value = dashboard
+ # `generate_dashboard` builds its re-fetch eager-load options with
+ # `subqueryload(Dashboard.slices).subqueryload(Slice.editors)` etc.
+ # against this same patched `Dashboard` class. SQLAlchemy 2.0 validates
+ # loader-path arguments eagerly and raises `ArgumentError` ("Wildcard
+ # token cannot be followed by another entity") when given a plain
+ # MagicMock attribute instead of a real `InstrumentedAttribute` --
+ # SQLAlchemy 1.4 didn't validate this eagerly, so the same mock chain
+ # silently worked before. Copy over the real class-level relationship
+ # attributes (captured at module import time, before `Dashboard` gets
+ # patched, since `from ... import Dashboard` done here would just
+ # return the mock itself) so `subqueryload`/`joinedload` construction
+ # sees genuine mapped attributes while `Dashboard(...)` instantiation
+ # (used to create new dashboards) still returns the mocked `dashboard`
+ # object.
+ mock_dashboard_cls.slices = _RealDashboard.slices
+ mock_dashboard_cls.editors = _RealDashboard.editors
+ mock_dashboard_cls.tags = _RealDashboard.tags
+
# Prevent Subject DB queries during dashboard creation.
Review Comment:
**Suggestion:** Starting a patcher without retaining or stopping it leaves
`get_user_subject` mocked after each helper call. `patch.stopall()` is never
called in this file, and `unittest.mock.patch` does not clean up automatically
at process exit, so later tests can observe the leaked mock and the stacked
patchers can contaminate unrelated subject/editor behavior. Use a
context-managed patch or register cleanup for every invocation. [missing
cleanup]
<details>
<summary><b>Severity Level:</b> Minor 🧹</summary>
```mdx
- ⚠️ Subject lookup remains mocked after dashboard tests.
- ⚠️ Later tests may receive incorrect editor or subject associations.
- ⚠️ Test results can become order-dependent across modules.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=12cf48c702a74e02b9565077226f13b6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=12cf48c702a74e02b9565077226f13b6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py
**Line:** 189:192
**Comment:**
*Missing Cleanup: Starting a patcher without retaining or stopping it
leaves `get_user_subject` mocked after each helper call. `patch.stopall()` is
never called in this file, and `unittest.mock.patch` does not clean up
automatically at process exit, so later tests can observe the leaked mock and
the stacked patchers can contaminate unrelated subject/editor behavior. Use a
context-managed patch or register cleanup for every invocation.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42803&comment_hash=9912f0d99c2d9821da3e644266a10f9cef9c3864591e417b7d895d6cf4c6a239&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42803&comment_hash=9912f0d99c2d9821da3e644266a10f9cef9c3864591e417b7d895d6cf4c6a239&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
