codeant-ai-for-open-source[bot] commented on code in PR #41080:
URL: https://github.com/apache/superset/pull/41080#discussion_r3417080960
##########
tests/unit_tests/utils/webdriver_test.py:
##########
@@ -898,3 +898,76 @@ def evaluate_side_effect(script):
"dashboard",
"http://example.com";,
)
+
+ @patch("superset.utils.webdriver.PLAYWRIGHT_AVAILABLE", True)
+ @patch("superset.utils.webdriver.sync_playwright")
+ @patch("superset.utils.webdriver.logger")
+ @patch("superset.utils.webdriver.take_tiled_screenshot")
+ def test_tiled_screenshot_failure_returns_none_without_fallback(
+ self, mock_take_tiled, mock_logger, mock_sync_playwright
+ ):
Review Comment:
**Suggestion:** Add full type annotations to this new test method, including
parameter types and an explicit `-> None` return type. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This is a newly added Python test method and it has no parameter type hints
or explicit return type annotation. The custom rule requires newly added Python
functions and methods to be fully typed, so this is a real violation.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=7cb5528f79424b46ad7397470cec1f7f&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=7cb5528f79424b46ad7397470cec1f7f&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/utils/webdriver_test.py
**Line:** 906:908
**Comment:**
*Custom Rule: Add full type annotations to this new test method,
including parameter types and an explicit `-> None` return type.
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%2F41080&comment_hash=ace16a4d0768afe70cb11ded992d03ea6972b9761b37c5e611ff262d4f161974&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41080&comment_hash=ace16a4d0768afe70cb11ded992d03ea6972b9761b37c5e611ff262d4f161974&reaction=dislike'>👎</a>
##########
superset/utils/webdriver.py:
##########
@@ -602,29 +556,25 @@ def _create(self) -> WebDriver:
logger.debug("Init selenium driver")
return driver_class(**kwargs)
- def _auth(self, user: User) -> None:
- """Authenticate the persistent driver in-place."""
- if self._driver is None:
- raise RuntimeError("WebDriver is not initialized")
- machine_auth_provider_factory.instance.authenticate_webdriver(
- self._driver, user
+ def auth(self, user: User) -> WebDriver:
+ driver = self.create()
+ return machine_auth_provider_factory.instance.authenticate_webdriver(
+ driver, user
Review Comment:
**Suggestion:** Add a method docstring describing the authentication
behavior and returned driver so the newly introduced function is documented
inline. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The new `auth` method was added without a docstring, and the custom rule
explicitly requires newly added Python functions and classes to be documented
inline. This is a real violation in the final file state.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=72e8647b631e4de9b1f9d4a3b99310c1&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=72e8647b631e4de9b1f9d4a3b99310c1&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/utils/webdriver.py
**Line:** 559:562
**Comment:**
*Custom Rule: Add a method docstring describing the authentication
behavior and returned driver so the newly introduced function is documented
inline.
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%2F41080&comment_hash=1f6080c4c3f579e187225d2ba374ba5f35d8b41d1f56205eaa81455075b7bb7c&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41080&comment_hash=1f6080c4c3f579e187225d2ba374ba5f35d8b41d1f56205eaa81455075b7bb7c&reaction=dislike'>👎</a>
##########
tests/unit_tests/utils/webdriver_test.py:
##########
@@ -898,3 +898,76 @@ def evaluate_side_effect(script):
"dashboard",
"http://example.com";,
)
+
+ @patch("superset.utils.webdriver.PLAYWRIGHT_AVAILABLE", True)
+ @patch("superset.utils.webdriver.sync_playwright")
+ @patch("superset.utils.webdriver.logger")
+ @patch("superset.utils.webdriver.take_tiled_screenshot")
+ def test_tiled_screenshot_failure_returns_none_without_fallback(
+ self, mock_take_tiled, mock_logger, mock_sync_playwright
+ ):
+ """When take_tiled_screenshot fails, return None rather than fall back
to a
+ potentially blank standard screenshot."""
+ mock_user = MagicMock()
+ mock_user.username = "test_user"
+
+ mock_playwright_instance = MagicMock()
+ mock_browser = MagicMock()
+ mock_context = MagicMock()
+ mock_page = MagicMock()
+ mock_element = MagicMock()
+
+ mock_sync_playwright.return_value.__enter__.return_value = (
+ mock_playwright_instance
+ )
+ mock_playwright_instance.chromium.launch.return_value = mock_browser
+ mock_browser.new_context.return_value = mock_context
+ mock_context.new_page.return_value = mock_page
+ mock_page.locator.return_value = mock_element
+ mock_element.wait_for.return_value = None
+ mock_element.screenshot.return_value = b"should_not_be_called"
+
+ def evaluate_side_effect(script):
Review Comment:
**Suggestion:** Add type hints to this newly added helper function,
including the `script` parameter type and an explicit return type. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This newly added helper function omits both the `script` parameter type and
a return type annotation. That matches the custom rule requiring new Python
functions to be fully typed.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=3cbfb40a2e8442f493b1861d0cc63111&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=3cbfb40a2e8442f493b1861d0cc63111&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/utils/webdriver_test.py
**Line:** 930:930
**Comment:**
*Custom Rule: Add type hints to this newly added helper function,
including the `script` parameter type and an explicit return type.
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%2F41080&comment_hash=88c6edc0f81ccf880cdfd547df7e6f83063cff8c14d8c725239a2be178aa7d7e&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41080&comment_hash=88c6edc0f81ccf880cdfd547df7e6f83063cff8c14d8c725239a2be178aa7d7e&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]
