codeant-ai-for-open-source[bot] commented on code in PR #39404:
URL: https://github.com/apache/superset/pull/39404#discussion_r3525684083
##########
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:
**Suggestion:** The DST setup uses reversed `is_dst` flags: the
pre-transition timestamp is marked as DST and the post-transition timestamp as
standard time, which is opposite for US/Eastern on this date. This can produce
incorrect offsets (or brittle behavior across timezone implementations) and
make the test validate the wrong scenario. Swap the flags (`False` before
transition, `True` after) or omit them for these unambiguous times. [incorrect
variable usage]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ DST transition test mislabels pre/post DST offsets.
- ⚠️ datetime_to_epoch DST coverage brittle to timezone changes.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run pytest targeting `test_datetime_to_epoch_dst_transition` in
`tests/unit_tests/utils/test_date_parsing.py` (function defined around lines
306-327 in
the PR hunk and lines 87-107 in the current file).
2. Observe the pre-transition datetime construction: `dt_before_dst =
eastern.localize(datetime(2023, 3, 12, 1, 59, 59), is_dst=True)` at
`tests/unit_tests/utils/test_date_parsing.py:313` in the PR (line 94 in the
current file),
which marks 01:59:59 as DST even though US/Eastern is still on standard time
(EST) before
the 02:00 spring-forward.
3. Observe the post-transition datetime construction: `dt_after_dst =
eastern.localize(datetime(2023, 3, 12, 3, 0, 1), is_dst=False)` at
`tests/unit_tests/utils/test_date_parsing.py:317` in the PR (line 98 in the
current file),
which marks 03:00:01 as non-DST even though US/Eastern is on daylight time
(EDT) after the
transition.
4. These aware datetimes are passed into `datetime_to_epoch()` in
`superset/utils/dates.py:24-30`, which normalizes to UTC via
`dttm.astimezone(pytz.utc)`;
because the `is_dst` flags are reversed relative to the comment (`1:59:59
EST -> 3:00:01
EDT`), the test conceptually models the wrong pre/post DST states, making
the edge-case
coverage brittle and potentially incorrect if timezone implementation
behavior around
`is_dst` changes.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=3154b12dd3c14391a975eef1e0074b81&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=3154b12dd3c14391a975eef1e0074b81&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/utils/test_date_parsing.py
**Line:** 313:317
**Comment:**
*Incorrect Variable Usage: The DST setup uses reversed `is_dst` flags:
the pre-transition timestamp is marked as DST and the post-transition timestamp
as standard time, which is opposite for US/Eastern on this date. This can
produce incorrect offsets (or brittle behavior across timezone implementations)
and make the test validate the wrong scenario. Swap the flags (`False` before
transition, `True` after) or omit them for these unambiguous times.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39404&comment_hash=4ccf99fa2ac5731c53a0cd0481d1e0004ede4e17714e056b71d3c3a51b8f87ae&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39404&comment_hash=4ccf99fa2ac5731c53a0cd0481d1e0004ede4e17714e056b71d3c3a51b8f87ae&reaction=dislike'>👎</a>
--
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]
