Skip to content

Commit

Permalink
fix: Broken tests due to timezone offsets
Browse files Browse the repository at this point in the history
This commit fixes a bug in the test suite that was caused by looking up
the offsets for a specific timezone by the name of the timezone using
the current time as the time to calculate the offset.  This means that
for timezones that are dst observing, they were returning an incorrect
offset breaking the test suite.
  • Loading branch information
staylorwr committed Apr 23, 2021
1 parent 797e1de commit e7e0b47
Show file tree
Hide file tree
Showing 8 changed files with 26 additions and 20 deletions.
2 changes: 1 addition & 1 deletion lib/ex_teal/metric/metric.ex
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ defmodule ExTeal.Metric do
value: integer()
}

@callback date_field() :: :naive_date_time | :utc_datetime
@callback date_field() :: atom()
@callback date_field_type() :: :naive_date_time | :utc_datetime
@callback multiple_results() :: boolean()

Expand Down
4 changes: 2 additions & 2 deletions lib/ex_teal/metric/postgres_trend_expression.ex
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ defmodule ExTeal.Metric.PostgresTrendExpression do
use ExTeal.Metric.TrendExpression

@impl true
def generate(query, metric, timezone, unit) do
offset = fetch_offset(timezone)
def generate(query, metric, timezone, unit, start_dt) do
offset = fetch_offset(timezone, start_dt)

format = date_format(unit)

Expand Down
2 changes: 1 addition & 1 deletion lib/ex_teal/metric/trend.ex
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ defmodule ExTeal.Metric.Trend do
results =
query
|> aggregate_as(aggregate_type, field)
|> TrendExpressionFactory.make(metric, timezone, request.unit)
|> TrendExpressionFactory.make(metric, timezone, request.unit, start_dt)
|> between(
start_dt: start_dt,
end_dt: end_dt,
Expand Down
11 changes: 6 additions & 5 deletions lib/ex_teal/metric/trend_expression.ex
Original file line number Diff line number Diff line change
Expand Up @@ -13,27 +13,28 @@ defmodule ExTeal.Metric.TrendExpression do
query :: Ecto.Queryable.t(),
metric :: module(),
timezone :: String.t(),
unit :: String.t()
unit :: String.t(),
start_dt :: DateTime.t()
) ::
Ecto.Queryable.t()

defmacro __using__(_opts) do
quote do
@behaviour ExTeal.Metric.TrendExpression
import Ecto.Query
import ExTeal.Metric.TrendExpression, only: [fetch_offset: 1]
import ExTeal.Metric.TrendExpression, only: [fetch_offset: 2]
end
end

use Timex

@type valid_timezone :: String.t() | integer() | :utc | :local

@spec fetch_offset(String.t()) :: float()
def fetch_offset(timezone) do
@spec fetch_offset(timezone :: String.t(), datetime :: DateTime.t()) :: float()
def fetch_offset(timezone, datetime) do
seconds =
timezone
|> Timezone.get(Timex.now())
|> Timezone.get(datetime)
|> Timezone.total_offset()

seconds / 3_600
Expand Down
4 changes: 2 additions & 2 deletions lib/ex_teal/metric/trend_expression_factory.ex
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ defmodule ExTeal.Metric.TrendExpressionFactory do
Based on the adapter, select a trend factory and build
a date expression to be used in a Ecto Query fragment for the query
"""
def make(query, metric, timezone, unit) do
def make(query, metric, timezone, unit, start_dt) do
case metric.repo().__adapter__ do
Ecto.Adapters.Postgres ->
PostgresTrendExpression.generate(query, metric, timezone, unit)
PostgresTrendExpression.generate(query, metric, timezone, unit, start_dt)

adapter ->
raise ArgumentError, message: "Adapter #{adapter} does not support trend metrics"
Expand Down
15 changes: 10 additions & 5 deletions test/ex_teal/metric/postgres_trend_expression_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,12 @@ defmodule ExTeal.Metric.PostgresTrendExpressionTest do
results =
Post
|> select([p], %{aggregate: fragment("count(?) as aggregate", p.id)})
|> PostgresTrendExpression.generate(NewPostTrend, "America/Chicago", "year")
|> PostgresTrendExpression.generate(
NewPostTrend,
"America/Chicago",
"year",
two_years_ago
)
|> Repo.all()

assert results == [
Expand Down Expand Up @@ -59,7 +64,7 @@ defmodule ExTeal.Metric.PostgresTrendExpressionTest do
results =
Post
|> select([p], %{aggregate: fragment("count(?) as aggregate", p.id)})
|> PostgresTrendExpression.generate(NewPostTrend, "America/Chicago", "month")
|> PostgresTrendExpression.generate(NewPostTrend, "America/Chicago", "month", dt)
|> Repo.all()

assert results == [
Expand Down Expand Up @@ -104,7 +109,7 @@ defmodule ExTeal.Metric.PostgresTrendExpressionTest do
results =
Post
|> select([p], %{aggregate: fragment("count(?) as aggregate", p.id)})
|> PostgresTrendExpression.generate(NewPostTrend, "America/Chicago", "week")
|> PostgresTrendExpression.generate(NewPostTrend, "America/Chicago", "week", dt)
|> Repo.all()

assert results == [
Expand Down Expand Up @@ -138,7 +143,7 @@ defmodule ExTeal.Metric.PostgresTrendExpressionTest do
results =
Post
|> select([p], %{aggregate: fragment("count(?) as aggregate", p.id)})
|> PostgresTrendExpression.generate(NewPostTrend, "America/Chicago", "hour")
|> PostgresTrendExpression.generate(NewPostTrend, "America/Chicago", "hour", dt)
|> Repo.all()

expected_date_results =
Expand Down Expand Up @@ -171,7 +176,7 @@ defmodule ExTeal.Metric.PostgresTrendExpressionTest do
results =
Post
|> select([p], %{aggregate: fragment("count(?) as aggregate", p.id)})
|> PostgresTrendExpression.generate(NewPostTrend, "America/Chicago", "minute")
|> PostgresTrendExpression.generate(NewPostTrend, "America/Chicago", "minute", dt)
|> Repo.all()

expected_date_results =
Expand Down
2 changes: 1 addition & 1 deletion test/ex_teal/metric/trend_expression_factory_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ defmodule ExTeal.Metric.TrendExpressionFactoryTest do
describe "make/1" do
test "throws an error for an unsupported adapter" do
assert_raise ArgumentError, fn ->
TrendExpressionFactory.make(User, FooTrend, "Etc/UTC", "day")
TrendExpressionFactory.make(User, FooTrend, "Etc/UTC", "day", Timex.now())
end
end
end
Expand Down
6 changes: 3 additions & 3 deletions test/ex_teal/metric/trend_expression_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ defmodule ExTeal.Metric.TrendExpressionTest do

alias ExTeal.Metric.TrendExpression

test "fetch_offset/1 returns the offset in hours" do
assert TrendExpression.fetch_offset("Etc/UTC") == 0
assert TrendExpression.fetch_offset("America/Chicago")
test "fetch_offset/2 returns the offset in hours" do
assert TrendExpression.fetch_offset("Etc/UTC", Timex.now()) == 0
assert TrendExpression.fetch_offset("America/Chicago", Timex.now())
end
end

0 comments on commit e7e0b47

Please sign in to comment.