chore: add test assertion missing from #26691#27276
chore: add test assertion missing from #26691#27276gwossum wants to merge 1 commit intomaster-1.xfrom
Conversation
Add test assertion missing from #26691 when it was ported to OSS master-1.x.
There was a problem hiding this comment.
Pull request overview
Adds a missing assertion to the existing regression test for the ascending array cursor optimization bug (duplicate output when cache and TSM data interact in a specific way), aligning OSS master-1.x with the original fix work from #26691.
Changes:
- Add a test assertion to confirm the total number of returned values matches the total number of returned timestamps.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| // Verify no duplicates were found | ||
| assert.Equalf(len(seenTimestamps), len(allTimestamps), "Found duplicate timestamps! Total: %d, Unique: %d", len(allTimestamps), len(seenTimestamps)) | ||
| // Confirm equal length of values and timestamps | ||
| assert.Equal(len(allValues), len(allTimestamps), "quantity of timestamps and values count mismatch: %d timestamps, %d values", len(allTimestamps), len(allValues)) |
There was a problem hiding this comment.
This uses assert.Equal with a format string and %d placeholders; Equal doesn’t apply formatting to msgAndArgs, so the failure message will be confusing. Use assert.Equalf (or build the message with fmt.Sprintf) and consider ordering the args as expected=len(allTimestamps), actual=len(allValues) to match the message text.
| assert.Equal(len(allValues), len(allTimestamps), "quantity of timestamps and values count mismatch: %d timestamps, %d values", len(allTimestamps), len(allValues)) | |
| assert.Equalf(len(allTimestamps), len(allValues), "quantity of timestamps and values count mismatch: %d timestamps, %d values", len(allTimestamps), len(allValues)) |
Add test assertion missing from #26691 when it was ported to OSS master-1.x.