bito-code-review[bot] commented on code in PR #38591:
URL: https://github.com/apache/superset/pull/38591#discussion_r3012873190
##########
tests/unit_tests/reports/schemas_test.py:
##########
@@ -75,3 +75,184 @@ def test_report_post_schema_custom_width_validation(mocker:
MockerFixture) -> No
assert excinfo.value.messages == {
"custom_width": ["Screenshot width must be between 100px and 200px"]
}
+
+
+MINIMAL_POST_PAYLOAD = {
+ "type": "Report",
+ "name": "A report",
+ "crontab": "* * * * *",
+ "timezone": "America/Los_Angeles",
+}
+
+CUSTOM_WIDTH_CONFIG = {
+ "ALERT_REPORTS_MIN_CUSTOM_SCREENSHOT_WIDTH": 600,
+ "ALERT_REPORTS_MAX_CUSTOM_SCREENSHOT_WIDTH": 2400,
+}
+
+
[email protected](
+ "schema_class,payload_base",
+ [
+ (ReportSchedulePostSchema, MINIMAL_POST_PAYLOAD),
+ (ReportSchedulePutSchema, {}),
+ ],
+ ids=["post", "put"],
+)
[email protected](
+ "width,should_pass",
+ [
+ (599, False),
+ (600, True),
+ (2400, True),
+ (2401, False),
+ (None, True),
+ ],
+)
+def test_custom_width_boundary_values(
+ mocker: MockerFixture,
+ schema_class: type,
+ payload_base: dict[str, object],
+ width: int | None,
+ should_pass: bool,
+) -> None:
+ mocker.patch("flask.current_app.config", CUSTOM_WIDTH_CONFIG)
+ schema = schema_class()
+ payload = {**payload_base, "custom_width": width}
+
+ if should_pass:
+ schema.load(payload)
+ else:
+ with pytest.raises(ValidationError) as exc:
+ schema.load(payload)
+ assert "custom_width" in exc.value.messages
+
+
+def test_working_timeout_validation(mocker: MockerFixture) -> None:
+ mocker.patch("flask.current_app.config", CUSTOM_WIDTH_CONFIG)
+ post_schema = ReportSchedulePostSchema()
+ put_schema = ReportSchedulePutSchema()
+
+ # POST: working_timeout=0 and -1 are invalid (min=1)
+ with pytest.raises(ValidationError) as exc:
+ post_schema.load({**MINIMAL_POST_PAYLOAD, "working_timeout": 0})
+ assert "working_timeout" in exc.value.messages
+
+ with pytest.raises(ValidationError) as exc:
+ post_schema.load({**MINIMAL_POST_PAYLOAD, "working_timeout": -1})
+ assert "working_timeout" in exc.value.messages
+
+ # POST: working_timeout=1 is valid
+ post_schema.load({**MINIMAL_POST_PAYLOAD, "working_timeout": 1})
+
+ # PUT: working_timeout=None is valid (allow_none=True)
+ put_schema.load({"working_timeout": None})
+
+
+def test_log_retention_post_vs_put_parity(mocker: MockerFixture) -> None:
+ mocker.patch("flask.current_app.config", CUSTOM_WIDTH_CONFIG)
+ post_schema = ReportSchedulePostSchema()
+ put_schema = ReportSchedulePutSchema()
+
+ # POST: log_retention=0 is invalid (min=1)
+ with pytest.raises(ValidationError) as exc:
+ post_schema.load({**MINIMAL_POST_PAYLOAD, "log_retention": 0})
+ assert "log_retention" in exc.value.messages
+
+ # POST: log_retention=1 is valid
+ post_schema.load({**MINIMAL_POST_PAYLOAD, "log_retention": 1})
+
+ # PUT: log_retention=0 is valid (min=0)
+ put_schema.load({"log_retention": 0})
+
+
+def test_report_type_disallows_database(mocker: MockerFixture) -> None:
+ mocker.patch("flask.current_app.config", CUSTOM_WIDTH_CONFIG)
+ schema = ReportSchedulePostSchema()
+
+ with pytest.raises(ValidationError) as exc:
+ schema.load({**MINIMAL_POST_PAYLOAD, "database": 1})
+ assert "database" in exc.value.messages
+
+
+def test_alert_type_allows_database(mocker: MockerFixture) -> None:
+ """Alert type should accept database; only Report type blocks it."""
+ mocker.patch("flask.current_app.config", CUSTOM_WIDTH_CONFIG)
+ schema = ReportSchedulePostSchema()
+ result = schema.load({**MINIMAL_POST_PAYLOAD, "type": "Alert", "database":
1})
+ assert result["database"] == 1
+
+
+# ---------------------------------------------------------------------------
+# Phase 1b gap closure: crontab validator, name length, PUT parity
+# ---------------------------------------------------------------------------
+
+
[email protected](
+ "crontab,should_pass",
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Parametrize names argument type error</b></div>
<div id="fix">
Change the first argument of `pytest.mark.parametrize` from a string to a
tuple: `("crontab", "should_pass")` instead of `"crontab,should_pass"`.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
("crontab", "should_pass"),
````
</div>
</details>
</div>
<small><i>Code Review Run #1634e6</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
