search

matrix-org / sytest

Black-box integration testing for Matrix homeservers
Apache License 2.0
72 stars 55 forks source link

Presence tests relying on deprecated endpoints #1108

Closed S7evinK closed 2 years ago

S7evinK commented 3 years ago

A list of tests using deprecated endpoints related to presence. As simply deprecating those sounds like a bad idea, they should be updated to use /sync instead. (Or be deprecated and created new ones, whatever is the best solution.)

Using deprecated GET /r0/events:

tests/30rooms/02members-local.pl:test "Existing members see new members' presence"
tests/30rooms/03members-remote.pl:test "Existing members see new member's presence"
tests/40presence.pl:test "Presence changes are reported to local room members"
tests/40presence.pl:test "Presence changes are also reported to remote room members"
tests/40presence.pl:test "Presence changes to UNAVAILABLE are reported to local room members"
tests/40presence.pl:test "Presence changes to UNAVAILABLE are reported to remote room members"
tests/90jira/SYN-115.pl:multi_test "New federated private chats get full presence information (SYN-115)"

Using deprecated GET /r0/initialSync:

tests/40presence.pl:test "Newly created users see their own presence in /initialSync (SYT-34)"
tests/90jira/SYN-202.pl:multi_test "Left room members do not cause problems for presence"

Using deprecated POST /r0/presence/list/:user_id (#1101):

tests/21presence-events.pl:test "Friends presence changes reports events"
kegsay commented 3 years ago

Haven't forgotten about this, just been on holiday.

S7evinK commented 2 years ago

I'm currently unsure how to handle

tests/90jira/SYN-115.pl:multi_test "New federated private chats get full presence information (SYN-115)"
tests/90jira/SYN-202.pl:multi_test "Left room members do not cause problems for presence"

as they seem to be only related to Synapse. Deprecate, update or blacklist for Dendrite? The other tests are updated in #1148 and are passing for Synapse and Dendrite (with https://github.com/matrix-org/dendrite/pull/1879).

richvdh commented 2 years ago

as they seem to be only related to Synapse. Deprecate, update or blacklist for Dendrite?

can you explain why they only apply to Synapse?

S7evinK commented 2 years ago

you explain why they only apply to Synapse?

At least the names suggest they were created for specific Jira issues for Synapse and were specifically created for /initialSync and /events. If it doesn't matter whether any of them is used, I can update them to use /sync instead.

richvdh commented 2 years ago

we're talking about https://github.com/matrix-org/sytest/blob/develop/tests/90jira/SYN-115.pl and https://github.com/matrix-org/sytest/blob/develop/tests/90jira/SYN-202.pl, ftr.

for SYN-115: I don't see anything in there that makes me think it's testing behaviour specific to /events, so I think it would be great to update them to use /sync - thank you!

SYN-202.pl doesn't seem to do anything useful at all - I would just delete it.

(If the context is any use to you, you can see a bare-bones rendering of the relevant jira issues at https://matrix.org/jira/browse/SYN-115 and https://matrix.org/jira/browse/SYN-202.)

(Aside: I think the whole 90jira folder is a mistake - it would be much more sensible to file them alongside other tests for related functionality. Maybe we should move the SYN-115 test to a new file in https://github.com/matrix-org/sytest/tree/develop/tests/50federation.)

DMRobertson commented 2 years ago

Not sure if related, but I have just seen a test marked SYN-115 fail on synapse develop. Only happens on one of six sytest runs. Flake? https://github.com/matrix-org/synapse/runs/4067253219?check_suite_focus=true

richvdh commented 2 years ago

:tada: thanks for sorting this out, @S7evinK !

@DMRobertson I think you're suggesting that #1160 might have introduced flakiness into the SYN-115 test? Entirely plausible, but let's track it elsewhere?