bito-code-review[bot] commented on PR #43627:
URL: https://github.com/apache/superset/pull/43627#issuecomment-5446971105
<!-- Bito Reply -->
The flagged issue is correct. The current regression tests in
`tests/unit_tests/tasks/test_scheduler_executor.py` verify the final status of
the task but do not confirm that the `InternalStatusTransitionCommand` was
actually invoked to perform the transition. This means the test could pass even
if the task status was set incorrectly or if the transition logic was bypassed.
To resolve this, you should assert that the transition mock was called with
the expected terminal status (`TIMED_OUT` or `ABORTED`) and the correct state.
You can achieve this by inspecting the `transition` mock's call arguments in
your test cases, similar to how `_requested_statuses` is used in the existing
test file.
Would you like me to implement this fix for the flagged tests and check the
rest of the PR comments for similar issues?
**tests/unit_tests/tasks/test_scheduler_executor.py**
```
def test_exception_during_timeout_finalizes_timed_out_not_failure() -> None:
result, transition, task_manager, stats_logger =
_run_body_with_raising_executor(
timeout_triggered=True, abort_detected=False
)
assert TaskStatus.FAILURE not in _requested_statuses(transition)
assert result["status"] == TaskStatus.TIMED_OUT.value
# Add assertion for transition call:
transition.assert_called_with(new_status=TaskStatus.TIMED_OUT, ...)
task_manager.publish_completion.assert_called_once()
```
--
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]
