rusackas commented on PR #39782: URL: https://github.com/apache/superset/pull/39782#issuecomment-4433071036
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. -- 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]
