Skip to content

Add tests for server notices#322

Merged
S7evinK merged 7 commits intomainfrom
s7evink/server-notices
Feb 18, 2022
Merged

Add tests for server notices#322
S7evinK merged 7 commits intomainfrom
s7evink/server-notices

Conversation

@S7evinK
Copy link
Contributor

@S7evinK S7evinK commented Feb 18, 2022

Copy link
Member

@kegsay kegsay left a comment

Choose a reason for hiding this comment

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

Excellent battery of tests!

return gjson.GetBytes(body, "event_id").Str
}

func syncUntilInvite(t *testing.T, alice *client.CSAPI) string {
Copy link
Member

Choose a reason for hiding this comment

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

Needs a comment explaining why you can't use client.SyncInvitedTo - you don't know the room ID!

syncUntilInvite(t, alice)
})
t.Run("Alice cannot reject the invite", func(t *testing.T) {
sendServerNotice(t, admin, reqBody, nil)
Copy link
Member

Choose a reason for hiding this comment

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

Why are we constantly sending server notices? These subtests will run sequentially.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was running them in parallel first, but remembered that this might race since there can only be one active notice room. Fixed.

@S7evinK S7evinK merged commit 53e90f7 into main Feb 18, 2022
@S7evinK S7evinK deleted the s7evink/server-notices branch February 18, 2022 15:05
MadLittleMods added a commit to element-hq/synapse that referenced this pull request Feb 5, 2026
This is useful so we can test Synapse
specific behaviors like our admin API.

(see docs in PR, `complement/README.md`)
```
COMPLEMENT_DIR=../complement ./scripts-dev/complement.sh --in-repo
```

Complement calls these
["out-of-repo"](https://github.com/matrix-org/complement/blob/78c255edcebfcb0ac5e3c86d49d76cb21fdd035a/OUT-OF-REPO-TESTS.md)
tests but it's a bit of a misnomer once they're in your project. (just
depends on the perspective)

There has been [previous
desire](#19021 (comment))
for this kind of thing but this is spawning from wanting to have some
tests for our purge history admin API
(element-hq/synapse-rust-apps#430). There are
some Sytest tests ([`matrix-org/sytest` ->
`tests/48admin.pl#L91-L618`](https://github.com/matrix-org/sytest/blob/1be04cce46c0f84abac736390dc3fab17f35a756/tests/48admin.pl#L91-L618))
for this already but I'd much rather work in Complement instead of
Sytest. I'm wanting these tests to ensure that our new `event-cache`
rust app for Synapse Pro doesn't break these kind of erasure features
(element-hq/synapse-rust-apps#366 and
element-hq/synapse-rust-apps#153).

Interestingly, there is already [`matrix-org/complement` ->
`tests/csapi/admin_test.go`](https://github.com/matrix-org/complement/blob/78c255edcebfcb0ac5e3c86d49d76cb21fdd035a/tests/csapi/admin_test.go)
(added in matrix-org/complement#322) in the
Complement repo iteslf that tests the
`/_synapse/admin/v1/send_server_notice` endpoint but it's a bit of an
interesting case as [Dendrite also supports this
endpoint](matrix-org/dendrite#2180). I don't
think it's good practice to continually shove in more and more
Synapse-specific behavior into the Complement repo itself.

We already have success with other out-of-repo tests for projects like
the
[SBG](https://github.com/element-hq/sbg/tree/b76b05b53e40bf6890e51dd1b83cec3460274eb2/complement_tests),
[TI-Messenger
Proxy](https://github.com/element-hq/ti-messenger-proxy/tree/c8fa87feccc743c01cccbbc2685321206b532925/complement),
and our [Synapse Pro for small
hosts](https://github.com/element-hq/synapse-small-hosts/tree/c2ea7eabf3e1d7c26a5312ebef326b254937be99/complement).
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