Skip to content

Rewrote testing for the cache variant of the AutoRefresh() and AutoRefreshOnObservable() operators,...#1101

Open
JakenVeina wants to merge 2 commits into
mainfrom
enhancements/cache-auto-refresh-modernization
Open

Rewrote testing for the cache variant of the AutoRefresh() and AutoRefreshOnObservable() operators,...#1101
JakenVeina wants to merge 2 commits into
mainfrom
enhancements/cache-auto-refresh-modernization

Conversation

@JakenVeina
Copy link
Copy Markdown
Collaborator

…in accordance with #1014, and in a preliminary effort to resolve #1099.

…freshOnObservable() operators, in accordance with #1014, and in a preliminary effort to resolve #1099.
@JakenVeina JakenVeina force-pushed the enhancements/cache-auto-refresh-modernization branch from 3450986 to ada5983 Compare May 29, 2026 06:01
Copy link
Copy Markdown
Member

@dwcullop dwcullop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit but looks good to me.

item4.HasObservers.Should().BeTrue("adding an item should invoke its reevaluator and subscribe to it");
item1.HasObservers.Should().BeTrue("the item was not removed from the source");
item3.HasObservers.Should().BeTrue("the item was not removed from the source");
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test stops after asserting item4.HasObservers == true, but never verifies that the new reevaluator's notifications actually flow through end-to-end. A regression where the operator subscribes to the new item's reevaluator but mishandles the resulting notification (wrong key, dropped Refresh, etc.) would slip through. Consider adding ++item4.Value followed by an assertion that the next recorded changeset contains a Refresh for item4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: AutoRefreshOnObservable produces refreshes for immediately-removed items

2 participants