codeant-ai-for-open-source[bot] commented on code in PR #37026:
URL: https://github.com/apache/superset/pull/37026#discussion_r2678026643
##########
tests/unit_tests/db_engine_specs/test_starrocks.py:
##########
@@ -173,3 +190,94 @@ def test_impersonation_disabled(mocker: MockerFixture) ->
None:
) == (make_url("starrocks://service_user@localhost:9030/hive.default"), {})
assert StarRocksEngineSpec.get_prequeries(database) == []
+
+
+def test_get_default_catalog(mocker: MockerFixture) -> None:
+ """
+ Test the ``get_default_catalog`` method.
+ """
+ from superset.db_engine_specs.starrocks import StarRocksEngineSpec
+
+ # Test case 1: Catalog is in the URI
+ database = mocker.MagicMock()
+ database.url_object.database = "hive.default"
+
+ assert StarRocksEngineSpec.get_default_catalog(database) == "hive"
+
+ # Test case 2: Catalog is not in the URI, returns default
+ database = mocker.MagicMock()
+ database.url_object.database = "default"
+
+ assert StarRocksEngineSpec.get_default_catalog(database) ==
"default_catalog"
+
+
+def test_get_catalog_names(mocker: MockerFixture) -> None:
+ """
+ Test the ``get_catalog_names`` method.
+ """
+ from superset.db_engine_specs.starrocks import StarRocksEngineSpec
+
+ database = mocker.MagicMock()
+ inspector = mocker.MagicMock()
+
+ # Mock the actual StarRocks SHOW CATALOGS format
+ # StarRocks returns rows with keys: ['Catalog', 'Type', 'Comment']
+ mock_row_1 = mocker.MagicMock()
+ mock_row_1.keys.return_value = ["Catalog", "Type", "Comment"]
+ mock_row_1.__getitem__ = lambda self, key: "default_catalog" if key ==
"Catalog" else None
+
+ mock_row_2 = mocker.MagicMock()
+ mock_row_2.keys.return_value = ["Catalog", "Type", "Comment"]
+ mock_row_2.__getitem__ = lambda self, key: "hive" if key == "Catalog" else
None
+
+ mock_row_3 = mocker.MagicMock()
+ mock_row_3.keys.return_value = ["Catalog", "Type", "Comment"]
+ mock_row_3.__getitem__ = lambda self, key: "iceberg" if key == "Catalog"
else None
Review Comment:
**Suggestion:** The mocks for catalog rows override `__getitem__` on
`MagicMock` instances, but special methods are looked up on the class, so
`row["Catalog"]` still returns MagicMock objects instead of the expected
strings, causing `get_catalog_names` to return a set of mocks instead of
`{"default_catalog", "hive", "iceberg"}` and making the test assertions fail.
[logic error]
**Severity Level:** Minor ⚠️
```suggestion
mock_row_1 = {"Catalog": "default_catalog", "Type": None, "Comment":
None}
mock_row_2 = {"Catalog": "hive", "Type": None, "Comment": None}
mock_row_3 = {"Catalog": "iceberg", "Type": None, "Comment": None}
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The claim is correct: assigning __getitem__ on MagicMock instances does not
affect Python's special-method lookup which resolves special methods on the
object's class. As written the test may yield MagicMock values when code does
row["Catalog"] (or similar), causing the assertion to fail or be flaky.
Replacing the MagicMock rows with plain dicts (or configuring the mock's
class-level __getitem__) produces real strings and makes the test
deterministic. This is a real logic fix for the unit test rather than a
cosmetic change.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/db_engine_specs/test_starrocks.py
**Line:** 225:235
**Comment:**
*Logic Error: The mocks for catalog rows override `__getitem__` on
`MagicMock` instances, but special methods are looked up on the class, so
`row["Catalog"]` still returns MagicMock objects instead of the expected
strings, causing `get_catalog_names` to return a set of mocks instead of
`{"default_catalog", "hive", "iceberg"}` and making the test assertions fail.
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>
--
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]
