bito-code-review[bot] commented on code in PR #38410:
URL: https://github.com/apache/superset/pull/38410#discussion_r2891138994
##########
tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py:
##########
@@ -766,3 +828,55 @@ def test_ensure_layout_structure_adds_to_tab(self):
assert "ROW-new" in layout["TAB-first"]["children"]
assert "ROW-new" not in layout["GRID_ID"]["children"]
+
+
+class TestGenerateTitleFromCharts:
+ """Tests for _generate_title_from_charts helper."""
+
+ def test_empty_list_returns_dashboard(self):
+ assert _generate_title_from_charts([]) == "Dashboard"
+
+ def test_single_chart(self):
+ charts = [_mock_chart(id=1, slice_name="Revenue")]
+ assert _generate_title_from_charts(charts) == "Revenue"
+
+ def test_two_charts_joined_with_ampersand(self):
+ charts = [
+ _mock_chart(id=1, slice_name="Revenue"),
+ _mock_chart(id=2, slice_name="Costs"),
+ ]
+ assert _generate_title_from_charts(charts) == "Revenue & Costs"
+
+ def test_three_charts_joined_with_commas(self):
+ charts = [
+ _mock_chart(id=1, slice_name="Revenue"),
+ _mock_chart(id=2, slice_name="Costs"),
+ _mock_chart(id=3, slice_name="Profit"),
+ ]
+ assert _generate_title_from_charts(charts) == "Revenue, Costs, Profit"
+
+ def test_four_charts_shows_plus_more(self):
+ charts = [_mock_chart(id=i, slice_name=f"Chart {i}") for i in range(1,
5)]
+ assert (
+ _generate_title_from_charts(charts) == "Chart 1, Chart 2, Chart 3
+ 1 more"
+ )
+
+ def test_many_charts_shows_plus_more(self):
+ charts = [_mock_chart(id=i, slice_name=f"Chart {i}") for i in range(1,
8)]
+ assert (
+ _generate_title_from_charts(charts) == "Chart 1, Chart 2, Chart 3
+ 4 more"
+ )
+
+ def test_charts_without_names_returns_dashboard(self):
+ chart = Mock()
+ chart.slice_name = None
+ assert _generate_title_from_charts([chart]) == "Dashboard"
+
+ def test_long_title_is_truncated(self):
+ charts = [
+ _mock_chart(id=1, slice_name="A" * 100),
+ _mock_chart(id=2, slice_name="B" * 100),
+ ]
+ title = _generate_title_from_charts(charts)
+ assert len(title) <= 150
+ assert title.endswith("\u2026")
Review Comment:
<!-- Bito Reply -->
No, the suggestion isn’t fully correct — the original assertion already
checks for the ellipsis character, as \u2026 in Python string literals
represents U+2026. The implementation appends the ellipsis correctly, so no
change is needed.
--
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]
