[query] Fix absent function to work correctly #1871
Conversation
src/query/block/block_test.go
Outdated
| require.Equal(t, len(expectedTags), len(actualTags)) | ||
| for i, tag := range expectedTags { | ||
| fmt.Println("x", string(tag.Name), ":", string(tag.Value)) | ||
| fmt.Println("a", string(actualTags[i].Name), ":", string(actualTags[i].Value)) |
There was a problem hiding this comment.
We should probably remove these Println's now yeah?
| } | ||
| } | ||
|
|
||
| func mustMakeSeriesMeta(tags ...string) block.SeriesMeta { |
There was a problem hiding this comment.
Aren't these available already as test.MustMakeSeriesMeta and test.MustMakeMeta?
There was a problem hiding this comment.
Unfortunately adds a circular test dependency on test and block, is there a decent way of resolving it? Tried block_test package but don't think that's importable from other packages right?
src/query/test/block.go
Outdated
| @@ -228,3 +234,21 @@ func GenerateValuesAndBounds( | |||
|
|
|||
| return values, bounds | |||
| } | |||
|
|
|||
| // MustMakeTags creates tags given that the number of args is even. | |||
| func MustMakeTags(tag ...string) models.Tags { | |||
There was a problem hiding this comment.
Hm doesn't this already appear in block_test.go somewhere in this PR already?
There was a problem hiding this comment.
Yeah managed to move it to block package
There was a problem hiding this comment.
Hm, can we use this method instead of redeclaring this method in block/empty_test.go?
Seems a private version is declared there:
func mustMakeTags(tag ...string) models.Tags {
if len(tag)%2 != 0 {
panic("must have even tag length")
}
tagLength := len(tag) / 2
t := models.NewTags(tagLength, models.NewTagOptions())
for i := 0; i < tagLength; i++ {
t = t.AddTag(models.Tag{
Name: []byte(tag[i*2]),
Value: []byte(tag[i*2+1]),
})
}
return t
}
Codecov Report
@@ Coverage Diff @@
## master #1871 +/- ##
=========================================
+ Coverage 62.7% 65.9% +3.1%
=========================================
Files 1054 982 -72
Lines 101732 83610 -18122
=========================================
- Hits 63864 55115 -8749
+ Misses 33755 24723 -9032
+ Partials 4113 3772 -341
Continue to review full report at Codecov.
|
src/query/block/scalar.go
Outdated
| // allowing them to treat this as a regular block, while at the same time | ||
| // having an option to optimize by accessing the scalar value directly instead | ||
| // This represents constant values; it greatly simplifies downstream operations | ||
| // vy allowing them to treat this as a regular block, while at the same time |
src/query/models/tags.go
Outdated
| } | ||
|
|
||
| return strings.Join(tags, ", ") | ||
| return sb.String() | ||
| // return strings.Join(tags, ", ") |
There was a problem hiding this comment.
Do we need to remove this perhaps?
src/query/models/tags.go
Outdated
| func (t Tags) String() string { | ||
| tags := make([]string, len(t.Tags)) | ||
| var sb strings.Builder | ||
| // tags := make([]string, len(t.Tags)) |
| @@ -154,6 +154,8 @@ var aggregateParseTests = []struct { | |||
| {"bottomk(3, up)", aggregation.BottomKType}, | |||
| {"quantile(3, up)", aggregation.QuantileType}, | |||
| {"count_values(\"some_name\", up)", aggregation.CountValuesType}, | |||
|
|
|||
There was a problem hiding this comment.
nit: Remove this endline?
What this PR does / why we need it:
Fixes #1847