Fix timezone sensitive tests #870
Conversation
Some tests assume they're running in the UTC timezone. It's much simpler to use a fixed UTC timezone, than to make the expected test data correct for all timezones. The docker container tests were always run in always used UTC.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #870 +/- ##
========================================
Coverage 84.38% 84.39%
========================================
Files 35 35
Lines 4228 4230 +2
Branches 537 537
========================================
+ Hits 3568 3570 +2
Misses 464 464
Partials 196 196 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| @@ -147,6 +147,12 @@ def odc_test_db(cfg_env): | |||
| # during testing, and need any performance gains we can get. | |||
|
|
|||
| with index._db._engine.begin() as conn: # type: ignore[attr-defined] | |||
| # Some tests are sensitive to the timezone of the database, and expect it to be UTC | |||
| quoted_db_name = conn.dialect.identifier_preparer.quote( | |||
There was a problem hiding this comment.
Quoting the database name is not done by the corresponding code in datacube-core.
Since multiple downstream applications seem to need to re-invent the stuff already in datacube-core, can you make a PR on datacube-core that alters the timezone for a database so we can get rid of this functionality in Explorer in the future?
There was a problem hiding this comment.
I might not have bothered with the quoting, but I happened to name my test database odc-explorer-test, and Postgres requires that to be inside double quotes.
I'll think about it. It's kind of a weird niche testing thing, that I'm not sure is universally useful. On the other hand, datacube-core does take care of things like roles and permissions.
I'd want to do some more research and investigation into whether:
- the database timezone should always be UTC, or if it might make sense to have in a local timezone in some cases.
- if changing it would break things, or be unexpected otherwise
- it might be better as a warning when running
datacube database initorsystem check
omad
force-pushed
the
fix-timezone-sensitive-tests
branch
from
824c1d2 to
947e5b4
Compare
These tests started failing when I started running them outside the usual docker container, or with a different database configuration.
While it's possible there are bugs with the exact bounds of summary data generation, and not just in the test code, I absolutely do not want to go there this week.
The goal here is to make the tests more useful by being less flaky and move onto actual bugs/features/performance improvements.
📚 Documentation preview 📚: https://datacube-explorer--870.org.readthedocs.build/en/870/