codeant-ai-for-open-source[bot] commented on code in PR #39011:
URL: https://github.com/apache/superset/pull/39011#discussion_r3023180762
##########
tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py:
##########
@@ -444,7 +444,34 @@ async def test_generate_dashboard_minimal_request(
== "Minimal Dashboard"
)
- # Verify dashboard was created with default published=True
+ # Verify dashboard was created with default published=False
+ created = mock_dashboard_cls.return_value
+ assert created.published is False
+
+ @patch("superset.models.dashboard.Dashboard")
+ @patch("superset.daos.dashboard.DashboardDAO.find_by_id")
+ @patch("superset.db.session")
+ @pytest.mark.asyncio
+ async def test_generate_dashboard_explicit_published(
+ self, mock_db_session, mock_find_by_id, mock_dashboard_cls, mcp_server
+ ):
+ """Test dashboard generation with published explicitly set to True."""
+ charts = [_mock_chart(id=3)]
+ mock_dashboard = _mock_dashboard(id=41, title="Published Dashboard")
+ _setup_generate_dashboard_mocks(
Review Comment:
**Suggestion:** This test can produce a false positive because the mocked
dashboard starts with `published=True`, so the final assertion may pass even if
the tool never applies the explicit publish request. Initialize the mock
dashboard as unpublished first so the test actually verifies that
`published=True` is set by the code under test. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Regression in publish-flag handling may go undetected in tests.
- ⚠️ MCP dashboard publish semantics lack robust automated verification.
```
</details>
```suggestion
mock_dashboard.published = False
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open
`tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py` and
locate `_mock_dashboard` at lines 102–122, where the helper sets
`dashboard.published =
True` by default (line 109).
2. In the same file, locate `test_generate_dashboard_explicit_published` at
lines 455–476.
At line 460 it builds `mock_dashboard = _mock_dashboard(id=41,
title="Published
Dashboard")`, so `mock_dashboard.published` is already `True` before the
tool runs.
3. `_setup_generate_dashboard_mocks` at lines 125–157 wires this mock into
the tool by
assigning `mock_dashboard_cls.return_value = dashboard` (line 155), and
`mock_dashboard_cls` is the patched `superset.models.dashboard.Dashboard`
that
`generate_dashboard` imports at line 265 of
`superset/mcp_service/dashboard/tool/generate_dashboard.py`.
4. The test invokes the tool via
`Client(mcp_server).call_tool("generate_dashboard",
{"request": request})` (lines 471–472) and finally asserts `created =
mock_dashboard_cls.return_value` and `assert created.published is True`
(lines 475–476).
Because `created` is the same `_mock_dashboard` instance that started with
`published=True`, this assertion will still succeed even in a scenario where
`generate_dashboard` at
`superset/mcp_service/dashboard/tool/generate_dashboard.py:290–296` stops
applying
`dashboard.published = request.published` (line 295). Thus the test can pass
without
actually verifying that the explicit `published=True` request is honored,
confirming the
assertion is ineffective unless the mock is initialized with
`published=False` first.
```
</details>
<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:** 461:461
**Comment:**
*Logic Error: This test can produce a false positive because the mocked
dashboard starts with `published=True`, so the final assertion may pass even if
the tool never applies the explicit publish request. Initialize the mock
dashboard as unpublished first so the test actually verifies that
`published=True` is set by the code under test.
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.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39011&comment_hash=5d2777022073475a810b7cd1dd5976f6827f6faca369d3aeeca0b000685cdfc7&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39011&comment_hash=5d2777022073475a810b7cd1dd5976f6827f6faca369d3aeeca0b000685cdfc7&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]
