-
Notifications
You must be signed in to change notification settings - Fork 269
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(common): LinkedChunk
emits an Update::NewItemsChunk
when constructed
#4327
fix(common): LinkedChunk
emits an Update::NewItemsChunk
when constructed
#4327
Conversation
…tructed. This patch updates `LinkedChunk::new_with_update_history` to emit an `Update::NewItemsChunk` because the first chunk is created and it must emit an update accordingly.
LinkedChunk
emits an Update::NewItemsChunk
when constuctedLinkedChunk
emits an Update::NewItemsChunk
when constructed
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4327 +/- ##
==========================================
+ Coverage 85.05% 85.11% +0.05%
==========================================
Files 275 275
Lines 30309 30315 +6
==========================================
+ Hits 25780 25802 +22
+ Misses 4529 4513 -16 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sweet, thanks!
#[test] | ||
fn test_push_items() { | ||
use super::Update::*; | ||
|
||
let mut linked_chunk = LinkedChunk::<3, char, ()>::new_with_update_history(); | ||
|
||
// Ignore initial update. | ||
let _ = linked_chunk.updates().unwrap().take(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we could also check (here and everywhere below) that the initial update is what we expected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a specific test just for that: it builds a LinkedChunk
and test the initial state. It helps to not repeat this for all tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the test_new_with_initial_update
test, just above this one.
This patch updates
LinkedChunk::new_with_update_history
to emit anUpdate::NewItemsChunk
because the first chunk is created and it must emit an update accordingly.The fix is 9 lines changes, the rest is only test updates.