Skip to content

Commit

Permalink
support/db/dbtest: simplify writing tests that store utc time (#2401)
Browse files Browse the repository at this point in the history
### What
Explicitly set the timezone of the client connection to the Postgres database in tests to use the UTC timezone.

### Why
To make writing tests that have assertions on the timestamps stored in `TIMESTAMP WITH TIME ZONE` columns because times read out of Postgres will have their location field set to `time.UTC` and not random times that vary depending on how you run Postgres. This is one way we can ensure our tests run consistently across different development environments, and reduce the barriers to writing short quick tests that contain times.

By default the client connection uses the default timezone of the Postgres server which depends where the Postgres server is running. This can vary a lot because it depends for each developer on how Postgres is being run (inside docker, or locally). In docker with default settings the timezone will be `Etc/UTC` (same offset, but technically not the exact same location as simply `UTC`). When Postgres is installed on a developers machine the timezone may be their local time zone. This makes writing assertions on timestamps returned from Postgres difficult, especially when times are embedded in nested data objects that we're comparing.

The testing difficulty occurs when we're using the `TIMESTAMP WITH TIME ZONE` column type.  Postgres will convert timestamps with time zones from UTC (the timezone that Postgres actually stores the time in on disk) into the clients timezone when reading them out of those fields. When writing tests we cannot do exact equality checks with times unless we match the timezone. There's all sorts of ways to work around this when we do the equality check, but it's much simpler for our tests if we can assume all timestamps that come out of Postgres are in a constant timezone.

Is this safe to do? I think so, because custom timezones doesn't appear to be something we are actively building around. Most of our applications use `TIMESTAMP WITHOUT TIME ZONE` and write only UTC to and from the database.

We are using `TIMESTAMP WITH TIME ZONE` in Ticker and the tests that tests time in that codebase have to work around the timezone being variable by normalizing the times before comparing them. That has contributed to many of the tests in that package testing individual fields of data objects one-by-one instead of testing the data object as a whole. This is fine, but verbose. This change prevents us from needing to do that in tests.

### Why Even Use `TIMESTAMP WITH TIME ZONE`
See this article included in Postgres' Wiki: https://wiki.postgresql.org/wiki/Don%27t_Do_This#Don.27t_use_timestamp_.28without_time_zone.29_to_store_UTC_times

This doesn't invalidate our existing uses of `TIMESTAMP WITHOUT TIME ZONE`, and `TIMESTAMP WITH TIME ZONE` doesn't really make much of a difference to us application developers because we always use UTC in our applications, but even if it doesn't bring any gains to the application developer, it can make it easier for operations folks to interact with the database if they're investigating issues or looking at data and their client connection is using a non-UTC timezone.
  • Loading branch information
leighmcculloch authored Mar 20, 2020
1 parent 8fca4a5 commit bab6abc
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 1 deletion.
2 changes: 1 addition & 1 deletion support/db/dbtest/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func Postgres(t *testing.T) *DB {
// create the db
execStatement(t, pgUser, "CREATE DATABASE "+pq.QuoteIdentifier(result.dbName))

result.DSN = fmt.Sprintf("postgres://%s@localhost/%s?sslmode=disable", pgUser, result.dbName)
result.DSN = fmt.Sprintf("postgres://%s@localhost/%s?sslmode=disable&timezone=UTC", pgUser, result.dbName)

result.closer = func() {
execStatement(t, pgUser, "DROP DATABASE "+pq.QuoteIdentifier(result.dbName))
Expand Down
15 changes: 15 additions & 0 deletions support/db/dbtest/postgres_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ package dbtest
import (
"fmt"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

Expand All @@ -26,3 +28,16 @@ func TestPostgres(t *testing.T) {
// ensure Close() can be called multiple times
db.Close()
}

func TestPostgres_clientTimezone(t *testing.T) {
db := Postgres(t)
conn := db.Open()
defer conn.Close()

timestamp := time.Time{}
err := conn.Get(&timestamp, "SELECT TO_TIMESTAMP('2020-03-19 16:56:00', 'YYYY-MM-DD HH24:MI:SS')")
require.NoError(t, err)

wantTimestamp := time.Date(2020, 3, 19, 16, 56, 0, 0, time.UTC)
assert.Equal(t, wantTimestamp, timestamp)
}

0 comments on commit bab6abc

Please sign in to comment.