You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "Kaap10 (via GitHub)" <gi...@apache.org> on 2026/09/30 06:20:08 UTC
[PR] API: Clarify docstrings and assert detail in Task State Store tests [airflow]
Kaap10 opened a new pull request, #73939:
URL: https://github.com/apache/airflow/pull/73939
<!-- SPDX-License-Identifier: Apache-2.0
https://www.apache.org/licenses/LICENSE-2.0 -->
Follow-up to #73753 based on reviewer feedback from @henry3260:
1. **Docstring Accuracy in `test_task_state_store.py`**:
- `test_set_nonexistent_dag_run_returns_404`: Clarified that the 404 response originates from scope validation (`_validate_scope`), not `set()`.
- `test_set_nonexistent_task_id_returns_404`: Fixed docstring typo (`doesn not` -> `does not`) and clarified scope validation origin.
2. **Detail Assertions**:
- Added explicit assertions on `response.json()["detail"]` for both scope validation tests.
3. **Domain Exception Coverage for PUT**:
- Added `test_set_task_state_store_domain_error_returns_404` using `autospec=True` on `MetastoreBackend.set` to verify exception translation parity with `PATCH`.
related: #73753
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes - Antigravity
Generated-by: Antigravity following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
---
* Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
* For fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
* When adding dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
* For significant user-facing changes create newsfragment: `{pr_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] API: Clarify docstrings and assert detail in Task State Store tests [airflow]
Posted by "Kaap10 (via GitHub)" <gi...@apache.org>.
Kaap10 commented on code in PR #73939:
URL: https://github.com/apache/airflow/pull/73939#discussion_r4142746876
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_task_state_store.py:
##########
@@ -211,16 +211,29 @@ def test_non_finite_float_rejected_by_validator(self, bad_value):
TaskStateStoreBody(value=bad_value)
def test_set_nonexistent_dag_run_returns_404(self, test_client):
- """set() raises ValueError when DagRun doesn't exist — should surface as 404."""
+ """Scope validation rejects non-existent DagRun with 404."""
bad_url = f"/dags/{DAG_ID}/dagRuns/nonexistent_run/taskInstances/{TASK_ID}/state-store/job_id"
response = test_client.put(bad_url, json={"value": "v"})
assert response.status_code == 404
+ assert "Task instance not found" in response.json()["detail"]
def test_set_nonexistent_task_id_returns_404(self, test_client):
- """set() returns 404 when task_id doesn not match any TaskInstance in the run."""
+ """Scope validation returns 404 when task_id does not match any TaskInstance in the run."""
bad_url = f"/dags/{DAG_ID}/dagRuns/{RUN_ID}/taskInstances/nonexistent_task/state-store/job_id"
response = test_client.put(bad_url, json={"value": "v"})
assert response.status_code == 404
+ assert "Task instance not found" in response.json()["detail"]
+
Review Comment:
Good catch! Removed both redundant tests as they are already covered under `TestUnknownTaskInstance`.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] API: Clarify docstrings and assert detail in Task State Store tests [airflow]
Posted by "Eason09053360 (via GitHub)" <gi...@apache.org>.
Eason09053360 commented on code in PR #73939:
URL: https://github.com/apache/airflow/pull/73939#discussion_r4142252156
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_task_state_store.py:
##########
@@ -211,16 +211,29 @@ def test_non_finite_float_rejected_by_validator(self, bad_value):
TaskStateStoreBody(value=bad_value)
def test_set_nonexistent_dag_run_returns_404(self, test_client):
- """set() raises ValueError when DagRun doesn't exist — should surface as 404."""
+ """Scope validation rejects non-existent DagRun with 404."""
bad_url = f"/dags/{DAG_ID}/dagRuns/nonexistent_run/taskInstances/{TASK_ID}/state-store/job_id"
response = test_client.put(bad_url, json={"value": "v"})
assert response.status_code == 404
+ assert "Task instance not found" in response.json()["detail"]
def test_set_nonexistent_task_id_returns_404(self, test_client):
- """set() returns 404 when task_id doesn not match any TaskInstance in the run."""
+ """Scope validation returns 404 when task_id does not match any TaskInstance in the run."""
bad_url = f"/dags/{DAG_ID}/dagRuns/{RUN_ID}/taskInstances/nonexistent_task/state-store/job_id"
response = test_client.put(bad_url, json={"value": "v"})
assert response.status_code == 404
+ assert "Task instance not found" in response.json()["detail"]
+
Review Comment:
Non-blocking: after this change, these two tests might same as the existing `put` cases of `TestUnknownTaskInstance::test_returns_404` (unknown run / unknown task → 404 "Task instance not found"), so they no longer add coverage. Could we delete them? The suggestion below removes both.
```suggestion
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org