hanahmily commented on code in PR #1150:
URL: 
https://github.com/apache/skywalking-banyandb/pull/1150#discussion_r3341752245


##########
test/cases/trace/cmd/generate/seed.go:
##########
@@ -0,0 +1,230 @@
+// 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. Apache Software Foundation (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 main
+
+import (
+       "encoding/json"
+       "fmt"
+       "os"
+       "path/filepath"
+       "slices"
+)
+
+// TagType describes a trace tag value type.
+type TagType int
+
+const (
+       TagTypeString TagType = iota
+       TagTypeInt
+       TagTypeTimestamp
+)
+
+// Role describes how generated tests use a tag.
+type Role int
+
+const (
+       RoleIdentity Role = iota
+       RoleIndexed
+       RolePlain
+)
+
+// TagDef describes a trace tag in positional seed order.
+type TagDef struct {
+       Name string
+       Type TagType
+       Pos  int
+       Role Role
+}
+
+// SpanRow mirrors one seeded span row used for generation value selection.
+type SpanRow struct {
+       TagValues map[string]any
+       Span      string
+}
+
+// Trace describes the trace schema and rows used by generated tests.
+type Trace struct {
+       Name         string
+       Group        string
+       Groups       []string
+       Tags         []TagDef
+       OrderRules   []string
+       TraceIDTag   string
+       SpanIDTag    string
+       TimestampTag string
+       Rows         []SpanRow
+}
+
+// SeedData returns trace schemas available to the generator.
+func SeedData() []Trace {
+       return []Trace{swTrace()}
+}
+
+// FindTrace returns a trace schema by name.
+func FindTrace(name string) *Trace {
+       for _, traceDef := range SeedData() {
+               if traceDef.Name == name {
+                       return &traceDef
+               }
+       }
+       return nil
+}

Review Comment:
   Fixed in a555985b: `FindTrace` now indexes the `SeedData()` slice (`return 
&traces[idx]`) instead of returning the address of the range-loop copy.



##########
test/cases/trace/cmd/generate/pairwise.go:
##########
@@ -0,0 +1,174 @@
+// 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. Apache Software Foundation (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 main
+
+// Copied verbatim from test/cases/measure/cmd/generate/pairwise.go — keep in 
sync; pure combinatorics, no proto types.
+
+// Pair holds a parameter-value pair for pairwise testing.
+type Pair struct {
+       Param string
+       Value string
+}
+
+// TestVector is a set of parameter-value assignments.
+type TestVector map[string]string
+
+// PairwiseGenerate generates a minimal set of test vectors covering all pairs
+// of parameter values using a simplified IPO (In-Parameter-Order) algorithm.
+func PairwiseGenerate(params map[string][]string, constraints 
[]ConstraintFunc) []TestVector {
+       paramNames := sortedParamNames(params)
+       if len(paramNames) == 0 {
+               return nil
+       }
+
+       // Collect all required pairs
+       requiredPairs := collectAllPairs(params, constraints)
+
+       // Build test vectors using greedy pair covering
+       var vectors []TestVector
+       coveredPairs := make(map[Pair]bool)
+
+       for len(coveredPairs) < len(requiredPairs) {
+               tv := buildVector(params, paramNames, coveredPairs, constraints)
+               if tv == nil {
+                       break
+               }
+               // Mark newly covered pairs
+               newCovers := 0
+               for _, pn := range paramNames {
+                       for _, pn2 := range paramNames {
+                               if pn >= pn2 {
+                                       continue
+                               }
+                               p := Pair{Param: pn + "=" + tv[pn], Value: pn2 
+ "=" + tv[pn2]}
+                               if !coveredPairs[p] {
+                                       coveredPairs[p] = true
+                                       newCovers++
+                               }
+                       }
+               }
+               if newCovers == 0 {
+                       break
+               }
+               vectors = append(vectors, tv)
+       }
+       return vectors
+}
+
+// ConstraintFunc returns false if the combination is invalid.
+type ConstraintFunc func(tv TestVector) bool
+
+func collectAllPairs(params map[string][]string, constraints []ConstraintFunc) 
map[Pair]bool {
+       pairs := make(map[Pair]bool)
+       paramNames := sortedParamNames(params)
+       for _, pn := range paramNames {
+               for _, pn2 := range paramNames {
+                       if pn >= pn2 {
+                               continue
+                       }
+                       for _, v1 := range params[pn] {
+                               for _, v2 := range params[pn2] {
+                                       tv := TestVector{pn: v1, pn2: v2}
+                                       if validConstraints(tv, constraints) {
+                                               pairs[Pair{Param: pn + "=" + 
v1, Value: pn2 + "=" + v2}] = true
+                                       }
+                               }
+                       }
+               }
+       }
+       return pairs
+}
+
+func buildVector(params map[string][]string, paramNames []string, coveredPairs 
map[Pair]bool, constraints []ConstraintFunc) TestVector {
+       tv := make(TestVector)
+       fillVector(tv, params, paramNames, 0, coveredPairs, constraints)
+       return tv
+}

Review Comment:
   Fixed in a555985b: `buildVector` now returns nil when `fillVector` reports 
no constraint-satisfying assignment, so `PairwiseGenerate`'s `if tv == nil { 
break }` is live and the algorithm no longer records a partial/invalid vector.



##########
test/cases/trace/cmd/generate/pairwise.go:
##########
@@ -0,0 +1,174 @@
+// 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. Apache Software Foundation (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 main
+
+// Copied verbatim from test/cases/measure/cmd/generate/pairwise.go — keep in 
sync; pure combinatorics, no proto types.
+
+// Pair holds a parameter-value pair for pairwise testing.
+type Pair struct {
+       Param string
+       Value string
+}
+
+// TestVector is a set of parameter-value assignments.
+type TestVector map[string]string
+
+// PairwiseGenerate generates a minimal set of test vectors covering all pairs
+// of parameter values using a simplified IPO (In-Parameter-Order) algorithm.
+func PairwiseGenerate(params map[string][]string, constraints 
[]ConstraintFunc) []TestVector {
+       paramNames := sortedParamNames(params)
+       if len(paramNames) == 0 {
+               return nil
+       }
+
+       // Collect all required pairs
+       requiredPairs := collectAllPairs(params, constraints)
+
+       // Build test vectors using greedy pair covering
+       var vectors []TestVector
+       coveredPairs := make(map[Pair]bool)
+
+       for len(coveredPairs) < len(requiredPairs) {
+               tv := buildVector(params, paramNames, coveredPairs, constraints)
+               if tv == nil {
+                       break
+               }
+               // Mark newly covered pairs
+               newCovers := 0
+               for _, pn := range paramNames {
+                       for _, pn2 := range paramNames {
+                               if pn >= pn2 {
+                                       continue
+                               }
+                               p := Pair{Param: pn + "=" + tv[pn], Value: pn2 
+ "=" + tv[pn2]}
+                               if !coveredPairs[p] {
+                                       coveredPairs[p] = true
+                                       newCovers++
+                               }
+                       }
+               }
+               if newCovers == 0 {
+                       break
+               }
+               vectors = append(vectors, tv)
+       }
+       return vectors
+}
+
+// ConstraintFunc returns false if the combination is invalid.
+type ConstraintFunc func(tv TestVector) bool
+
+func collectAllPairs(params map[string][]string, constraints []ConstraintFunc) 
map[Pair]bool {
+       pairs := make(map[Pair]bool)
+       paramNames := sortedParamNames(params)
+       for _, pn := range paramNames {
+               for _, pn2 := range paramNames {
+                       if pn >= pn2 {
+                               continue
+                       }
+                       for _, v1 := range params[pn] {
+                               for _, v2 := range params[pn2] {
+                                       tv := TestVector{pn: v1, pn2: v2}
+                                       if validConstraints(tv, constraints) {
+                                               pairs[Pair{Param: pn + "=" + 
v1, Value: pn2 + "=" + v2}] = true
+                                       }
+                               }
+                       }
+               }
+       }
+       return pairs
+}
+
+func buildVector(params map[string][]string, paramNames []string, coveredPairs 
map[Pair]bool, constraints []ConstraintFunc) TestVector {
+       tv := make(TestVector)
+       fillVector(tv, params, paramNames, 0, coveredPairs, constraints)
+       return tv
+}
+
+func fillVector(tv TestVector, params map[string][]string, paramNames 
[]string, paramIdx int, coveredPairs map[Pair]bool, constraints 
[]ConstraintFunc) bool {
+       if paramIdx >= len(paramNames) {
+               return validConstraints(tv, constraints)
+       }
+       pn := paramNames[paramIdx]
+       bestVal := ""
+       bestScore := -1
+       for _, val := range params[pn] {
+               tv[pn] = val
+               if !validConstraints(tv, constraints) {
+                       continue
+               }
+               score := countNewPairs(tv, paramNames, paramIdx, coveredPairs)
+               if score > bestScore {
+                       bestScore = score
+                       bestVal = val
+               }
+       }
+       if bestVal == "" {
+               // Fallback: pick first valid value
+               for _, val := range params[pn] {
+                       tv[pn] = val
+                       if validConstraints(tv, constraints) {
+                               bestVal = val
+                               break
+                       }
+               }
+               if bestVal == "" {
+                       tv[pn] = params[pn][0]
+               }
+       }
+       tv[pn] = bestVal
+       return fillVector(tv, params, paramNames, paramIdx+1, coveredPairs, 
constraints)

Review Comment:
   Fixed in a555985b: the last-resort fallback now assigns `bestVal = 
params[pn][0]` (instead of `tv[pn] = ...`), so the trailing `tv[pn] = bestVal` 
no longer overwrites it with the empty string.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to