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

Add tests for clients sending m.room.power_levels events #146

Closed
wants to merge 1 commit into from

Conversation

aaronraimist
Copy link

Signed-off-by: Aaron Raimist <aaron@raim.ist>
package b

// BlueprintAliceAndBob is a two user homeserver
var BlueprintAliceAndBob = MustValidate(Blueprint{
Copy link
Member

Choose a reason for hiding this comment

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

}
for _, jm := range m.JSON {
if err = jm(body); err != nil {
t.Fatalf("MatchResponse %s - %s", err, contextStr)
t.Errorf("MatchResponse %s - %s", err, contextStr)
Copy link
Member

@kegsay kegsay Jul 5, 2021

Choose a reason for hiding this comment

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

Please do not do this. It is intended for test assertions in the must package to fail the test immediately and not to continue executing.

@@ -127,6 +122,18 @@ func (c *CSAPI) SendEventSynced(t *testing.T, roomID string, e b.Event) string {
return eventID
}

// SendEventSynced sends `e` into the room.
func (c *CSAPI) SendEvent(t *testing.T, roomID string, e b.Event) *http.Response {
Copy link
Member

Choose a reason for hiding this comment

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

Please move this into a helper function inside tests/client_power_levels_test.go and not inside the Complement CSAPI client struct.

The reason for this is because CSAPI should be reserved for functions which are commonly used across many tests, and this function is primarily only useful to check error conditions when sending events. In addition, by having both forms sit alongside each other in CSAPI, it confuses test authors who may prefer to use this function over SendEventSynced which would end up being disasterous as SendEventSynced is the only form which guards against race conditions when processing new events.

@kegsay kegsay added the stale This issue or PR is old and may be closed soon label Oct 26, 2021
@kegsay kegsay closed this Nov 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale This issue or PR is old and may be closed soon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants