codeant-ai-for-open-source[bot] commented on code in PR #39914:
URL: https://github.com/apache/superset/pull/39914#discussion_r3508335127
##########
tests/unit_tests/commands/report/execute_test.py:
##########
@@ -1309,73 +1309,109 @@ def
test_update_recipient_to_slack_v2_missing_channels(mocker: MockerFixture):
mock_cmmd.update_report_schedule_slack_v2()
-def test_update_recipient_to_slack_v2_reverts_all_on_partial_failure(
+def test_update_recipient_to_slack_v2_multiple_recipients(
mocker: MockerFixture,
) -> None:
- """
- When the second of two Slack recipients fails channel resolution, BOTH
- recipients are fully reverted β type AND exact original
- ``recipient_config_json`` string β not just the loop variable's type. This
- prevents the intervening ``create_log`` commit from flushing a
half-migrated,
- inconsistent state.
+ """All Slack recipients are upgraded atomically when every channel
resolves."""
+
+ def fake_get_channels(search_string, types, exact_match):
Review Comment:
β
**Customized review instruction saved!**
**Instruction:**
> Do not flag missing type hints for helper functions in test files, since
typing is intentionally relaxed for tests via the projectβs mypy overrides.
**Applied to:**
- `**/test/**`
- `**/tests/**`
- `**/*test*.py`
---
π‘ *To manage or update this instruction, visit: [CodeAnt AI
Settings](https://app.codeant.ai/org/settings/learnings)*
##########
tests/unit_tests/reports/notifications/slack_tests.py:
##########
@@ -16,16 +16,50 @@
# under the License.
import uuid
-from unittest.mock import MagicMock, patch
+from typing import Any
+from unittest.mock import call, MagicMock, patch
import pandas as pd
import pytest
-from slack_sdk.errors import SlackApiError
+from slack_sdk.errors import (
+ BotUserAccessError,
+ SlackApiError,
+ SlackClientConfigurationError,
+ SlackClientError,
+ SlackClientNotConnectedError,
+ SlackObjectFormationError,
+ SlackRequestError,
+ SlackTokenRotationError,
+)
+from slack_sdk.web.slack_response import SlackResponse
-from superset.reports.notifications.slackv2 import SlackV2Notification
+from superset.reports.notifications.exceptions import (
+ NotificationAuthorizationException,
+ NotificationMalformedException,
+ NotificationParamException,
+ NotificationUnprocessableException,
+)
+from superset.reports.notifications.slackv2 import (
+ _give_up_slack_api_retry,
+ SlackV2Notification,
+)
from superset.utils.core import HeaderDataType
[email protected](autouse=True)
+def _skip_backoff_sleep():
Review Comment:
β
**Customized review instruction saved!**
**Instruction:**
> Avoid flagging missing return type annotations in test modules covered by
the `tests.*` mypy override, since untyped defs are allowed there.
**Applied to:**
- `**/test/**`
- `**/tests/**`
- `**/*test*.py`
---
π‘ *To manage or update this instruction, visit: [CodeAnt AI
Settings](https://app.codeant.ai/org/settings/learnings)*
##########
tests/unit_tests/reports/notifications/slack_tests.py:
##########
@@ -431,3 +470,632 @@ def test_slack_mixin_get_body_truncates_large_table(
)
body = notification._get_body(content=content)
assert "(table was truncated)" in body
+
+
+# ---------------------------------------------------------------------------
+# Bulletproof v2 send-path coverage
+#
+# The tests above exercise the chat_postMessage path (text-only sends). The
+# tests below cover files_upload_v2 across screenshots/CSV/PDF, multi-channel
+# fan-out, exception mapping, backoff, statsd, and logs propagation. Together
+# they guarantee that every observable behavior of SlackV2Notification.send()
+# is locked down before Slack v1 is removed.
+# ---------------------------------------------------------------------------
+
+
+def _make_v2_notification(content, target: str = "C12345"):
Review Comment:
β
**Customized review instruction saved!**
**Instruction:**
> Do not flag missing type annotations in test helpers when the repository's
tests configuration explicitly relaxes untyped-def requirements; test files may
omit annotations if linting/type-checking already passes.
**Applied to:**
- `**/test/**`
- `**/tests/**`
- `**/*test*.<ext>`
---
π‘ *To manage or update this instruction, visit: [CodeAnt AI
Settings](https://app.codeant.ai/org/settings/learnings)*
##########
tests/unit_tests/reports/notifications/slack_tests.py:
##########
@@ -431,3 +470,632 @@ def test_slack_mixin_get_body_truncates_large_table(
)
body = notification._get_body(content=content)
assert "(table was truncated)" in body
+
+
+# ---------------------------------------------------------------------------
+# Bulletproof v2 send-path coverage
+#
+# The tests above exercise the chat_postMessage path (text-only sends). The
+# tests below cover files_upload_v2 across screenshots/CSV/PDF, multi-channel
+# fan-out, exception mapping, backoff, statsd, and logs propagation. Together
+# they guarantee that every observable behavior of SlackV2Notification.send()
+# is locked down before Slack v1 is removed.
+# ---------------------------------------------------------------------------
+
+
+def _make_v2_notification(content, target: str = "C12345"):
+ """Helper to build a SlackV2Notification with a given target string."""
+ from superset.reports.models import ReportRecipients, ReportRecipientType
+
+ return SlackV2Notification(
+ recipient=ReportRecipients(
+ type=ReportRecipientType.SLACKV2,
+ recipient_config_json=f'{{"target": "{target}"}}',
+ ),
+ content=content,
+ )
+
+
+def _make_content(mock_header_data, **overrides):
Review Comment:
β
**Customized review instruction saved!**
**Instruction:**
> Do not flag missing type hints in test helpers; the `tests.*` mypy
override already relaxes `disallow_untyped_defs` and these files are covered by
existing lint/type-checking settings.
**Applied to:**
- `**/test/**`
- `**/tests/**`
- `**/*test*.py`
---
π‘ *To manage or update this instruction, visit: [CodeAnt AI
Settings](https://app.codeant.ai/org/settings/learnings)*
--
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]
