fat-catTW commented on code in PR #74127: URL: https://github.com/apache/airflow/pull/74127#discussion_r4176229791
########## airflow-core/tests/unit/api_fastapi/core_api/routes/ui/test_deadlines.py: ########## Review Comment: Thanks for the PR, the API change itself looks good to me. One test concern: `test_alert_response_fields` now depends on `deadline_alerts[0]`, but this PR adds a second `DeadlineAlert` to the shared fixture. Since the endpoint default ordering is only by `created_at`, the first returned alert is not a stable contract if the two rows tie or the DB orders ties differently. Could we avoid relying on index `0` here? For example: ```python data = response.json() alert = next(alert for alert in data["deadline_alerts"] if alert["name"] == ALERT_NAME) assert alert["name"] == ALERT_NAME assert alert["interval"] == 3600.0 assert alert["reference_type"] == "DagRunQueuedAtDeadline" assert "id" in alert assert "created_at" in alert -- 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]
