You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by "owenwillison-vajra (via GitHub)" <gi...@apache.org> on 2026/08/25 16:45:19 UTC
[PR] Fix focused standalone vectorized query verification [skywalking-banyandb]
owenwillison-vajra opened a new pull request, #1310:
URL: https://github.com/apache/skywalking-banyandb/pull/1310
Fix the standalone vectorized-query suite when it is run with a Ginkgo filter.
The vector-dispatch coverage assertion now applies only to the full table. A focused TopN case intentionally uses the row path and cannot satisfy that unrelated coverage check. The full, unfiltered suite retains the assertion, and filter detection has unit coverage.
This changes test control flow only; there is no production performance impact.
Flaky-Test: github.com/apache/skywalking-banyandb/test/integration/standalone/query.TestPropertyIntegrationQuery
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix focused standalone vectorized query verification [skywalking-banyandb]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #1310:
URL: https://github.com/apache/skywalking-banyandb/pull/1310#issuecomment-5413875789
## [Codecov](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/1310?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 50.22%. Comparing base ([`3530dd9`](https://app.codecov.io/gh/apache/skywalking-banyandb/commit/3530dd9b5cefaa4fef60eec91ecbfbdfcaf243c5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`8cd4b60`](https://app.codecov.io/gh/apache/skywalking-banyandb/commit/8cd4b60aa57c62ce6c0ea59dba83931a199484ae?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 404 commits behind head on main.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #1310 +/- ##
==========================================
+ Coverage 45.97% 50.22% +4.24%
==========================================
Files 328 281 -47
Lines 55505 29032 -26473
==========================================
- Hits 25520 14582 -10938
+ Misses 27909 13152 -14757
+ Partials 2076 1298 -778
```
| [Flag](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/1310/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [fodc](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/1310/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `59.91% <ø> (?)` | |
| [pkg](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/1310/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `47.56% <ø> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/1310?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix focused standalone vectorized query verification [skywalking-banyandb]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #1310:
URL: https://github.com/apache/skywalking-banyandb/pull/1310#discussion_r3855586710
##########
test/integration/standalone/query/vectorized_test.go:
##########
@@ -135,19 +136,21 @@ var _ = ginkgo.Describe("vec independent verification (standalone)", ginkgo.Orde
// any later test that targets the row baseline cluster (same
// process) sees the original proto codec contract.
data.SetMeasureWireModeRaw(savedWireModeRaw)
- // G8e observability: dispatch MUST fire for at least one of the
- // replayed cases. If this assertion ever drops to zero, the
- // vec subsystem is silently 0%-covered — either the dispatch
- // eligibility gate is too tight or the wire-up regressed.
+ // G8e observability: an unfiltered table must dispatch at least one
+ // replayed case. A focused run may select only row-path cases.
handledDelta := vecplan.HandledCount() - startHandledCount
fellThroughDelta := vecplan.FellThroughCount() - startFellThroughCount
ginkgo.GinkgoWriter.Printf(
"vec dispatch (standalone): handled=%d fell_through=%d (deltas across vec-standalone table)\n",
handledDelta, fellThroughDelta,
)
- gomega.Expect(handledDelta).To(gomega.BeNumerically(">", int64(0)),
- "vec dispatch did not fire for any case in the vec-standalone table; "+
- "either the eligibility gate is too tight or processor.go's tryVecDispatch regressed")
+ if suiteFiltersSpecs() {
+ ginkgo.GinkgoWriter.Println("vec dispatch proof omitted because Ginkgo filters selected a subset of the table")
+ } else {
Review Comment:
The bool helper name `suiteFiltersSpecs` is misleading (it doesn't filter specs; it detects whether the run is configured with focus/skip/label/semver filters). The log message also claims a subset was selected, but the check only detects that filters were provided, not what they matched. Consider renaming to a "has"/"is"-style predicate and adjusting the message to match the actual behavior.
##########
test/integration/standalone/query/vectorized_config_test.go:
##########
@@ -0,0 +1,45 @@
+// Licensed to Apache Software Foundation (ASF) under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership. The ASF
+// licenses this file to You under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations
+// under the License.
+
+package query_test
+
+import (
+ "testing"
+
+ "github.com/onsi/ginkgo/v2/types"
+ "github.com/stretchr/testify/assert"
+)
+
+func TestHasSpecFilter(t *testing.T) {
+ testCases := []struct {
+ name string
+ suiteConfig types.SuiteConfig
+ want bool
+ }{
+ {name: "unfiltered suite", want: false},
+ {name: "focused specs", suiteConfig: types.SuiteConfig{FocusStrings: []string{"TopN"}}, want: true},
+ {name: "skipped specs", suiteConfig: types.SuiteConfig{SkipStrings: []string{"TopN"}}, want: true},
+ {name: "focused files", suiteConfig: types.SuiteConfig{FocusFiles: []string{"vectorized_test.go"}}, want: true},
+ {name: "skipped files", suiteConfig: types.SuiteConfig{SkipFiles: []string{"vectorized_test.go"}}, want: true},
+ {name: "label filter", suiteConfig: types.SuiteConfig{LabelFilter: "vectorized"}, want: true},
+ {name: "semantic version filter", suiteConfig: types.SuiteConfig{SemVerFilter: ">=1.0.0"}, want: true},
+ }
+
+ for _, testCase := range testCases {
+ t.Run(testCase.name, func(t *testing.T) {
+ assert.Equal(t, testCase.want, hasSpecFilter(testCase.suiteConfig))
+ })
+ }
Review Comment:
`t.Run` executes the subtest function in its own goroutine; capturing the range loop variable can make all subtests observe the last iteration's values (and `go vet` reports this as a loop-closure issue). Rebind the loop variable inside the loop before calling `t.Run`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix focused standalone vectorized query verification [skywalking-banyandb]
Posted by "owenwillison-vajra (via GitHub)" <gi...@apache.org>.
owenwillison-vajra commented on code in PR #1310:
URL: https://github.com/apache/skywalking-banyandb/pull/1310#discussion_r3855671960
##########
test/integration/standalone/query/vectorized_test.go:
##########
@@ -135,19 +136,21 @@ var _ = ginkgo.Describe("vec independent verification (standalone)", ginkgo.Orde
// any later test that targets the row baseline cluster (same
// process) sees the original proto codec contract.
data.SetMeasureWireModeRaw(savedWireModeRaw)
- // G8e observability: dispatch MUST fire for at least one of the
- // replayed cases. If this assertion ever drops to zero, the
- // vec subsystem is silently 0%-covered — either the dispatch
- // eligibility gate is too tight or the wire-up regressed.
+ // G8e observability: an unfiltered table must dispatch at least one
+ // replayed case. A focused run may select only row-path cases.
handledDelta := vecplan.HandledCount() - startHandledCount
fellThroughDelta := vecplan.FellThroughCount() - startFellThroughCount
ginkgo.GinkgoWriter.Printf(
"vec dispatch (standalone): handled=%d fell_through=%d (deltas across vec-standalone table)\n",
handledDelta, fellThroughDelta,
)
- gomega.Expect(handledDelta).To(gomega.BeNumerically(">", int64(0)),
- "vec dispatch did not fire for any case in the vec-standalone table; "+
- "either the eligibility gate is too tight or processor.go's tryVecDispatch regressed")
+ if suiteFiltersSpecs() {
+ ginkgo.GinkgoWriter.Println("vec dispatch proof omitted because Ginkgo filters selected a subset of the table")
+ } else {
Review Comment:
Updated the predicate to hasConfiguredSpecFilters and changed the message to say that suite filters are configured. This matches the check’s actual condition without inferring that a subset was selected.
##########
test/integration/standalone/query/vectorized_config_test.go:
##########
@@ -0,0 +1,45 @@
+// Licensed to Apache Software Foundation (ASF) under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership. The ASF
+// licenses this file to You under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations
+// under the License.
+
+package query_test
+
+import (
+ "testing"
+
+ "github.com/onsi/ginkgo/v2/types"
+ "github.com/stretchr/testify/assert"
+)
+
+func TestHasSpecFilter(t *testing.T) {
+ testCases := []struct {
+ name string
+ suiteConfig types.SuiteConfig
+ want bool
+ }{
+ {name: "unfiltered suite", want: false},
+ {name: "focused specs", suiteConfig: types.SuiteConfig{FocusStrings: []string{"TopN"}}, want: true},
+ {name: "skipped specs", suiteConfig: types.SuiteConfig{SkipStrings: []string{"TopN"}}, want: true},
+ {name: "focused files", suiteConfig: types.SuiteConfig{FocusFiles: []string{"vectorized_test.go"}}, want: true},
+ {name: "skipped files", suiteConfig: types.SuiteConfig{SkipFiles: []string{"vectorized_test.go"}}, want: true},
+ {name: "label filter", suiteConfig: types.SuiteConfig{LabelFilter: "vectorized"}, want: true},
+ {name: "semantic version filter", suiteConfig: types.SuiteConfig{SemVerFilter: ">=1.0.0"}, want: true},
+ }
+
+ for _, testCase := range testCases {
+ t.Run(testCase.name, func(t *testing.T) {
+ assert.Equal(t, testCase.want, hasSpecFilter(testCase.suiteConfig))
+ })
+ }
Review Comment:
No change is needed: these subtests do not call t.Parallel, so t.Run waits for each subtest before the loop advances. The closure therefore observes its own iteration, and go vet is clean.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix focused standalone vectorized query verification [skywalking-banyandb]
Posted by "hanahmily (via GitHub)" <gi...@apache.org>.
hanahmily closed pull request #1310: Fix focused standalone vectorized query verification
URL: https://github.com/apache/skywalking-banyandb/pull/1310
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix focused standalone vectorized query verification [skywalking-banyandb]
Posted by "hanahmily (via GitHub)" <gi...@apache.org>.
hanahmily commented on PR #1310:
URL: https://github.com/apache/skywalking-banyandb/pull/1310#issuecomment-5427513992
It is a stale failure
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org