Copilot commented on code in PR #41966:
URL: https://github.com/apache/superset/pull/41966#discussion_r3576350236
##########
tests/integration_tests/reports/commands_tests.py:
##########
@@ -159,20 +159,26 @@ def assert_log(state: str, error_message: Optional[str] =
None):
db.session.commit()
logs = db.session.query(ReportExecutionLog).all()
- if state == ReportState.ERROR:
- # On error we send an email
- assert len(logs) == 3
- else:
+ if state == ReportState.WORKING:
+ # A report that is already in the WORKING state logs an extra WORKING
row
+ # for the refused re-computation, on top of the row seeded by the
fixture.
+ assert len(logs) == 2
+ elif state == ReportState.ERROR:
+ # On error we also send a notification, which is recorded as a separate
+ # error-notification marker row.
assert len(logs) == 2
+ else:
+ # A single execution yields a single log row: the terminal result
replaces
+ # the WORKING "trigger" row rather than adding a second row (issue
#29857).
+ assert len(logs) == 1
log_states = [log.state for log in logs]
- assert ReportState.WORKING in log_states
assert state in log_states
- assert error_message in [log.error_message for log in logs]
-
- for log in logs:
- if log.state == ReportState.WORKING:
- assert log.value is None
- assert log.value_row_json is None
+ # Previously a standalone WORKING "trigger" row always contributed a
``None``
+ # error_message, so the default ``None`` match was trivially satisfied and
never
+ # verified the terminal row. With the result now written onto that single
row,
+ # only assert an explicitly expected message.
+ if error_message is not None:
+ assert error_message in [log.error_message for log in logs]
Review Comment:
`assert_log` no longer verifies the invariant that WORKING log rows should
not carry `value`/`value_row_json` (those are cleared when transitioning to
WORKING). Dropping this check makes the integration tests less likely to catch
regressions in the WORKING placeholder row behavior.
##########
tests/integration_tests/reports/commands_tests.py:
##########
@@ -772,6 +778,45 @@ def test_email_chart_report_schedule(
assert_log(ReportState.SUCCESS)
[email protected](
+ "load_birth_names_dashboard_with_slices", "create_report_email_chart"
+)
+@patch("superset.reports.notifications.email.send_email_smtp")
+@patch("superset.utils.screenshots.ChartScreenshot.get_screenshot")
+def test_email_chart_report_schedule_single_log_per_execution(
+ screenshot_mock,
+ email_mock,
+ create_report_email_chart,
+):
+ """
+ ExecuteReport Command: a single execution should produce a single log row.
+
+ Regression for #29857: the Alerts & Reports execution log shows duplicated
+ entries for a single execution. Each execution transitions through the
+ WORKING state and then a terminal state (SUCCESS/ERROR), and every
+ transition writes a ReportExecutionLog row sharing the same execution
+ ``uuid``. As a result one execution surfaces as two rows in the log view
+ (the "trigger" row and the "result" row). This test asserts that one
+ execution -- identified by its execution uuid -- yields exactly one log
row.
+ """
+ screenshot_mock.return_value = SCREENSHOT_FILE
+
+ with freeze_time("2020-01-01T00:00:00Z"):
+ AsyncExecuteReportScheduleCommand(
+ TEST_ID, create_report_email_chart.id, datetime.utcnow()
+ ).run()
+
+ db.session.commit()
+ logs = (
+ db.session.query(ReportExecutionLog)
+ .filter(ReportExecutionLog.uuid == TEST_ID)
+ .all()
+ )
Review Comment:
This test filters `ReportExecutionLog.uuid` (a `UUIDType` column) using
`TEST_ID`, which is defined as a string. Coercion often works, but using a
`UUID` object here matches production (`AsyncExecuteReportScheduleCommand`
casts `task_id` to `UUID`) and avoids dialect/type-decorator edge cases.
--
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]
