Skip to content

Commit

Permalink
services/ticker: fix flakey spec comparing time with postgres stored …
Browse files Browse the repository at this point in the history
…time (#1774)

Change the `TestInsertOrUpdateAsset` test to `Round` instead of `Truncate` time values that are being compared with time that was stored in Postgres.

Intermittently the `TestInsertOrUpdateAsset` test will fail because a time value that's been stored in Postgres doesn't match the time value we gave to Postgres to store. We attempt to do our best to make them match by using the `Truncate` value on both the input and output values, but that doesn't work for some cases.

I managed to reproduce the failure 100% of the time by using a specific input time of `12:08:50.9419997` where the `milliseconds` component will be rounded up because of the values in the `nanosecond` component by Postgres when it reduces the value to `microseconds`. To see the exact failure case, take a look at the first commit where I hardcoded it for consistent failures. This isn't the only failure case though. When I ran the tests thousands of times I had plenty of times where it failed. Because we use the `Truncate` function, we are comparing a rounded down value with a rounded up value in any cases where the millisecond component gets rounded up when the microsecond component is rounded up.

To illustrate exactly what is happening here, this is our original time:
```
12:08:50.9419997
```

When Postgres is truncating it to microseconds, which is six decimal places, it is rounding to the nearest, which in this case is up:
```
12:08:50.942000
```

In our test we use the truncate function to truncate to milliseconds:
```
12:08:50.941
```

Using `Round` instead causes us to round up to the same value.

The fix in this change isn't perfect according to discussion at lib/pq#227 (comment), where a commenter states that Postgres' rounding function rounds to nearest and *in the event of a tie rounds to even*, which is different to Go's `Round` function which rounds to nearest and *in the event of a tie rounds away from zero*. This would cause a problem in this situation:

However, in the tests I ran with time `12:08:50.9419995` and rounding to the nearest `microsecond`, both Postgres and Go had rounded in the same directions, so I don't see evidence that this is an issue.

Even if this was still an issue I think this change is the best simple change we can make to the test and it would significantly reduce the number of cases this test fails.

Close #1733
  • Loading branch information
leighmcculloch authored Sep 24, 2019
1 parent d72ea29 commit 3e23070
Showing 1 changed file with 12 additions and 12 deletions.
24 changes: 12 additions & 12 deletions services/ticker/internal/tickerdb/queries_asset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,13 @@ func TestInsertOrUpdateAsset(t *testing.T) {
assert.Equal(t, dbIssuer.ID, dbAsset1.IssuerID)
assert.Equal(
t,
firstTime.Local().Truncate(time.Millisecond),
dbAsset1.LastValid.Local().Truncate(time.Millisecond),
firstTime.Local().Round(time.Millisecond),
dbAsset1.LastValid.Local().Round(time.Millisecond),
)
assert.Equal(
t,
firstTime.Local().Truncate(time.Millisecond),
dbAsset1.LastChecked.Local().Truncate(time.Millisecond),
firstTime.Local().Round(time.Millisecond),
dbAsset1.LastChecked.Local().Round(time.Millisecond),
)

// Creating Seconde Asset:
Expand Down Expand Up @@ -109,13 +109,13 @@ func TestInsertOrUpdateAsset(t *testing.T) {
assert.True(t, dbAsset2.LastChecked.After(firstTime))
assert.Equal(
t,
secondTime.Local().Truncate(time.Millisecond),
dbAsset2.LastValid.Local().Truncate(time.Millisecond),
secondTime.Local().Round(time.Millisecond),
dbAsset2.LastValid.Local().Round(time.Millisecond),
)
assert.Equal(
t,
secondTime.Local().Truncate(time.Millisecond),
dbAsset2.LastChecked.Local().Truncate(time.Millisecond),
secondTime.Local().Round(time.Millisecond),
dbAsset2.LastChecked.Local().Round(time.Millisecond),
)

// Creating Third Asset:
Expand All @@ -142,12 +142,12 @@ func TestInsertOrUpdateAsset(t *testing.T) {
assert.True(t, dbAsset3.LastChecked.Before(thirdTime))
assert.Equal(
t,
dbAsset2.LastValid.Local().Truncate(time.Millisecond),
dbAsset3.LastValid.Local().Truncate(time.Millisecond),
dbAsset2.LastValid.Local().Round(time.Millisecond),
dbAsset3.LastValid.Local().Round(time.Millisecond),
)
assert.Equal(
t, dbAsset2.LastValid.Local().Truncate(time.Millisecond),
dbAsset3.LastChecked.Local().Truncate(time.Millisecond),
t, dbAsset2.LastValid.Local().Round(time.Millisecond),
dbAsset3.LastChecked.Local().Round(time.Millisecond),
)
}

Expand Down

0 comments on commit 3e23070

Please sign in to comment.