rusackas commented on code in PR #39404:
URL: https://github.com/apache/superset/pull/39404#discussion_r3565300050
##########
tests/unit_tests/utils/test_date_parsing.py:
##########
@@ -254,3 +254,89 @@ def test_warning_suppression():
assert len(warnings_list) == 0 # Should suppress all format inference
warnings
assert pd.api.types.is_datetime64_any_dtype(df["date"]) # Should still
parse dates
+
+
+# ============================================================================
+# NEW TESTS FOR datetime_to_epoch() - Edge case coverage
+# ============================================================================
+
+import pytz
+from datetime import datetime
+from superset.utils.dates import datetime_to_epoch
+
+
+def test_datetime_to_epoch_naive_at_epoch():
+ """Test naive datetime exactly at epoch returns 0.0"""
+ # Edge case: Datetime at epoch boundary
+ epoch_dt = datetime(1970, 1, 1, 0, 0, 0)
+ result = datetime_to_epoch(epoch_dt)
+ assert result == 0.0, f"Epoch datetime should be 0.0, got {result}"
+
+
+def test_datetime_to_epoch_naive_one_second_after():
+ """Test naive datetime 1 second after epoch"""
+ dt = datetime(1970, 1, 1, 0, 0, 1)
+ result = datetime_to_epoch(dt)
+ expected = 1000.0 # 1 second * 1000 ms
Review Comment:
Agreed on the coverage gap — since maintainer edits are disabled on this PR
I posted the addition as a suggestion below (comment #3565299507) rather than
pushing directly.
##########
tests/unit_tests/utils/test_date_parsing.py:
##########
@@ -254,3 +257,91 @@ def test_warning_suppression():
assert len(warnings_list) == 0 # Should suppress all format inference
warnings
assert pd.api.types.is_datetime64_any_dtype(df["date"]) # Should still
parse dates
+
+
+# ============================================================================
+# NEW TESTS FOR datetime_to_epoch() - Edge case coverage
+# ============================================================================
+
+
+def test_datetime_to_epoch_naive_at_epoch():
+ """Test naive datetime exactly at epoch returns 0.0"""
+ # Edge case: Datetime at epoch boundary
+ epoch_dt = datetime(1970, 1, 1, 0, 0, 0)
+ result = datetime_to_epoch(epoch_dt)
+ assert result == 0.0, f"Epoch datetime should be 0.0, got {result}"
+
+
+def test_datetime_to_epoch_naive_one_second_after():
+ """Test naive datetime 1 second after epoch"""
+ dt = datetime(1970, 1, 1, 0, 0, 1)
+ result = datetime_to_epoch(dt)
+ expected = 1000.0 # 1 second * 1000 ms
+ assert result == expected, f"Expected {expected}ms, got {result}ms"
+
+
+def test_datetime_to_epoch_timezone_aware_utc():
+ """Test timezone-aware datetime in UTC"""
+ # Create UTC datetime
+ utc_tz = pytz.UTC
+ dt_utc = utc_tz.localize(datetime(1970, 1, 1, 0, 0, 1))
+ result = datetime_to_epoch(dt_utc)
+ expected = 1000.0 # 1 second * 1000 ms
+ assert result == expected, f"UTC datetime should convert correctly, got
{result}ms"
+
+
+def test_datetime_to_epoch_timezone_aware_different_tz():
+ """Test timezone-aware datetime in different timezone converts to UTC
correctly"""
+ # Create datetime in EST (UTC-5 in January)
+ est = pytz.timezone("US/Eastern")
+ # 1970-01-01 05:00:00 EST = 1970-01-01 10:00:00 UTC (5 hours offset)
+ dt_est = est.localize(datetime(1970, 1, 1, 5, 0, 0))
+ result = datetime_to_epoch(dt_est)
+ expected = 10 * 60 * 60 * 1000 # 10 hours in milliseconds
+ assert result == expected, (
+ f"EST datetime should convert to UTC correctly, got {result}ms"
+ )
+
+
+def test_datetime_to_epoch_dst_transition():
+ """Test datetime during DST transition is handled correctly"""
+ # Use a known DST transition date in US/Eastern
+ # 2023-03-12: Spring forward (2 AM becomes 3 AM, gap of 1 hour)
+ eastern = pytz.timezone("US/Eastern")
+
+ # Create datetime before DST transition
+ dt_before_dst = eastern.localize(datetime(2023, 3, 12, 1, 59, 59),
is_dst=True)
+ result_before = datetime_to_epoch(dt_before_dst)
+
+ # Create datetime after DST transition
+ dt_after_dst = eastern.localize(datetime(2023, 3, 12, 3, 0, 1),
is_dst=False)
Review Comment:
Already addressed in a later commit (6a095a04e6) — the flags were flipped
(`is_dst=False` before the transition, `is_dst=True` after) and the tolerance
tightened to an exact `==`.
##########
tests/unit_tests/utils/test_date_parsing.py:
##########
@@ -254,3 +257,91 @@ def test_warning_suppression():
assert len(warnings_list) == 0 # Should suppress all format inference
warnings
assert pd.api.types.is_datetime64_any_dtype(df["date"]) # Should still
parse dates
+
+
+# ============================================================================
+# NEW TESTS FOR datetime_to_epoch() - Edge case coverage
+# ============================================================================
+
+
+def test_datetime_to_epoch_naive_at_epoch():
Review Comment:
Not applying — none of the other test functions in this file (existing or
new) have return type hints either, so this would be inconsistent with the
file's established convention. Would be a separate, file-wide cleanup, not
something to single out on this one function.
##########
tests/unit_tests/utils/test_date_parsing.py:
##########
@@ -254,3 +254,89 @@ def test_warning_suppression():
assert len(warnings_list) == 0 # Should suppress all format inference
warnings
assert pd.api.types.is_datetime64_any_dtype(df["date"]) # Should still
parse dates
+
+
+# ============================================================================
+# NEW TESTS FOR datetime_to_epoch() - Edge case coverage
+# ============================================================================
+
+import pytz
+from datetime import datetime
+from superset.utils.dates import datetime_to_epoch
+
+
+def test_datetime_to_epoch_naive_at_epoch():
+ """Test naive datetime exactly at epoch returns 0.0"""
+ # Edge case: Datetime at epoch boundary
+ epoch_dt = datetime(1970, 1, 1, 0, 0, 0)
+ result = datetime_to_epoch(epoch_dt)
+ assert result == 0.0, f"Epoch datetime should be 0.0, got {result}"
+
+
+def test_datetime_to_epoch_naive_one_second_after():
+ """Test naive datetime 1 second after epoch"""
+ dt = datetime(1970, 1, 1, 0, 0, 1)
+ result = datetime_to_epoch(dt)
+ expected = 1000.0 # 1 second * 1000 ms
+ assert result == expected, f"Expected {expected}ms, got {result}ms"
+
+
+def test_datetime_to_epoch_timezone_aware_utc():
+ """Test timezone-aware datetime in UTC"""
+ # Create UTC datetime
+ utc_tz = pytz.UTC
+ dt_utc = utc_tz.localize(datetime(1970, 1, 1, 0, 0, 1))
+ result = datetime_to_epoch(dt_utc)
+ expected = 1000.0 # 1 second * 1000 ms
+ assert result == expected, f"UTC datetime should convert correctly, got
{result}ms"
+
+
+def test_datetime_to_epoch_timezone_aware_different_tz():
+ """Test timezone-aware datetime in different timezone converts to UTC
correctly"""
+ # Create datetime in EST (UTC-5 in January)
+ est = pytz.timezone('US/Eastern')
+ # 1970-01-01 05:00:00 EST = 1970-01-01 10:00:00 UTC (5 hours offset)
+ dt_est = est.localize(datetime(1970, 1, 1, 5, 0, 0))
+ result = datetime_to_epoch(dt_est)
+ expected = 10 * 60 * 60 * 1000 # 10 hours in milliseconds
+ assert result == expected, f"EST datetime should convert to UTC correctly,
got {result}ms"
+
+
+def test_datetime_to_epoch_dst_transition():
+ """Test datetime during DST transition is handled correctly"""
+ # Use a known DST transition date in US/Eastern
+ # 2023-03-12: Spring forward (2 AM becomes 3 AM, gap of 1 hour)
+ eastern = pytz.timezone('US/Eastern')
+
+ # Create datetime before DST transition
+ dt_before_dst = eastern.localize(datetime(2023, 3, 12, 1, 59, 59),
is_dst=True)
+ result_before = datetime_to_epoch(dt_before_dst)
+
+ # Create datetime after DST transition
+ dt_after_dst = eastern.localize(datetime(2023, 3, 12, 3, 0, 1),
is_dst=False)
+ result_after = datetime_to_epoch(dt_after_dst)
+
+ # The difference should be only 2 seconds, not 1 hour + 2 seconds
+ # (because of the DST jump, 1:59:59 EST -> 3:00:01 EDT)
Review Comment:
Not applying — `datetime_to_epoch()` doesn't do its own DST arithmetic; it
just calls `.astimezone(pytz.utc)` on whatever aware datetime it's given, so
all DST/offset resolution happens inside `pytz` when the datetime is
constructed via `localize()`, not in this function. Testing a literal
nonexistent local time (e.g. 2:30 AM on the spring-forward day) would exercise
pytz's own ambiguous-time handling, not any logic in `datetime_to_epoch`
itself. The existing test already covers the meaningful assertion — that the
UTC-ms difference across a DST-crossing boundary is computed correctly.
--
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]
