Update some Beatmapset notification tests#13044
Conversation
switch to attributes; stricter receiverIds checking;
notbakaneko
force-pushed
the
feature/discussion-reply-test-update
branch
from
9a33ef3 to
b4cad66
Compare
| protected function receiversInclude( | ||
| NewPrivateNotificationEvent|BroadcastNotificationBase $obj, | ||
| array|Collection|Model $includes = [], | ||
| array|Collection|Model $excludes = [] |
There was a problem hiding this comment.
is there any point in checking this $excludes when the function is already checking that $obj receiver ids are exactly the same as $includes'? Seems rather redundant.
There was a problem hiding this comment.
and might as well sort($includeIds); sort($receiverIds); $this->assertSame($includeIds, $receiverIds);...
There was a problem hiding this comment.
It's for checking if something is specifically not included but we don't care about the rest; typically they're not both going to be used at the same time, although it should probably be a separate function atp 🤔
I originally wrote this to handle both and then discovered assert(Not)Pushed/Dispatched didn't quite work the way expected :|
There was a problem hiding this comment.
It's for checking if something is specifically not included but we don't care about the rest; typically they're not both going to be used at the same time, although it should probably be a separate function atp 🤔
that won't work here though if there's anything in the receiver ids with empty includes passed?
2 participants
Tests only update part of #13008 while I do more checking for splitting the notification options for #13017