You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@devlake.apache.org by GitBox <gi...@apache.org> on 2022/10/20 10:40:15 UTC
[GitHub] [incubator-devlake] warren830 opened a new pull request, #3513: refactor(e2e): add e2e diversity
warren830 opened a new pull request, #3513:
URL: https://github.com/apache/incubator-devlake/pull/3513
# Summary
add e2e diversity check
1. will check raw table's `data`, `params` if they are the same
2. will check snapshot tables' target fields except pk fields if they are the same
### Does this close any open issues?
relate to #3182
### Screenshots
### Other Information
Any other information that is important to this PR.
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] warren830 commented on a diff in pull request #3513: refactor(e2e): add e2e diversity
Posted by "warren830 (via GitHub)" <gi...@apache.org>.
warren830 commented on code in PR #3513:
URL: https://github.com/apache/incubator-devlake/pull/3513#discussion_r1143289528
##########
backend/helpers/e2ehelper/data_flow_tester.go:
##########
@@ -506,6 +518,65 @@ func (t *DataFlowTester) VerifyTableWithOptions(dst schema.Tabler, opts TableOpt
assert.Equal(t.T, expected[field], formatDbValue(actual[field], opts.Nullable), fmt.Sprintf(`%s.%s not match (with params from csv %s)`, dst.TableName(), field, pkValues))
}
}
+ err = checkDiversity(dbRows, 3, opts.IgnoreDataDiversityCheckFields...)
+ if err != nil {
+ panic(err)
+ }
assert.Equal(t.T, expectedTotal, actualTotal, fmt.Sprintf(`%s count not match count,[expected:%d][actual:%d]`, dst.TableName(), expectedTotal, actualTotal))
}
+
+func checkDiversity(rows *[]map[string]interface{}, minUniqueValues int, ignoreFieldNames ...string) error {
+ // Check if the input rows is a non-nil pointer to a slice.
+ if rows == nil || reflect.ValueOf(rows).Kind() != reflect.Ptr || reflect.ValueOf(rows).Elem().Kind() != reflect.Slice {
+ return errors.Default.New("expected a non-nil pointer to a slice")
+ }
+
+ // Dereference the pointer to the slice of maps.
+ records := *rows
+
+ // Initialize a map to store unique values for each field.
+ fieldMaps := make(map[string]map[interface{}]bool)
+
+ // Iterate over the records.
+ for _, record := range records {
+ // Iterate over the fields of each record.
+ for field, value := range record {
+ // If ignoreFieldNames are provided, check if the current field is in the list, if true, skip it.
+ if ignoreFieldNames != nil && len(ignoreFieldNames) > 0 && contains(ignoreFieldNames, field) {
+ continue
+ }
+
+ // If the map for the current field doesn't exist, create one.
+ if fieldMaps[field] == nil {
+ fieldMaps[field] = make(map[interface{}]bool)
+ }
+ // Add the current field value to the map.
+ fieldMaps[field][value] = true
+ }
+ }
+
+ // Iterate over the maps for each field and check if the unique value count meets the requirement.
+ for filedName, fieldMap := range fieldMaps {
+ if len(fieldMap) < minUniqueValues {
+ valueList := make([]interface{}, len(fieldMap))
Review Comment:
Great! Thanks!
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] klesh commented on a diff in pull request #3513: refactor(e2e): add e2e diversity
Posted by GitBox <gi...@apache.org>.
klesh commented on code in PR #3513:
URL: https://github.com/apache/incubator-devlake/pull/3513#discussion_r1001524150
##########
helpers/e2ehelper/data_flow_tester.go:
##########
@@ -121,26 +121,30 @@ func (t *DataFlowTester) ImportCsvIntoRawTable(csvRelPath string, rawTableName s
csvIter := pluginhelper.NewCsvFileIterator(csvRelPath)
defer csvIter.Close()
t.FlushRawTable(rawTableName)
- // load rows and insert into target table
Review Comment:
Why delete the comment?
We need more instead of less
##########
helpers/e2ehelper/data_flow_tester.go:
##########
@@ -121,26 +121,30 @@ func (t *DataFlowTester) ImportCsvIntoRawTable(csvRelPath string, rawTableName s
csvIter := pluginhelper.NewCsvFileIterator(csvRelPath)
defer csvIter.Close()
t.FlushRawTable(rawTableName)
- // load rows and insert into target table
+ fc := fieldCheck{}
Review Comment:
How about `DataDiversityChecker`?
##########
helpers/e2ehelper/data_flow_tester.go:
##########
@@ -469,3 +484,48 @@ func (t *DataFlowTester) VerifyTableWithOptions(dst schema.Tabler, opts TableOpt
}
assert.Equal(t.T, expectedTotal, actualTotal, fmt.Sprintf(`%s count not match`, dst.TableName()))
}
+
+// fieldCheck will hold all fields and relevant values
+type fieldCheck map[string][]interface{}
+
+// feedInRawTable will add value of fields to a list, later we will use this list to judge diversity
+func (f fieldCheck) feedInRawTable(toInsertValues map[string]interface{}) {
+ for k, v := range toInsertValues {
+ if k == `created_at` || (k == `input` && v == `null`) || k == `url` {
+ continue
+ }
+ if f[k] == nil {
+ valueList := make([]interface{}, 0)
Review Comment:
I think using `map` directly would be more efficient.
##########
helpers/e2ehelper/data_flow_tester.go:
##########
@@ -469,3 +484,48 @@ func (t *DataFlowTester) VerifyTableWithOptions(dst schema.Tabler, opts TableOpt
}
assert.Equal(t.T, expectedTotal, actualTotal, fmt.Sprintf(`%s count not match`, dst.TableName()))
}
+
+// fieldCheck will hold all fields and relevant values
+type fieldCheck map[string][]interface{}
+
+// feedInRawTable will add value of fields to a list, later we will use this list to judge diversity
+func (f fieldCheck) feedInRawTable(toInsertValues map[string]interface{}) {
+ for k, v := range toInsertValues {
+ if k == `created_at` || (k == `input` && v == `null`) || k == `url` {
+ continue
+ }
+ if f[k] == nil {
+ valueList := make([]interface{}, 0)
Review Comment:
I don't really understand the algo here 😂
##########
helpers/e2ehelper/data_flow_tester.go:
##########
@@ -469,3 +484,48 @@ func (t *DataFlowTester) VerifyTableWithOptions(dst schema.Tabler, opts TableOpt
}
assert.Equal(t.T, expectedTotal, actualTotal, fmt.Sprintf(`%s count not match`, dst.TableName()))
}
+
+// fieldCheck will hold all fields and relevant values
+type fieldCheck map[string][]interface{}
+
+// feedInRawTable will add value of fields to a list, later we will use this list to judge diversity
+func (f fieldCheck) feedInRawTable(toInsertValues map[string]interface{}) {
+ for k, v := range toInsertValues {
+ if k == `created_at` || (k == `input` && v == `null`) || k == `url` {
+ continue
+ }
+ if f[k] == nil {
+ valueList := make([]interface{}, 0)
+ valueList = append(valueList, v)
+ f[k] = valueList
+ } else {
+ if v != f[k][0] {
+ f[k] = append(f[k], v)
+ }
+ }
+ }
+}
+
+// feedInSnapshotTable will add value of fields(only targetFields) to a list, later we will use this list to judge diversity
+func (f fieldCheck) feedInSnapshotTable(toInsertValues map[string]interface{}, targetFields map[string]bool) {
Review Comment:
I think `snapshots` are the output of a subtask, am I wrong?
##########
helpers/e2ehelper/data_flow_tester.go:
##########
@@ -121,26 +121,30 @@ func (t *DataFlowTester) ImportCsvIntoRawTable(csvRelPath string, rawTableName s
csvIter := pluginhelper.NewCsvFileIterator(csvRelPath)
defer csvIter.Close()
t.FlushRawTable(rawTableName)
- // load rows and insert into target table
+ fc := fieldCheck{}
Review Comment:
And we should unit test this Checker to make sure it would fail on low-diversity and pass on high-diversity.
##########
helpers/e2ehelper/data_flow_tester.go:
##########
@@ -419,17 +424,26 @@ func (t *DataFlowTester) VerifyTableWithOptions(dst schema.Tabler, opts TableOpt
}
targetFields := t.resolveTargetFields(dst, opts)
+ targetFieldMap := make(map[string]bool)
+ for _, v := range targetFields {
+ targetFieldMap[v] = true
+ }
pkColumns, err := dal.GetPrimarykeyColumns(t.Dal, dst)
if err != nil {
panic(err)
}
-
+ // will not check pk values as connection_id or board_id will be unique in snapshot tables
Review Comment:
If they would be all unique, and we are expecting them, why not check them to make sure of it? what is the harm?
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] mindlesscloud commented on a diff in pull request #3513: refactor(e2e): add e2e diversity
Posted by "mindlesscloud (via GitHub)" <gi...@apache.org>.
mindlesscloud commented on code in PR #3513:
URL: https://github.com/apache/incubator-devlake/pull/3513#discussion_r1143209694
##########
backend/helpers/e2ehelper/data_flow_tester.go:
##########
@@ -506,6 +518,65 @@ func (t *DataFlowTester) VerifyTableWithOptions(dst schema.Tabler, opts TableOpt
assert.Equal(t.T, expected[field], formatDbValue(actual[field], opts.Nullable), fmt.Sprintf(`%s.%s not match (with params from csv %s)`, dst.TableName(), field, pkValues))
}
}
+ err = checkDiversity(dbRows, 3, opts.IgnoreDataDiversityCheckFields...)
+ if err != nil {
+ panic(err)
+ }
assert.Equal(t.T, expectedTotal, actualTotal, fmt.Sprintf(`%s count not match count,[expected:%d][actual:%d]`, dst.TableName(), expectedTotal, actualTotal))
}
+
+func checkDiversity(rows *[]map[string]interface{}, minUniqueValues int, ignoreFieldNames ...string) error {
+ // Check if the input rows is a non-nil pointer to a slice.
+ if rows == nil || reflect.ValueOf(rows).Kind() != reflect.Ptr || reflect.ValueOf(rows).Elem().Kind() != reflect.Slice {
+ return errors.Default.New("expected a non-nil pointer to a slice")
+ }
+
+ // Dereference the pointer to the slice of maps.
+ records := *rows
+
+ // Initialize a map to store unique values for each field.
+ fieldMaps := make(map[string]map[interface{}]bool)
+
+ // Iterate over the records.
+ for _, record := range records {
+ // Iterate over the fields of each record.
+ for field, value := range record {
+ // If ignoreFieldNames are provided, check if the current field is in the list, if true, skip it.
+ if ignoreFieldNames != nil && len(ignoreFieldNames) > 0 && contains(ignoreFieldNames, field) {
+ continue
+ }
+
+ // If the map for the current field doesn't exist, create one.
+ if fieldMaps[field] == nil {
+ fieldMaps[field] = make(map[interface{}]bool)
+ }
+ // Add the current field value to the map.
+ fieldMaps[field][value] = true
+ }
+ }
+
+ // Iterate over the maps for each field and check if the unique value count meets the requirement.
+ for filedName, fieldMap := range fieldMaps {
+ if len(fieldMap) < minUniqueValues {
+ valueList := make([]interface{}, len(fieldMap))
Review Comment:
This is not what we want, try this:
```go
valueList := make([]interface{}, 0, len(fieldMap))
```
or just
```go
var valueList []interface{}
```
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] github-actions[bot] commented on pull request #3513: refactor(e2e): add e2e diversity
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3513:
URL: https://github.com/apache/incubator-devlake/pull/3513#issuecomment-1650736915
This pull request has been automatically marked as stale because it has not had recent activity for 120 days. It will be closed in 7 days if no further activity occurs.
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] likyh commented on a diff in pull request #3513: refactor(e2e): add e2e diversity
Posted by "likyh (via GitHub)" <gi...@apache.org>.
likyh commented on code in PR #3513:
URL: https://github.com/apache/incubator-devlake/pull/3513#discussion_r1142983670
##########
backend/helpers/e2ehelper/data_flow_tester.go:
##########
@@ -509,3 +523,54 @@ func (t *DataFlowTester) VerifyTableWithOptions(dst schema.Tabler, opts TableOpt
assert.Equal(t.T, expectedTotal, actualTotal, fmt.Sprintf(`%s count not match count,[expected:%d][actual:%d]`, dst.TableName(), expectedTotal, actualTotal))
}
+
+func checkDiversity(rows *[]map[string]interface{}, minUniqueValues int, fieldNames ...string) error {
+ // Check if the input rows is a non-nil pointer to a slice.
+ if rows == nil || reflect.ValueOf(rows).Kind() != reflect.Ptr || reflect.ValueOf(rows).Elem().Kind() != reflect.Slice {
+ return errors.Default.New("expected a non-nil pointer to a slice")
+ }
+
+ // Dereference the pointer to the slice of maps.
+ records := *rows
+
+ // Initialize a map to store unique values for each field.
+ fieldMaps := make(map[string]map[interface{}]bool)
+
+ // Iterate over the records.
+ for _, record := range records {
+ // Iterate over the fields of each record.
+ for field, value := range record {
+ // If specific field names are provided, check if the current field is in the list.
+ if len(fieldNames) > 0 && !contains(fieldNames, field) {
+ continue
+ }
+
+ // If the map for the current field doesn't exist, create one.
+ if fieldMaps[field] == nil {
+ fieldMaps[field] = make(map[interface{}]bool)
+ }
+ // Add the current field value to the map.
+ fieldMaps[field][value] = true
+ }
+ }
+
+ // Iterate over the maps for each field and check if the unique value count meets the requirement.
+ for _, fieldMap := range fieldMaps {
+ if len(fieldMap) < minUniqueValues {
+ return errors.Default.New("data diversity check failed")
Review Comment:
add more detail messages in this error. Such as which column, existing values...
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] mindlesscloud commented on a diff in pull request #3513: refactor(e2e): add e2e diversity
Posted by "mindlesscloud (via GitHub)" <gi...@apache.org>.
mindlesscloud commented on code in PR #3513:
URL: https://github.com/apache/incubator-devlake/pull/3513#discussion_r1143209694
##########
backend/helpers/e2ehelper/data_flow_tester.go:
##########
@@ -506,6 +518,65 @@ func (t *DataFlowTester) VerifyTableWithOptions(dst schema.Tabler, opts TableOpt
assert.Equal(t.T, expected[field], formatDbValue(actual[field], opts.Nullable), fmt.Sprintf(`%s.%s not match (with params from csv %s)`, dst.TableName(), field, pkValues))
}
}
+ err = checkDiversity(dbRows, 3, opts.IgnoreDataDiversityCheckFields...)
+ if err != nil {
+ panic(err)
+ }
assert.Equal(t.T, expectedTotal, actualTotal, fmt.Sprintf(`%s count not match count,[expected:%d][actual:%d]`, dst.TableName(), expectedTotal, actualTotal))
}
+
+func checkDiversity(rows *[]map[string]interface{}, minUniqueValues int, ignoreFieldNames ...string) error {
+ // Check if the input rows is a non-nil pointer to a slice.
+ if rows == nil || reflect.ValueOf(rows).Kind() != reflect.Ptr || reflect.ValueOf(rows).Elem().Kind() != reflect.Slice {
+ return errors.Default.New("expected a non-nil pointer to a slice")
+ }
+
+ // Dereference the pointer to the slice of maps.
+ records := *rows
+
+ // Initialize a map to store unique values for each field.
+ fieldMaps := make(map[string]map[interface{}]bool)
+
+ // Iterate over the records.
+ for _, record := range records {
+ // Iterate over the fields of each record.
+ for field, value := range record {
+ // If ignoreFieldNames are provided, check if the current field is in the list, if true, skip it.
+ if ignoreFieldNames != nil && len(ignoreFieldNames) > 0 && contains(ignoreFieldNames, field) {
+ continue
+ }
+
+ // If the map for the current field doesn't exist, create one.
+ if fieldMaps[field] == nil {
+ fieldMaps[field] = make(map[interface{}]bool)
+ }
+ // Add the current field value to the map.
+ fieldMaps[field][value] = true
+ }
+ }
+
+ // Iterate over the maps for each field and check if the unique value count meets the requirement.
+ for filedName, fieldMap := range fieldMaps {
+ if len(fieldMap) < minUniqueValues {
+ valueList := make([]interface{}, len(fieldMap))
Review Comment:
`valueList := make([]interface{}, 0, len(fieldMap))` or just `var valueList []interface{}`
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] github-actions[bot] commented on pull request #3513: refactor(e2e): add e2e diversity
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3513:
URL: https://github.com/apache/incubator-devlake/pull/3513#issuecomment-1666650725
This pull request has been closed because it has not had recent activity. You could reopen it if you try to continue your work, and anyone who are interested in it are encouraged to continue work on this pull request.
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] warren830 commented on a diff in pull request #3513: refactor(e2e): add e2e diversity
Posted by GitBox <gi...@apache.org>.
warren830 commented on code in PR #3513:
URL: https://github.com/apache/incubator-devlake/pull/3513#discussion_r1001560610
##########
helpers/e2ehelper/data_flow_tester.go:
##########
@@ -121,26 +121,30 @@ func (t *DataFlowTester) ImportCsvIntoRawTable(csvRelPath string, rawTableName s
csvIter := pluginhelper.NewCsvFileIterator(csvRelPath)
defer csvIter.Close()
t.FlushRawTable(rawTableName)
- // load rows and insert into target table
Review Comment:
fixed
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] warren830 commented on a diff in pull request #3513: refactor(e2e): add e2e diversity
Posted by GitBox <gi...@apache.org>.
warren830 commented on code in PR #3513:
URL: https://github.com/apache/incubator-devlake/pull/3513#discussion_r1001562519
##########
helpers/e2ehelper/data_flow_tester.go:
##########
@@ -121,26 +121,30 @@ func (t *DataFlowTester) ImportCsvIntoRawTable(csvRelPath string, rawTableName s
csvIter := pluginhelper.NewCsvFileIterator(csvRelPath)
defer csvIter.Close()
t.FlushRawTable(rawTableName)
- // load rows and insert into target table
+ fc := fieldCheck{}
Review Comment:
> How about `DataDiversityChecker`?
fixed
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] warren830 commented on a diff in pull request #3513: refactor(e2e): add e2e diversity
Posted by GitBox <gi...@apache.org>.
warren830 commented on code in PR #3513:
URL: https://github.com/apache/incubator-devlake/pull/3513#discussion_r1001562313
##########
helpers/e2ehelper/data_flow_tester.go:
##########
@@ -469,3 +484,48 @@ func (t *DataFlowTester) VerifyTableWithOptions(dst schema.Tabler, opts TableOpt
}
assert.Equal(t.T, expectedTotal, actualTotal, fmt.Sprintf(`%s count not match`, dst.TableName()))
}
+
+// fieldCheck will hold all fields and relevant values
+type fieldCheck map[string][]interface{}
+
+// feedInRawTable will add value of fields to a list, later we will use this list to judge diversity
+func (f fieldCheck) feedInRawTable(toInsertValues map[string]interface{}) {
+ for k, v := range toInsertValues {
+ if k == `created_at` || (k == `input` && v == `null`) || k == `url` {
+ continue
+ }
+ if f[k] == nil {
+ valueList := make([]interface{}, 0)
+ valueList = append(valueList, v)
+ f[k] = valueList
+ } else {
+ if v != f[k][0] {
+ f[k] = append(f[k], v)
+ }
+ }
+ }
+}
+
+// feedInSnapshotTable will add value of fields(only targetFields) to a list, later we will use this list to judge diversity
+func (f fieldCheck) feedInSnapshotTable(toInsertValues map[string]interface{}, targetFields map[string]bool) {
Review Comment:
no, you are correct, here we are putting data of snapshot table into `checker`
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] github-actions[bot] closed pull request #3513: refactor(e2e): add e2e diversity
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #3513: refactor(e2e): add e2e diversity
URL: https://github.com/apache/incubator-devlake/pull/3513
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] github-actions[bot] commented on pull request #3513: refactor(e2e): add e2e diversity
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3513:
URL: https://github.com/apache/incubator-devlake/pull/3513#issuecomment-1435799055
This pull request has been automatically marked as stale because it has not had recent activity for 120 days. It will be closed in 7 days if no further activity occurs.
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] warren830 commented on a diff in pull request #3513: refactor(e2e): add e2e diversity
Posted by GitBox <gi...@apache.org>.
warren830 commented on code in PR #3513:
URL: https://github.com/apache/incubator-devlake/pull/3513#discussion_r1001565672
##########
helpers/e2ehelper/data_flow_tester.go:
##########
@@ -419,17 +424,26 @@ func (t *DataFlowTester) VerifyTableWithOptions(dst schema.Tabler, opts TableOpt
}
targetFields := t.resolveTargetFields(dst, opts)
+ targetFieldMap := make(map[string]bool)
+ for _, v := range targetFields {
+ targetFieldMap[v] = true
+ }
pkColumns, err := dal.GetPrimarykeyColumns(t.Dal, dst)
if err != nil {
panic(err)
}
-
+ // will not check pk values as connection_id or board_id will be unique in snapshot tables
Review Comment:
we have several pk columns, some will be diversity, some should be unique
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] warren830 commented on a diff in pull request #3513: refactor(e2e): add e2e diversity
Posted by "warren830 (via GitHub)" <gi...@apache.org>.
warren830 commented on code in PR #3513:
URL: https://github.com/apache/incubator-devlake/pull/3513#discussion_r1143289012
##########
backend/helpers/e2ehelper/data_flow_tester_test.go:
##########
@@ -208,3 +208,70 @@ func TestGetTableMetaData(t *testing.T) {
}
})
}
+
+func TestCheckDiversity(t *testing.T) {
+ // Define test cases using a struct that contains input values and expected output.
+ testCases := []struct {
+ name string
+ rows *[]map[string]interface{}
+ minUniqueValues int
+ fieldNames []string
Review Comment:
fixed
##########
backend/helpers/e2ehelper/data_flow_tester.go:
##########
@@ -491,6 +501,10 @@ func (t *DataFlowTester) VerifyTableWithOptions(dst schema.Tabler, opts TableOpt
if err != nil {
panic(err)
}
+ err = checkDiversity(dbRows, 3)
Review Comment:
fixed
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] likyh commented on a diff in pull request #3513: refactor(e2e): add e2e diversity
Posted by "likyh (via GitHub)" <gi...@apache.org>.
likyh commented on code in PR #3513:
URL: https://github.com/apache/incubator-devlake/pull/3513#discussion_r1143036142
##########
backend/helpers/e2ehelper/data_flow_tester_test.go:
##########
@@ -208,3 +208,70 @@ func TestGetTableMetaData(t *testing.T) {
}
})
}
+
+func TestCheckDiversity(t *testing.T) {
+ // Define test cases using a struct that contains input values and expected output.
+ testCases := []struct {
+ name string
+ rows *[]map[string]interface{}
+ minUniqueValues int
+ fieldNames []string
Review Comment:
change to ignoreFieldNames?
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] warren830 commented on a diff in pull request #3513: refactor(e2e): add e2e diversity
Posted by "warren830 (via GitHub)" <gi...@apache.org>.
warren830 commented on code in PR #3513:
URL: https://github.com/apache/incubator-devlake/pull/3513#discussion_r1144157740
##########
backend/helpers/e2ehelper/data_flow_tester.go:
##########
@@ -506,6 +518,65 @@ func (t *DataFlowTester) VerifyTableWithOptions(dst schema.Tabler, opts TableOpt
assert.Equal(t.T, expected[field], formatDbValue(actual[field], opts.Nullable), fmt.Sprintf(`%s.%s not match (with params from csv %s)`, dst.TableName(), field, pkValues))
}
}
+ err = checkDiversity(dbRows, 3, opts.IgnoreDataDiversityCheckFields...)
+ if err != nil {
+ panic(err)
+ }
assert.Equal(t.T, expectedTotal, actualTotal, fmt.Sprintf(`%s count not match count,[expected:%d][actual:%d]`, dst.TableName(), expectedTotal, actualTotal))
}
+
+func checkDiversity(rows *[]map[string]interface{}, minUniqueValues int, ignoreFieldNames ...string) error {
+ // Check if the input rows is a non-nil pointer to a slice.
+ if rows == nil || reflect.ValueOf(rows).Kind() != reflect.Ptr || reflect.ValueOf(rows).Elem().Kind() != reflect.Slice {
+ return errors.Default.New("expected a non-nil pointer to a slice")
+ }
+
+ // Dereference the pointer to the slice of maps.
+ records := *rows
+
+ // Initialize a map to store unique values for each field.
+ fieldMaps := make(map[string]map[interface{}]bool)
+
+ // Iterate over the records.
+ for _, record := range records {
+ // Iterate over the fields of each record.
+ for field, value := range record {
+ // If ignoreFieldNames are provided, check if the current field is in the list, if true, skip it.
+ if ignoreFieldNames != nil && len(ignoreFieldNames) > 0 && contains(ignoreFieldNames, field) {
+ continue
+ }
+
+ // If the map for the current field doesn't exist, create one.
+ if fieldMaps[field] == nil {
+ fieldMaps[field] = make(map[interface{}]bool)
+ }
+ // Add the current field value to the map.
+ fieldMaps[field][value] = true
+ }
+ }
+
+ // Iterate over the maps for each field and check if the unique value count meets the requirement.
+ for filedName, fieldMap := range fieldMaps {
+ if len(fieldMap) < minUniqueValues {
+ valueList := make([]interface{}, len(fieldMap))
+ for k, _ := range fieldMap {
Review Comment:
fixed
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] mindlesscloud commented on a diff in pull request #3513: refactor(e2e): add e2e diversity
Posted by "mindlesscloud (via GitHub)" <gi...@apache.org>.
mindlesscloud commented on code in PR #3513:
URL: https://github.com/apache/incubator-devlake/pull/3513#discussion_r1143212007
##########
backend/helpers/e2ehelper/data_flow_tester.go:
##########
@@ -506,6 +518,65 @@ func (t *DataFlowTester) VerifyTableWithOptions(dst schema.Tabler, opts TableOpt
assert.Equal(t.T, expected[field], formatDbValue(actual[field], opts.Nullable), fmt.Sprintf(`%s.%s not match (with params from csv %s)`, dst.TableName(), field, pkValues))
}
}
+ err = checkDiversity(dbRows, 3, opts.IgnoreDataDiversityCheckFields...)
+ if err != nil {
+ panic(err)
+ }
assert.Equal(t.T, expectedTotal, actualTotal, fmt.Sprintf(`%s count not match count,[expected:%d][actual:%d]`, dst.TableName(), expectedTotal, actualTotal))
}
+
+func checkDiversity(rows *[]map[string]interface{}, minUniqueValues int, ignoreFieldNames ...string) error {
+ // Check if the input rows is a non-nil pointer to a slice.
+ if rows == nil || reflect.ValueOf(rows).Kind() != reflect.Ptr || reflect.ValueOf(rows).Elem().Kind() != reflect.Slice {
+ return errors.Default.New("expected a non-nil pointer to a slice")
+ }
+
+ // Dereference the pointer to the slice of maps.
+ records := *rows
+
+ // Initialize a map to store unique values for each field.
+ fieldMaps := make(map[string]map[interface{}]bool)
+
+ // Iterate over the records.
+ for _, record := range records {
+ // Iterate over the fields of each record.
+ for field, value := range record {
+ // If ignoreFieldNames are provided, check if the current field is in the list, if true, skip it.
+ if ignoreFieldNames != nil && len(ignoreFieldNames) > 0 && contains(ignoreFieldNames, field) {
+ continue
+ }
+
+ // If the map for the current field doesn't exist, create one.
+ if fieldMaps[field] == nil {
+ fieldMaps[field] = make(map[interface{}]bool)
+ }
+ // Add the current field value to the map.
+ fieldMaps[field][value] = true
+ }
+ }
+
+ // Iterate over the maps for each field and check if the unique value count meets the requirement.
+ for filedName, fieldMap := range fieldMaps {
+ if len(fieldMap) < minUniqueValues {
+ valueList := make([]interface{}, len(fieldMap))
+ for k, _ := range fieldMap {
Review Comment:
```go
for k := range fieldMap {
```
would be fine
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] likyh commented on a diff in pull request #3513: refactor(e2e): add e2e diversity
Posted by "likyh (via GitHub)" <gi...@apache.org>.
likyh commented on code in PR #3513:
URL: https://github.com/apache/incubator-devlake/pull/3513#discussion_r1142985373
##########
backend/helpers/e2ehelper/data_flow_tester.go:
##########
@@ -491,6 +501,10 @@ func (t *DataFlowTester) VerifyTableWithOptions(dst schema.Tabler, opts TableOpt
if err != nil {
panic(err)
}
+ err = checkDiversity(dbRows, 3)
Review Comment:
check diversity after the value checker. Because maybe there are some developers who want to add new data after the prev test passed.
--
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: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org