SameerMesiah97 commented on code in PR #64054:
URL: https://github.com/apache/airflow/pull/64054#discussion_r2971481370


##########
airflow-core/tests/unit/api_fastapi/execution_api/versions/head/test_xcoms.py:
##########


Review Comment:
   Right now, your new test only covers the performance impact of your changes 
(i.e. when COUNT is called). It doesn’t validate the actual slicing behavior.
   
   We already have E2E coverage in `test_xcom_get_with_slice`, but the dataset 
there is very small (effectively length 3), so some of the more complex cases 
introduced here don’t really get exercised (e.g. mixed-sign slicing that 
requires COUNT normalization, or early return paths).
   
   I think a simple approach would be:
   
   1) Modify this test to includde a slightly larger (but still sparse) 
dataset, for example:
   
   `xcom_values = [0, None, 2, None, 4, 5, None, 7, 8, 9]`
   
   2) Add this line right above the assertions:
   
   `expected = [v for v in xcom_values if v is not None][key]`
   
   3) Assert against that instead of a hardcoded list.
   
   I would also add a few more targeted cases to cover the different scenarios 
(same-sign, mixed-sign, early return, etc).



-- 
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]

Reply via email to