Skip to content
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

test: rename some MatrixMockServer matcher functions #4701

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bnjbvr
Copy link
Member

@bnjbvr bnjbvr commented Feb 20, 2025

The .from(), .with_delay() and .limit() functions are not very explicit about what they did, and the with_delay one in particular led me to think that it would introduce a delay in the response, while it indicated a delay was expected as part of the matched URL.

Instead, this patch proposes to prefix all matchers with match_, to make it clearer that… they will match the incoming query: match_from, match_delay, match_limit.

Thanks to this change, the RoomMessagesResponseTemplate can be renamed from delayed to with_delay, which was my original intent, before I noticed another with_delay with totally different semantics.

@bnjbvr bnjbvr requested a review from a team as a code owner February 20, 2025 14:58
@bnjbvr bnjbvr requested review from poljar and removed request for a team February 20, 2025 14:58
@bnjbvr bnjbvr force-pushed the bnjbvr/rename-matrix-mock-server-helpers branch from 73e038d to 2f320fa Compare February 20, 2025 15:17
@@ -1387,7 +1387,7 @@ impl<'a> MockEndpoint<'a, RoomSendEndpoint> {
/// assert_eq!("$some_id", response.delay_id);
/// # anyhow::Ok(()) });
/// ```
pub fn with_delay(self, delay: Duration) -> Self {
pub fn match_delay(self, delay: Duration) -> Self {
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps match_delayted_event() would be an even better name, also the docs should really link to the MSC.

@@ -1615,7 +1615,7 @@ impl<'a> MockEndpoint<'a, RoomSendStateEndpoint> {
///
/// # anyhow::Ok(()) });
/// ```
pub fn with_delay(self, delay: Duration) -> Self {
pub fn match_delay(self, delay: Duration) -> Self {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same argument here.

Copy link

codecov bot commented Feb 20, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.90%. Comparing base (61fa339) to head (20e6bc9).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4701   +/-   ##
=======================================
  Coverage   85.90%   85.90%           
=======================================
  Files         292      292           
  Lines       33907    33907           
=======================================
  Hits        29127    29127           
  Misses       4780     4780           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bnjbvr bnjbvr force-pushed the bnjbvr/rename-matrix-mock-server-helpers branch from 2f320fa to f503321 Compare February 20, 2025 16:16
@bnjbvr bnjbvr enabled auto-merge (rebase) February 20, 2025 16:16
… they're matchers

The `.from()`, `.with_delay()` and `.limit()` functions are not very
explicit about what they did, and the `with_delay` one in particular led
me to think that it would introduce a delay *in the response*, while it
indicated a delay was expected as part of the matched URL.

Instead, this patch proposes to prefix all matchers with `match_`, to
make it clearer that… they will match the incoming query: match_from,
match_delay, match_limit.

Thanks to this change, the `RoomMessagesResponseTemplate` can be renamed
from `delayed` to `with_delay`, which was my original intent, before I
noticed another `with_delay` with totally different semantics.
@bnjbvr bnjbvr force-pushed the bnjbvr/rename-matrix-mock-server-helpers branch from f503321 to 20e6bc9 Compare February 20, 2025 16:17
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.

2 participants