fix: Assert fields exist in collection before saving to them #604
Conversation
AndrewSisley
added
bug
AndrewSisley
force-pushed
the
sisley/fix/I573-collection-field-validation
branch
from
def69ca
to
9750553
Compare
Codecov Report
@@ Coverage Diff @@
## develop #604 +/- ##
===========================================
+ Coverage 56.36% 56.41% +0.04%
===========================================
Files 121 121
Lines 14303 14316 +13
===========================================
+ Hits 8062 8076 +14
+ Misses 5543 5542 -1
Partials 698 698
|
db/collection.go
Outdated
| return core.DataStoreKey{ | ||
| CollectionId: key.CollectionId, | ||
| DocKey: key.DocKey, | ||
| FieldId: fmt.Sprint(c.getSchemaFieldID(fieldName)), | ||
| } | ||
| FieldId: fmt.Sprint(fieldId), |
There was a problem hiding this comment.
suggestion: I suggestion using something a little more efficient than Sprint.
| FieldId: fmt.Sprint(fieldId), | |
| FieldId: strconv.FormatUint(uint64(fieldId), 10), |
There was a problem hiding this comment.
Is a surprising performance gap between the two, and a bit odd fmt.Sprint doesnt take advantage of this. Will use strconv.Iota as that reads better. Thanks for flagging - is for sure good to know
https://developpaper.com/performance-test-of-several-methods-based-on-go-int-to-string/
- do it this way
There was a problem hiding this comment.
ah Iota only takes an int, which could technically overflow on a 32 bit machine - will use strconv.FormatUint as originally suggested. Really weird that Golang doesnt have a nicer way of doing this.
tests/integration/utils.go
Outdated
| @@ -78,6 +78,9 @@ type QueryTestCase struct { | |||
| // updates is a map from document index, to a list | |||
| // of changes in strinigied JSON format | |||
| Updates map[int]map[int][]string | |||
|
|
|||
| CollectionCalls map[int]func(client.Collection) error | |||
There was a problem hiding this comment.
suggestion: can we call this UpdateFuncs?
There was a problem hiding this comment.
Not a problem, will do
- Rename
Panic hides any expected errors in case results are still returned
AndrewSisley
force-pushed
the
sisley/fix/I573-collection-field-validation
branch
from
9750553
to
b591f94
Compare
Should allow us to assert collection-abi functionality within the main integration test suite
AndrewSisley
force-pushed
the
sisley/fix/I573-collection-field-validation
branch
from
b591f94
to
26ec210
Compare
…etwork#604) * Check expected length before accessing index Panic hides any expected errors in case results are still returned * Add support for collection abi calls in tests Should allow us to assert collection-abi functionality within the main integration test suite * Error if field does not exist
Relevant issue(s)
Resolves #573
Description
Asserts that fields exist in collection before saving to them. Issue made a lot easier by Shahzad spotting the
return 0in collection.go. Also adds support for testing the collection abi within our test framework - should hopefully make testing it much easier going forward.Tasks
Specify the platform(s) on which this was tested: