Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setup test fixtures for resolvers #5317

Merged
merged 34 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
698fcc1
resolvers-test-fixture
egor-ryashin Jul 20, 2024
c76d60c
resolvers tests - clickhouse
egor-ryashin Aug 1, 2024
4836668
resolvers tests - druid
egor-ryashin Aug 1, 2024
c5e1560
resolvers tests - style
egor-ryashin Aug 2, 2024
eb344c1
Merge remote-tracking branch 'origin/main' into resolvers-test-fixture
egor-ryashin Aug 2, 2024
eeab03b
resolvers tests - merge fix
egor-ryashin Aug 2, 2024
4b7baa6
resolvers tests - merge fix
egor-ryashin Aug 2, 2024
7979b7d
resolvers tests - style
egor-ryashin Aug 2, 2024
a980ad2
resolvers tests - druid
egor-ryashin Aug 5, 2024
a2989ec
resolvers tests - updates
egor-ryashin Aug 7, 2024
7fce8af
resolvers tests - go.mod
egor-ryashin Aug 12, 2024
a9d5ec7
resolvers tests - masked
egor-ryashin Aug 12, 2024
ead0534
resolvers tests - testdata
egor-ryashin Aug 12, 2024
0dd4820
resolvers tests - zap
egor-ryashin Aug 12, 2024
5976e37
resolvers tests - filepath
egor-ryashin Aug 12, 2024
a028a8a
resolvers tests - reconcile errors
egor-ryashin Aug 13, 2024
2246de9
resolvers tests - reconcile errors
egor-ryashin Aug 21, 2024
27456cd
resolvers tests - reconcile errors
egor-ryashin Aug 21, 2024
578e919
resolvers tests - csv
egor-ryashin Aug 21, 2024
4ffb84c
resolvers tests - error update
egor-ryashin Aug 21, 2024
ac3bb1c
resolvers tests - gocritic
egor-ryashin Aug 21, 2024
3d9ad74
resolvers tests - filepath
egor-ryashin Aug 21, 2024
10d5848
resolvers tests - style
egor-ryashin Aug 22, 2024
4f3da82
resolvers tests - clickhouse image
egor-ryashin Sep 2, 2024
a457f04
Merge remote-tracking branch 'origin/main' into resolvers-test-fixture
egor-ryashin Sep 2, 2024
5b51e56
merge fix
egor-ryashin Sep 2, 2024
594ba0c
clickhouse image version
egor-ryashin Sep 3, 2024
1da4e0d
Merge remote-tracking branch 'origin/main' into resolvers-test-fixture
egor-ryashin Sep 3, 2024
5188c19
resolvers tests - CH CSV ISO ts parsing
egor-ryashin Sep 4, 2024
ddf6c00
resolvers tests - .env
egor-ryashin Sep 5, 2024
ba55ff7
resolvers tests - url mask
egor-ryashin Sep 5, 2024
2191e3c
resolvers tests - .env fail fast
egor-ryashin Sep 5, 2024
bd11fd8
resolvers tests - testClaims
egor-ryashin Sep 5, 2024
d38f6b0
resolvers tests - testClaims
egor-ryashin Sep 5, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/go-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,5 @@ jobs:
run: test -z $(gofmt -l .)
- name: Go test
run: go test -short -v ./...
env:
RILL_RUNTIME_DRUID_TEST_DSN: ${{ secrets.RILL_RUNTIME_DRUID_TEST_DSN }}
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ require (
github.com/mattn/go-isatty v0.0.20 // indirect
github.com/mattn/go-runewidth v0.0.15 // indirect
github.com/mattn/go-sqlite3 v2.0.3+incompatible // indirect
github.com/maxatome/go-testdeep v1.14.0 // indirect
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indirect dependency added without change to a direct dependency – try running go mod tidy

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d // indirect
github.com/minio/asm2plan9s v0.0.0-20200509001527-cdd76441f9d8 // indirect
github.com/minio/c2goasm v0.0.0-20190812172519-36a3d3bbc4f3 // indirect
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1718,6 +1718,8 @@ github.com/mattn/go-sqlite3 v2.0.3+incompatible h1:gXHsfypPkaMZrKbD5209QV9jbUTJK
github.com/mattn/go-sqlite3 v2.0.3+incompatible/go.mod h1:FPy6KqzDD04eiIsT53CuJW3U88zkxoIYsOqkbpncsNc=
github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0=
github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369/go.mod h1:BSXmuO+STAnVfrANrmjBb36TMTDstsz7MSK+HVaYKv4=
github.com/maxatome/go-testdeep v1.14.0 h1:rRlLv1+kI8eOI3OaBXZwb3O7xY3exRzdW5QyX48g9wI=
github.com/maxatome/go-testdeep v1.14.0/go.mod h1:lPZc/HAcJMP92l7yI6TRz1aZN5URwUBUAfUNvrclaNM=
github.com/maxbrunsfeld/counterfeiter/v6 v6.2.2/go.mod h1:eD9eIE7cdwcMi9rYluz88Jz2VyhSmden33/aXg4oVIY=
github.com/mazznoer/csscolorparser v0.1.3 h1:vug4zh6loQxAUxfU1DZEu70gTPufDPspamZlHAkKcxE=
github.com/mazznoer/csscolorparser v0.1.3/go.mod h1:Aj22+L/rYN/Y6bj3bYqO3N6g1dtdHtGfQ32xZ5PJQic=
Expand Down
2 changes: 2 additions & 0 deletions runtime/drivers/clickhouse/olap.go
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,8 @@ func databaseTypeToPB(dbt string, nullable bool) (*runtimev1.Type, error) {
t.Code = runtimev1.Type_CODE_STRING
case "OTHER":
t.Code = runtimev1.Type_CODE_JSON
case "NOTHING":
t.Code = runtimev1.Type_CODE_STRING
default:
match = false
}
Expand Down
7 changes: 7 additions & 0 deletions runtime/drivers/druid/druidsqldriver/druid_api_sql_driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"net/http"
"reflect"
"strconv"
"strings"
"time"

"github.com/google/uuid"
Expand Down Expand Up @@ -65,12 +66,18 @@ func (c *sqlConnection) QueryContext(ctx context.Context, query string, args []d
})
req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.dsn, bodyReader)
if err != nil {
if strings.Contains(err.Error(), c.dsn) { // avoid returning the actual DSN with the password which will be logged
return nil, fmt.Errorf("invalid dsn")
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps return the original error, but with something like strings.Replace(errMsg, c.dsn, "<masked>")?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

return nil, err
}

req.Header.Add("Content-Type", "application/json")
resp, err := c.client.Do(req)
if err != nil {
if strings.Contains(err.Error(), c.dsn) { // avoid returning the actual DSN with the password which will be logged
return nil, fmt.Errorf("invalid dsn")
}
return nil, err
}

Expand Down
11 changes: 11 additions & 0 deletions runtime/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import (
"encoding/json"
"fmt"
"io"
"strconv"

"github.com/mitchellh/hashstructure/v2"
runtimev1 "github.com/rilldata/rill/proto/gen/rill/runtime/v1"
"github.com/rilldata/rill/runtime/drivers"
"github.com/rilldata/rill/runtime/pkg/jsonval"
Expand Down Expand Up @@ -135,6 +137,15 @@ func (r *Runtime) Resolve(ctx context.Context, opts *ResolveOptions) (ResolverRe
if _, err := hash.Write([]byte(resolver.Key())); err != nil {
return nil, err
}
if opts.Claims.UserAttributes != nil {
h, err := hashstructure.Hash(opts.Claims.UserAttributes, hashstructure.FormatV2, nil)
if err != nil {
return nil, err
}
if _, err = hash.Write([]byte(strconv.FormatUint(h, 16))); err != nil {
return nil, err
}
}
for _, ref := range resolver.Refs() {
res, err := ctrl.Get(ctx, ref, false)
if err != nil {
Expand Down
91 changes: 91 additions & 0 deletions runtime/resolvers/clickhouse_resolvers_test.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
project:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move the .yaml test fixtures into a separate directory at runtime/resolvers/testdata since the Go docs recommend putting non-Go test files in a separate testdata directory

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

models:
ad_bids_mini.yaml:
type: model
sql: SELECT * FROM url('https://raw.githubusercontent.com/rilldata/rill/main/runtime/testruntime/testdata/ad_bids/data/AdBids_mini.csv', CSV)
output:
columns: (id UInt32,timestamp DateTime64,publisher varchar,domain varchar,bid_price Float32,volume UInt8,impressions UInt8,"ad words" varchar,clicks Float32,device varchar)
materialize: true
incremental_strategy: append
dashboards:
ad_bids_mini_metrics_with_policy.yaml:
model: ad_bids_mini
display_name: Ad bids
description:

timeseries: timestamp
smallest_time_grain: ""

dimensions:
- label: Publisher
name: publisher
expression: publisher
description: ""
- label: Domain
property: domain
description: ""

measures:
- label: "Number of bids"
name: bid's number
expression: count(*)
- label: "Total volume"
name: total volume
expression: sum(volume)
- label: "Total impressions"
name: total impressions
expression: sum(impressions)
- label: "Total clicks"
name: total click"s
expression: sum(clicks)

security:
access: true
row_filter: "domain = '{{ .user.domain }}'"
exclude:
- if: "'{{ .user.domain }}' != 'msn.com'"
names:
- total volume
apis:
mv_sql_policy_api.yaml:
kind : api
metrics_sql: |
select
publisher,
domain,
"total impressions"
FROM
ad_bids_mini_metrics_with_policy
connectors:
clickhouse: ~
tests:
simple:
resolver: mv_sql_policy_api
options:
claims:
user_attributes:
domain: yahoo.com
email: user@yahoo.com
result:
- domain: yahoo.com
publisher: Yahoo
"total impressions": 3.0
msn:
resolver: mv_sql_policy_api
options:
claims:
user_attributes:
domain: msn.com
email: user@msn.com
result:
- domain: msn.com
publisher: ""
"total impressions": 3.0
empty:
resolver: mv_sql_policy_api
options:
claims:
user_attributes:
domain: google.com
email: user@google.com
result: ~
57 changes: 57 additions & 0 deletions runtime/resolvers/druid_resolvers_test.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
project:
sources: {}
models: {}
dashboards:
ad_bids_mini_metrics_with_policy.yaml:
model: AdBids
display_name: Ad bids
description:
timeseries: __time
smallest_time_grain: ""
dimensions:
- label: Publisher
name: publisher
expression: publisher
description: ""
- label: Domain
property: domain
description: ""
measures:
- label: "Number of bids"
name: bid's number
expression: count(*)
- label: "Max bid price"
name: max bid price
expression: max(bid_price)
- label: "min bid price"
name: min bid price
expression: min(bid_price)
security:
access: true
# row_filter: "domain = '{{ .user.domain }}'" is not supported in Druid
# exclude: is not supported in Druid
apis:
mv_sql_policy_api.yaml:
kind: api
metrics_sql: "select \n publisher,\n domain, \n \"min bid price\",\n \"max bid price\"\nFROM \n ad_bids_mini_metrics_with_policy \nWHERE\n publisher is not null AND domain = 'news.yahoo.com'\nORDER BY \n publisher,\n domain\nLIMIT 1\n"
connectors:
druid: null
tests:
simple:
resolver: mv_sql_policy_api
options:
instanceid: ""
resolver: ""
resolverproperties: {}
args: {}
claims:
user_attributes:
domain: yahoo.com
email: user@yahoo.com
additionalrules: []
skipchecks: false
result:
- domain: news.yahoo.com
max bid price: 6.00
min bid price: 1.00
publisher: Yahoo
99 changes: 99 additions & 0 deletions runtime/resolvers/duckdb_resolvers_test.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
project:
sources:
ad_bids_mini_source.yaml:
connector: https
path: https://raw.githubusercontent.com/rilldata/rill/main/runtime/testruntime/testdata/ad_bids/data/AdBids_mini.csv
models:
"ad_bids_mini.yaml":
sql: |
select
id,
timestamp,
publisher,
domain,
volume,
impressions,
clicks
from ad_bids_mini_source
dashboards:
ad_bids_mini_metrics_with_policy.yaml:
model: ad_bids_mini
display_name: Ad bids
description:

timeseries: timestamp
smallest_time_grain: ""

dimensions:
- label: Publisher
name: publisher
expression: upper(publisher)
description: ""
- label: Domain
property: domain
description: ""

measures:
- label: "Number of bids"
name: bid's number
expression: count(*)
- label: "Total volume"
name: total volume
expression: sum(volume)
- label: "Total impressions"
name: total impressions
expression: sum(impressions)
- label: "Total clicks"
name: total click"s
expression: sum(clicks)

security:
access: true
row_filter: "domain = '{{ .user.domain }}'"
exclude:
- if: "'{{ .user.domain }}' != 'msn.com'"
names:
- total volume
apis:
mv_sql_policy_api.yaml:
kind : api
metrics_sql: |
select
publisher,
domain,
"total impressions"
FROM
ad_bids_mini_metrics_with_policy
connectors:
duckdb: ~
tests:
simple:
resolver: mv_sql_policy_api
options:
claims:
user_attributes:
domain: yahoo.com
email: user@yahoo.com
result:
- domain: yahoo.com
publisher: YAHOO
"total impressions": 3.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there are many output values, this might get tedious to type – do you think it would make sense to optionally support a result_csv: key to represent multi-row output more concisely?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You cannot specify the type number or string with CSV.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

msn:
resolver: mv_sql_policy_api
options:
claims:
user_attributes:
domain: msn.com
email: user@msn.com
result:
- domain: msn.com
publisher: ~
"total impressions": 3.0
empty:
resolver: mv_sql_policy_api
options:
claims:
user_attributes:
domain: google.com
email: user@google.com
result: ~
Loading
Loading