fix: Deterministic ordering for records returned by readAll in tests #1072
Conversation
yevgenypats
left a comment
yevgenypats
left a comment
There was a problem hiding this comment.
Looks good but have one comment on the design of the read funciton/sorting.
plugin/plugin_read.go
Outdated
| // sort records by "id" column, if present. Because "id" is auto-incrementing in the test | ||
| // data generator, this should result in records being returned in insertion order. | ||
| sch := table.ToArrowSchema() | ||
| if sch.HasField("id") { |
There was a problem hiding this comment.
I think this should be done outside of read with a helper function because we might want to sort it with different logic and then it will either be impossible or this function would grow.
There was a problem hiding this comment.
The vanilla read should just return it as it is as it is not aware of what columns are there and so on.
There was a problem hiding this comment.
This is a private function, I figured we can refactor it later if/when the need arises. But okay, I can do it now
There was a problem hiding this comment.
Refactored 👍 I also added some random shuffling to the memdb Read function so that we can be sure this is used everywhere it needs to be used
…k into fix-tests-some-more
🤖 I have created a release *beep* *boop* --- ## [4.8.0-rc1](v4.7.1-rc1...v4.8.0-rc1) (2023-07-05) ### Features * **transformers:** Add `Apply` to apply extra transformations ([#1069](#1069)) ([a40598e](a40598e)) ### Bug Fixes * Deterministic ordering for records returned by readAll in tests ([#1072](#1072)) ([cf7510f](cf7510f)) * Handle null-related test options ([#1074](#1074)) ([88f08ee](88f08ee)) * **naming:** Rename `SyncMessages.InsertMessage()` to `SyncMessages.GetInserts()` ([#1070](#1070)) ([ab9e768](ab9e768)) * Reset timers on flush ([#1076](#1076)) ([767327f](767327f)) * Reverse order of records in memdb ([#1075](#1075)) ([8356590](8356590)) * **scalar:** Test `AppendTime` on TimestampBuilder ([#1068](#1068)) ([888c9ee](888c9ee)) * **testdata:** Exclude only the correct type ([#1067](#1067)) ([1c72fb2](1c72fb2)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This:
idfield that is used as an auto-incrementing integer field in tests, and can be used to order records by so that tests can pass regardless of sort order returned by a plugin's Read method