test: fix test by comparing timezone-aware timestamps by hy144328 · Pull Request #39782 · apache/superset

hy144328 · 2026-04-30T13:55:03Z

SUMMARY

Currently, two testcases rely on the environment to have UTC timezone in order to pass.
The problem is that given timestamp is timezone-native, and that the solution timestamp is a UNIX timestamp, and thus timezone-aware.
Fix the timezone for those timestamps.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

This test passes here but not on the main branch:

env TZ='US/Eastern' python3 -m pytest tests/unit_tests/models/core_test.py -k test_dttm_sql_literal

ADDITIONAL INFORMATION

Has associated issue: test_dttm_sql_literal() fails in non-UTC environment #39781
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

bito-code-review · 2026-04-30T13:55:14Z

Code Review Agent Run #0014a6

Actionable Suggestions - 0

Additional Suggestions - 1

tests/unit_tests/models/core_test.py - 1
- Test Data Inconsistency · Line 204-210
  
  The diff updates some datetime objects to be timezone-aware, but others in the same test remain naive. This creates inconsistency in test data creation. Since dttm_sql_literal uses timestamp() for epoch formats, which assumes local time for naive datetimes, using timezone-aware objects ensures predictable behavior across environments.

Review Details

Files reviewed - 1 · Commit Range: c097733..c097733
- tests/unit_tests/models/core_test.py
Files skipped - 0
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

/review - Manually triggers a full AI review.
/pause - Pauses automatic reviews on this pull request.
/resume - Resumes automatic reviews.
/resolve - Marks all Bito-posted review comments as resolved.
/abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by

netlify · 2026-04-30T13:56:40Z

✅ Deploy Preview for superset-docs-preview ready!

Name	Link
🔨 Latest commit	`14ea0eb`
🔍 Latest deploy log	https://app.netlify.com/projects/superset-docs-preview/deploys/6a0606b3f2de610008f1ce57
😎 Deploy Preview	https://deploy-preview-39782--superset-docs-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

codecov · 2026-04-30T14:41:42Z

Codecov Report

❌ Patch coverage is 20.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.16%. Comparing base (8fa5a75) to head (9eb7e03).
⚠️ Report is 6 commits behind head on master.

Files with missing lines	Patch %	Lines
superset/models/helpers.py	20.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #39782      +/-   ##
==========================================
- Coverage   64.16%   64.16%   -0.01%     
==========================================
  Files        2590     2590              
  Lines      138087   138090       +3     
  Branches    32039    32040       +1     
==========================================
  Hits        88609    88609              
- Misses      47953    47956       +3     
  Partials     1525     1525

Flag	Coverage Δ
hive	`39.46% <20.00%> (-0.01%)`	⬇️
mysql	`59.17% <20.00%> (-0.01%)`	⬇️
postgres	`59.25% <20.00%> (-0.01%)`	⬇️
presto	`41.16% <20.00%> (-0.01%)`	⬇️
python	`60.69% <20.00%> (-0.01%)`	⬇️
sqlite	`58.89% <20.00%> (-0.01%)`	⬇️
unit	`100.00% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

bito-code-review · 2026-05-01T04:19:30Z

Code Review Agent Run #8e8ef1

Actionable Suggestions - 0

Review Details

Files reviewed - 1 · Commit Range: ec790df..ec790df
- tests/unit_tests/models/core_test.py
Files skipped - 0
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

/review - Manually triggers a full AI review.
/pause - Pauses automatic reviews on this pull request.
/resume - Resumes automatic reviews.
/resolve - Marks all Bito-posted review comments as resolved.
/abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by

Mayankaggarwal8055 · 2026-05-02T13:22:00Z

Hey @hy144328, nice catch — timezone issues in tests are always annoying 😅

This looks like a good fix, making the test independent of the environment makes a lot of sense.

Just wondering — are both timestamps being compared in the same timezone explicitly, or does it rely on how Python handles it internally?

Also might be worth adding a quick comment in the test about why this is needed, just so it’s clearer for anyone reading it later.

Overall looks solid.

hy144328 · 2026-05-04T21:42:55Z

@Mayankaggarwal8055 Thanks for the feedback, and sorry for the late reply!

Also might be worth adding a quick comment in the test about why this is needed, just so it’s clearer for anyone reading it later.

Good point.
I have just added a comment to the parameterization of the testcase.
Hope that it makes things clearer.

Just wondering — are both timestamps being compared in the same timezone explicitly, or does it rely on how Python handles it internally?

It relies on how Python implicitly handles timezone-naive timestamps.
If no timezone is provided, the timestamp is assumed relative to the local timezone.
If input and output timestamps are both timezone-naive, this is acceptable.
If exactly one timestamp is timezone-naive, this will inevitably lead to failure unless the timezone is UTC.

bito-code-review · 2026-05-04T22:36:03Z

Code Review Agent Run #24174

Actionable Suggestions - 0

Review Details

Files reviewed - 1 · Commit Range: ec790df..a56bf63
- tests/unit_tests/models/core_test.py
Files skipped - 0
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

/review - Manually triggers a full AI review.
/pause - Pauses automatic reviews on this pull request.
/resume - Resumes automatic reviews.
/resolve - Marks all Bito-posted review comments as resolved.
/abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by

Mayankaggarwal8055 · 2026-05-05T17:06:18Z

Got it, that makes sense — thanks for the clarification!

The explanation about how Python handles timezone-naive vs aware timestamps really helps. The added comment in the test should definitely make this clearer for future readers.

Happy to see this getting addressed 👍

rusackas · 2026-05-12T17:22:49Z

Running CI 🤞

Noting that #40034 tackles this implementation as well... taking a look at that too.

rusackas · 2026-05-12T17:26:46Z

This fix makes the test environment-independent, but it does so by changing the nature of the test input — the datetimes are no longer naive. The production code path being tested (dttm.timestamp() on a naive datetime) is what actually runs in the real application, because the callers throughout the Superset codebase pass naive datetimes. So the test is now no longer testing the real code path — it's testing a scenario that doesn't happen in practice (a timezone-aware input), and the underlying bug in the implementation (where a naive datetime produces a timezone-dependent epoch) remains live and unfixed.

The best aproach would be to combine elements of both...

Fix the implementation to handle naive datetimes deterministically (using timezone.utc, not datetime.UTC), with a proper astimezone() guard for already-aware datetimes.
Keep the test inputs naive (as they reflect real usage), so the test continues to cover the actual production code path.

Mayankaggarwal8055 · 2026-05-12T17:55:40Z

Thanks for the explanation @rusackas — that clears it up a lot.

I didn’t think about the fact that the updated test was no longer covering the actual production path with naive datetimes. Keeping the test behavior aligned with real usage definitely makes more sense here.

Combining both approaches sounds like the best fix:

handling naive datetimes consistently in the implementation
while still keeping the tests focused on the real input scenario

I also noticed the failing pre-commit check. I’ll try running things locally too and see if I can spot anything.

Been enjoying following these timezone and dashboard issues lately — there are a lot of subtle edge cases in Superset.

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Fixes flaky unit tests that depended on the host environment’s local timezone by making the test datetimes explicitly timezone-aware (UTC) so they compare correctly against epoch-based expected values.

Changes:

Import timezone and use timezone.utc on test datetimes used for epoch formatting assertions
Add clarifying comments explaining timezone-naive vs timezone-aware expectations in the parametrized test data

…one-specific timestamp.

hy144328 · 2026-05-14T18:45:34Z

This fix makes the test environment-independent, but it does so by changing the nature of the test input — the datetimes are no longer naive. The production code path being tested (dttm.timestamp() on a naive datetime) is what actually runs in the real application, because the callers throughout the Superset codebase pass naive datetimes. So the test is now no longer testing the real code path — it's testing a scenario that doesn't happen in practice (a timezone-aware input), and the underlying bug in the implementation (where a naive datetime produces a timezone-dependent epoch) remains live and unfixed.

The best aproach would be to combine elements of both...
1. Fix the implementation to handle naive datetimes deterministically (using timezone.utc, not datetime.UTC), with a proper astimezone() guard for already-aware datetimes.

2. Keep the test inputs naive (as they reflect real usage), so the test continues to cover the actual production code path.

@rusackas Thank you for the clarification.
I was reluctant to change the code because, as the one who originally raised #39781 , I observed this bug when running tests locally but not in production.
This said, most servers probably run on UTC so this bug will stay unnoticed for a long time.

As suggested, I have reverted my own changes to the unit test, and applied @charliesheh 's ideas from #40034 .
Would you please give this PR another review?
Apologies for the extra loop.
This would be my first contribution to this project.
Happy to squash into a single commit if the history is not needed anymore.

hy144328 · 2026-05-14T19:01:16Z

/review

bito-code-review

Code Review Agent Run #354d9b

Actionable Suggestions - 1

superset/models/helpers.py - 1
- Assert statement used in production code · Line 2401-2401

Review Details

Files reviewed - 1 · Commit Range: 52fa7f3..9dcf023
- superset/models/helpers.py
Files skipped - 0
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

/review - Manually triggers a full AI review.
/pause - Pauses automatic reviews on this pull request.
/resume - Resumes automatic reviews.
/resolve - Marks all Bito-posted review comments as resolved.
/abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by

hy144328 · 2026-05-14T19:17:45Z

/review

bito-code-review · 2026-05-14T19:17:54Z

Code Review Agent Run #ad8868

Actionable Suggestions - 0

Review Details

Files reviewed - 1 · Commit Range: 52fa7f3..9eb7e03
- superset/models/helpers.py
Files skipped - 0
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

/review - Manually triggers a full AI review.
/pause - Pauses automatic reviews on this pull request.
/resume - Resumes automatic reviews.
/resolve - Marks all Bito-posted review comments as resolved.
/abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by

bito-code-review · 2026-05-15T08:58:51Z

Code Review Agent Run #319287

Actionable Suggestions - 0

Review Details

Files reviewed - 1 · Commit Range: 52fa7f3..9eb7e03
- superset/models/helpers.py
Files skipped - 0
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

/review - Manually triggers a full AI review.
/pause - Pauses automatic reviews on this pull request.
/resume - Resumes automatic reviews.
/resolve - Marks all Bito-posted review comments as resolved.
/abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by

pull-request-size Bot added the size/XS label Apr 30, 2026

dosubot Bot added the global:timezone Related to timezones label Apr 30, 2026

hy144328 force-pushed the test/test_dttm_sql_literal branch from c097733 to 6e2720b Compare April 30, 2026 13:58

hy144328 force-pushed the test/test_dttm_sql_literal branch 2 times, most recently from 801d31c to 992e9b0 Compare April 30, 2026 14:57

hy144328 changed the title ~~test: fix test by comparing timezone-specific timestamps~~ test: fix test by comparing timezone-aware timestamps Apr 30, 2026

hy144328 force-pushed the test/test_dttm_sql_literal branch from 992e9b0 to ec790df Compare April 30, 2026 18:22

pull-request-size Bot added size/S and removed size/XS labels May 4, 2026

rusackas mentioned this pull request May 12, 2026

fix: Normalize epoch timestamp generation to UTC #40034

Open

sadpandajoe requested review from Copilot, rusackas and sadpandajoe May 13, 2026 17:35

Copilot AI reviewed May 13, 2026

View reviewed changes

Comment thread tests/unit_tests/models/core_test.py Outdated

Comment thread tests/unit_tests/models/core_test.py Outdated

Copilot started reviewing on behalf of sadpandajoe May 13, 2026 19:28 View session

Hans Yu added 2 commits May 14, 2026 19:29

test: Fix test by comparing timezone-specific timestamp against timez…

52fa7f3

…one-specific timestamp.

test: Document timezone-naive and timezone aware testcases.

14ea0eb

hy144328 force-pushed the test/test_dttm_sql_literal branch from a56bf63 to 14ea0eb Compare May 14, 2026 17:30

revert: Do not pass timezone-aware datetime objects to tests.

085e1e5

hy144328 force-pushed the test/test_dttm_sql_literal branch from bac86f2 to 9dcf023 Compare May 14, 2026 18:58

fix: Set timezone to UTC before calling timestamp() on datetime object.

9eb7e03

hy144328 force-pushed the test/test_dttm_sql_literal branch from 9dcf023 to 9eb7e03 Compare May 14, 2026 19:12

bito-code-review Bot reviewed May 14, 2026

View reviewed changes

Comment thread superset/models/helpers.py

Conversation

hy144328 commented Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Uh oh!

bito-code-review Bot commented Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #0014a6

Uh oh!

netlify Bot commented Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for superset-docs-preview ready!

Uh oh!

codecov Bot commented Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bito-code-review Bot commented May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #8e8ef1

Uh oh!

Mayankaggarwal8055 commented May 2, 2026

Uh oh!

hy144328 commented May 4, 2026

Uh oh!

bito-code-review Bot commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #24174

Uh oh!

Mayankaggarwal8055 commented May 5, 2026

Uh oh!

rusackas commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rusackas commented May 12, 2026

Uh oh!

Mayankaggarwal8055 commented May 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

hy144328 commented May 14, 2026

Uh oh!

hy144328 commented May 14, 2026

Uh oh!

bito-code-review Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Code Review Agent Run #354d9b

Uh oh!

Uh oh!

hy144328 commented May 14, 2026

Uh oh!

bito-code-review Bot commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #ad8868

Uh oh!

bito-code-review Bot commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Agent Run #319287

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hy144328 commented Apr 30, 2026 •

edited

Loading

bito-code-review Bot commented Apr 30, 2026 •

edited

Loading

netlify Bot commented Apr 30, 2026 •

edited

Loading

codecov Bot commented Apr 30, 2026 •

edited

Loading

bito-code-review Bot commented May 1, 2026 •

edited

Loading

bito-code-review Bot commented May 4, 2026 •

edited

Loading

rusackas commented May 12, 2026 •

edited

Loading

bito-code-review Bot left a comment •

edited

Loading

bito-code-review Bot commented May 14, 2026 •

edited

Loading

bito-code-review Bot commented May 15, 2026 •

edited

Loading