timescale / tsbs

Time Series Benchmark Suite, a tool for comparing and evaluating databases for time series data
MIT License
1.29k stars 300 forks source link

Combine both TimeInterval types into single type #75

Closed RobAtticus closed 5 years ago

RobAtticus commented 5 years ago

Query generation and Cassandra's query running both used a type called TimeInterval that did roughly the same thing. This change combines the two into one type that can be used from the utils package in internal/. This improves code reuse and keeps the two representations in sync, and also increases the testability of the code.

codecov-io commented 5 years ago

Codecov Report

Merging #75 into master will increase coverage by 0.47%. The diff coverage is 60.55%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #75      +/-   ##
==========================================
+ Coverage   57.13%   57.61%   +0.47%     
==========================================
  Files          76       77       +1     
  Lines        3719     3770      +51     
==========================================
+ Hits         2125     2172      +47     
- Misses       1570     1573       +3     
- Partials       24       25       +1
Impacted Files Coverage Δ
...d/tsbs_generate_queries/databases/influx/devops.go 32% <0%> (ø) :arrow_up:
...bs_generate_queries/databases/clickhouse/devops.go 9.94% <0%> (ø) :arrow_up:
...d/tsbs_generate_queries/databases/siridb/devops.go 26.92% <0%> (ø) :arrow_up:
cmd/tsbs_generate_queries/uses/devops/common.go 96.77% <100%> (-0.06%) :arrow_down:
...s_generate_queries/databases/timescaledb/devops.go 98.63% <100%> (+0.01%) :arrow_up:
...sbs_generate_queries/databases/cassandra/devops.go 24.09% <27.27%> (-0.6%) :arrow_down:
internal/utils/time_interval.go 95.83% <95.83%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 3cfaae0...3ce1a1f. Read the comment docs.

RobAtticus commented 5 years ago

Hey @antekresic , asking for a re-review because I added MustRandWindow for cleaner caller code. So this handles the panic issue by doing it in the library (we can eventually remove this in favor of bubbling up the errors).

I also rewrote the tests to use the t.Run trick I learned in your other PR :)