Skip to content

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

Merged
JakenVeina merged 1 commit into
mainfrom
enhancements/cache-auto-refresh-modernization
May 30, 2026
Merged

Rewrote testing for the cache variant of the AutoRefresh() and AutoRefreshOnObservable() operators,...#1101
JakenVeina merged 1 commit 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.

@JakenVeina
JakenVeina force-pushed the enhancements/cache-auto-refresh-modernization branch from 3450986 to ada5983 Compare May 29, 2026 06:01

@dwcullop dwcullop left a comment

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.

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.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Agreed.

@JakenVeina
JakenVeina force-pushed the enhancements/cache-auto-refresh-modernization branch from 67ee820 to e57d6d3 Compare May 30, 2026 00:25
…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 e57d6d3 to ef204b5 Compare May 30, 2026 00:26
@JakenVeina
JakenVeina merged commit 3de2fad into main May 30, 2026
2 checks passed
@JakenVeina
JakenVeina deleted the enhancements/cache-auto-refresh-modernization branch May 30, 2026 01:03
@github-actions

Copy link
Copy Markdown

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Jun 13, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

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