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

Fix muting API #116

Merged
merged 2 commits into from
Jan 6, 2024
Merged

Fix muting API #116

merged 2 commits into from
Jan 6, 2024

Conversation

S-S-X
Copy link
Member

@S-S-X S-S-X commented Jan 2, 2024

PLEASE DO NOT SQUASH, USE "REBASE AND MERGE" OR DO A FULL MERGE COMMIT. It has 2 commits which is intended, one for tests and other for actual code changes.

Some tests for /mute functionality.

  • Some tests for Some of the muting functionality and UX is broken #115

    I guess tests do now cover basics well enough.

  • Fix it

    Maybe... probably

  • Find out why tests for remote user fails (messages are not actually blocked in game)
    This is simply using API for check but actual message delivery from bridge isn't actually using API at all.
    Some remote tests might also be completely irrelevant for actual required functionality.

    API "shortcut" functions aren't actually utilized, instead it is direct beerchat.execute_callbacks(event, foo, bar)

Failures from test run:

Failure → spec/plugin_mute_spec.lua @ 72
mute plugin default behavior not muted from remote to local
spec/plugin_mute_spec.lua:73: [user] is muted by default

Failure → spec/plugin_mute_spec.lua @ 76
mute plugin default behavior allows private message from local to remote
spec/plugin_mute_spec.lua:77: [user] blocking pm by default

Failure → spec/plugin_mute_spec.lua @ 84
mute plugin default behavior does not mute remote self
spec/plugin_mute_spec.lua:85: [user] is muted by default

Failure → spec/plugin_mute_spec.lua @ 88
mute plugin default behavior allows pm from remote self
spec/plugin_mute_spec.lua:89: [user] blocking pm by default

Failure → spec/plugin_mute_spec.lua @ 98
mute plugin beerchat.has_player_muted_player /mute & /unmute player
spec/plugin_mute_spec.lua:101: Doe isn't muted but should be

Failure → spec/plugin_mute_spec.lua @ 120
mute plugin beerchat.has_player_muted_player /mute & /unmute both players
spec/plugin_mute_spec.lua:124: Doe isn't muted but should be

Failure → spec/plugin_mute_spec.lua @ 139
mute plugin beerchat.allow_private_message /mute & /unmute player
spec/plugin_mute_spec.lua:44: Sam blocking pm without mute

Failure → spec/plugin_mute_spec.lua @ 150
mute plugin beerchat.allow_private_message /mute & /unmute remote user
spec/plugin_mute_spec.lua:153: [user] blocking pm without mute

Failure → spec/plugin_mute_spec.lua @ 161
mute plugin beerchat.allow_private_message /mute & /unmute both players
spec/plugin_mute_spec.lua:165: Mute not blocking pm from Sam

Failure → spec/plugin_mute_spec.lua @ 180
mute plugin integrations whisper is muted & unmuted
spec/plugin_mute_spec.lua:187: Function was called with matching arguments at least once.
Called with (last matching call):
(values list) ((string) 'Sam', (string) '@#aaaaaa)<Doe> whispers: This is a pm mute test@#ffffff)')
Did not expect:
(values list) ((string) 'Sam', (matcher) is.matches((string) 'This is a pm mute test'))

Failure → spec/plugin_mute_spec.lua @ 197
mute plugin integrations pm is muted & unmuted
spec/plugin_mute_spec.lua:212: Function was never called with matching arguments.
Called with (last call if any):
(values list) ((string) 'Doe', (string) '[PM] sent to @(Dummy) This is a private message mute test')
(values list) ((string) 'Sam', (matcher) is.matches((string) 'This is a private message mute test'))

@S-S-X
Copy link
Member Author

S-S-X commented Jan 3, 2024

Tests for normal chatting are passing so it is just old API that is failing. And old API isn't really utilized that much internally.
Chat commands for mute are using old API which explains why this problem been unnoticed for some time: it only affects few rarely used chat commands.

Old API should be fixed anyway even if it is not currently used as it was supposed to serve as a shortcut / simple interface for the new event API. Also there's mods that are currently directly reading metadata / configuration, those should be updated to use API because configuration backend should also be updated to fix some other beerchat issues.

@S-S-X S-S-X marked this pull request as ready for review January 3, 2024 16:31
@S-S-X
Copy link
Member Author

S-S-X commented Jan 3, 2024

Hopefully fixed, at least way better test coverage.

PLEASE DO NOT SQUASH, USE "REBASE AND MERGE" OR DO A FULL MERGE COMMIT

LoC (excluding tests):

 common.lua      |  2 +-
 hooks.lua       |  7 +++++++
 plugin/mute.lua | 60 +++++++++++++++++++++++++++---------------------------------
 3 files changed, 35 insertions(+), 34 deletions(-)

Add test cases for default behavior

Full reset between test cases

Chatting tests for mute plugin

Test /unmute

Add generic chat command UX tests
@S-S-X
Copy link
Member Author

S-S-X commented Jan 3, 2024

Last addition (713dd2c -> 28631a2), few generic chat command tests for extra validation and even better coverage:

  • lists names with /list_muted
  • repeat /mute
  • repeat /unmute
  • empty /mute
  • empty /unmute

@S-S-X S-S-X changed the title Add some /mute tests Fix muting API Jan 3, 2024
@S-S-X S-S-X added the bug Something isn't working label Jan 3, 2024
@Athozus
Copy link
Member

Athozus commented Jan 4, 2024

Works fine with mail syncing scripts 👍 Also works with manual /mute, /unmute no longer the already muted just after unmuting.

@BuckarooBanzay BuckarooBanzay merged commit c27632d into master Jan 6, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants