Skip to content

test: add async fetch coverage for TimeWindow data source#869

Open
LeSingh1 wants to merge 2 commits into
NVIDIA:mainfrom
LeSingh1:lesingh1/timewindow-async-fetch-coverage
Open

test: add async fetch coverage for TimeWindow data source#869
LeSingh1 wants to merge 2 commits into
NVIDIA:mainfrom
LeSingh1:lesingh1/timewindow-async-fetch-coverage

Conversation

@LeSingh1
Copy link
Copy Markdown
Contributor

Summary

Adds focused async coverage for TimeWindow.fetch().

This verifies that async fetch:

  • applies time offsets
  • preserves output time coordinates
  • returns variables in the expected order

Closes #589.

Test plan

  • python -m pytest test/data/test_time_window.py -v

Result: 28 passed, 1 skipped.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 20, 2026

Greptile Summary

Adds async coverage for TimeWindow.fetch() by introducing a fetch coroutine on MockDataSource and one new test that verifies variable ordering, time-coordinate preservation, and offset application through the async path.

  • MockDataSource.fetch delegates to __call__, so existing call-history tracking works transparently for the new test.
  • The new test uses asyncio.run() rather than the @pytest.mark.asyncio / async def pattern used everywhere else in the repo, and it only asserts the first of the three offset calls, leaving the 0 h and +6 h paths unverified.

Confidence Score: 4/5

Test-only change; no production code is modified and the new test exercises a real code path.

The test uses asyncio.run() rather than the project's established @pytest.mark.asyncio pattern, and verifies only one of three offset calculations, leaving a gap in the coverage it claims to provide.

test/data/test_time_window.py — async test style and incomplete offset assertions.

Important Files Changed

Filename Overview
test/data/test_time_window.py Adds async def fetch to MockDataSource and one new test covering async variable ordering and time-coordinate alignment; uses asyncio.run() instead of the project's standard @pytest.mark.asyncio pattern, and only asserts the first of three offset calls.

Reviews (1): Last reviewed commit: "test: add async fetch coverage for TimeW..." | Re-trigger Greptile

Comment thread test/data/test_time_window.py Outdated
Comment on lines +365 to +375
def test_fetch_matches_sync_output_coordinates(self):
"""Test async fetch applies offsets and preserves output coordinates."""
ds = MockDataSource()
tw = TimeWindow(
datasource=ds,
offsets=[timedelta(hours=-6), timedelta(hours=0), timedelta(hours=6)],
suffixes=["_tm1", "_t", "_tp1"],
)

base_time = datetime(2024, 1, 1, 12, 0)
result = asyncio.run(tw.fetch(base_time, ["t2m", "u10m"]))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The project is configured with pytest-asyncio (asyncio_mode = "strict") and all other async tests in this repo use @pytest.mark.asyncio with async def (see test_data_utils.py, test_async_zarr.py, test_server_utils.py). Using asyncio.run() in a sync wrapper diverges from that established convention, and calling asyncio.run() from within a running event loop (e.g. if an async fixture or plugin attaches one) will raise RuntimeError: This event loop is already running.

Suggested change
def test_fetch_matches_sync_output_coordinates(self):
"""Test async fetch applies offsets and preserves output coordinates."""
ds = MockDataSource()
tw = TimeWindow(
datasource=ds,
offsets=[timedelta(hours=-6), timedelta(hours=0), timedelta(hours=6)],
suffixes=["_tm1", "_t", "_tp1"],
)
base_time = datetime(2024, 1, 1, 12, 0)
result = asyncio.run(tw.fetch(base_time, ["t2m", "u10m"]))
@pytest.mark.asyncio
async def test_fetch_matches_sync_output_coordinates(self):
"""Test async fetch applies offsets and preserves output coordinates."""
ds = MockDataSource()
tw = TimeWindow(
datasource=ds,
offsets=[timedelta(hours=-6), timedelta(hours=0), timedelta(hours=6)],
suffixes=["_tm1", "_t", "_tp1"],
)
base_time = datetime(2024, 1, 1, 12, 0)
result = await tw.fetch(base_time, ["t2m", "u10m"])

Comment thread test/data/test_time_window.py Outdated
Comment on lines +386 to +388
assert actual_vars == expected_vars
assert result.time.values[0] == np.datetime64(base_time)
assert ds.call_history[0]["time"] == [datetime(2024, 1, 1, 6, 0)]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Only the first offset call (-6 h) is verified. The corresponding sync test test_offset_times_calculated_correctly checks all three call-history entries. Without checking indices 1 (0 h → 12:00) and 2 (+6 h → 18:00) the test could pass even if the offset arithmetic for the later calls was broken.

Suggested change
assert actual_vars == expected_vars
assert result.time.values[0] == np.datetime64(base_time)
assert ds.call_history[0]["time"] == [datetime(2024, 1, 1, 6, 0)]
assert actual_vars == expected_vars
assert result.time.values[0] == np.datetime64(base_time)
assert ds.call_history[0]["time"] == [datetime(2024, 1, 1, 6, 0)] # -6h
assert ds.call_history[1]["time"] == [datetime(2024, 1, 1, 12, 0)] # 0h
assert ds.call_history[2]["time"] == [datetime(2024, 1, 1, 18, 0)] # +6h

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🐛[BUG]: Add better test coverage for TimeWindow data source fetch

1 participant