Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ update-ca-certificates

## Sytest parity

As of 10 February 2023:
As of 29 October 2025:
```
$ go build ./cmd/sytest-coverage
$ ./sytest-coverage -v
Expand Down Expand Up @@ -507,7 +507,13 @@ $ ./sytest-coverage -v
✓ Can get rooms/{roomId}/members

30rooms/60version_upgrade 0/19 tests
30rooms/70publicroomslist 0/5 tests
30rooms/70publicroomslist 2/5 tests
× Asking for a remote rooms list, but supplying the local server's name, returns the local rooms list
× Can get remote public room list
× Can paginate public room list
✓ Can search public room list
✓ Name/topic keys are correct

31sync/01filter 2/2 tests
✓ Can create filter
✓ Can download filter
Expand Down Expand Up @@ -707,5 +713,5 @@ $ ./sytest-coverage -v
90jira/SYN-516 0/1 tests
90jira/SYN-627 0/1 tests

TOTAL: 220/610 tests converted
TOTAL: 222/610 tests converted
```
195 changes: 180 additions & 15 deletions tests/csapi/public_rooms_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package csapi_tests

import (
"fmt"
"net/http"
"testing"
"time"
Expand All @@ -12,11 +13,13 @@ import (
"github.com/matrix-org/complement/helpers"
"github.com/matrix-org/complement/match"
"github.com/matrix-org/complement/must"
"github.com/matrix-org/complement/should"
)

func TestPublicRooms(t *testing.T) {
deployment := complement.Deploy(t, 1)
defer deployment.Destroy(t)
hostname := deployment.GetFullyQualifiedHomeserverName(t, "hs1")
Copy link
Collaborator

@MadLittleMods MadLittleMods Oct 30, 2025

Choose a reason for hiding this comment

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

This isn't just a hostname. It's the "server name" of the homeserver.

(could use a better variable name)


t.Run("parallel", func(t *testing.T) {
// sytest: Can search public room list
Expand All @@ -40,22 +43,8 @@ func TestPublicRooms(t *testing.T) {
},
}),
client.WithRetryUntil(15*time.Second, func(res *http.Response) bool {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
client.WithRetryUntil(15*time.Second, func(res *http.Response) bool {
client.WithRetryUntil(authedClient.SyncUntilTimeout, func(res *http.Response) bool {

body := must.ParseJSON(t, res.Body)
results := parsePublicRoomsResponse(t, res)

must.MatchGJSON(
t,
body,
match.JSONKeyPresent("chunk"),
match.JSONKeyTypeEqual("chunk", gjson.JSON),
)

chunk := body.Get("chunk")
if !chunk.IsArray() {
t.Logf("chunk is not an array")
return false
}

results := chunk.Array()
if len(results) != 1 {
t.Logf("expected a single search result, got %d", len(results))
return false
Expand All @@ -71,5 +60,181 @@ func TestPublicRooms(t *testing.T) {
}),
)
})

// sytest: Name/topic keys are correct
t.Run("Name/topic keys are correct", func(t *testing.T) {
t.Parallel()
Comment on lines +65 to +66
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think either of these tests are resilient to being run in parallel.

The first test assumes there will be one and only room in the results.

This test keeps track of unexpectedRooms. These other rooms are not unexpected if the tests are running in parallel. They are not the expected room though.


Could be resolved a few different ways like making a new deployment for each test.

authedClient := deployment.Register(t, "hs1", helpers.RegistrationOpts{})

// Define room configurations
roomConfigs := []struct {
alias string
name string
topic string
}{
{"publicroomalias_no_name", "", ""},
{"publicroomalias_with_name", "name_1", ""},
{"publicroomalias_with_topic", "", "topic_1"},
{"publicroomalias_with_name_topic", "name_2", "topic_2"},
{"publicroom_with_unicode_chars_name", "un nom français", ""},
{"publicroom_with_unicode_chars_topic", "", "un topic à la française"},
{"publicroom_with_unicode_chars_name_topic", "un nom français", "un topic à la française"},
}

for _, roomConfig := range roomConfigs {
t.Run(fmt.Sprintf("Creating room with alias %s", roomConfig.alias), func(t *testing.T) {
expectedCanonicalAlias := fmt.Sprintf("#%s:%s", roomConfig.alias, hostname)

// Create the room
roomOptions := map[string]interface{}{
// Add the room to the public rooms list.
"visibility": "public",
"room_alias_name": roomConfig.alias,
}

if roomConfig.name != "" {
roomOptions["name"] = roomConfig.name
}
if roomConfig.topic != "" {
roomOptions["topic"] = roomConfig.topic
}

roomID := authedClient.MustCreateRoom(t, roomOptions)
t.Logf("Created room %s with alias %s", roomID, roomConfig.alias)

// Poll /publicRooms until the room appears with the correct data

// Keep track of any rooms that we didn't expect to see.
unexpectedRooms := make([]string, 0)

var discoveredRoomData gjson.Result
authedClient.MustDo(t, "GET", []string{"_matrix", "client", "v3", "publicRooms"},
client.WithRetryUntil(authedClient.SyncUntilTimeout, func(res *http.Response) bool {
results := parsePublicRoomsResponse(t, res)

// Check each room in the public rooms list
for _, roomData := range results {
discoveredRoomID := roomData.Get("room_id").Str
if discoveredRoomID != roomID {
// Not our room, skip.
unexpectedRooms = append(unexpectedRooms, discoveredRoomID)
continue
}

// We found our room. Stop calling /publicRooms and validate the response.
discoveredRoomData = roomData
}

if !discoveredRoomData.Exists() {
t.Logf("Room %s not found in public rooms list, trying again...", roomID)
return false
}

return true
}),
)

t.Logf("Warning: Found unexpected rooms in public rooms list: %v", unexpectedRooms)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should only log this if len(unexpectedRooms) > 0


// Verify required keys are present in the room data.
err := should.MatchGJSON(
discoveredRoomData,
match.JSONKeyPresent("world_readable"),
match.JSONKeyPresent("guest_can_join"),
match.JSONKeyPresent("num_joined_members"),
)
if err != nil {
// The room is missing required keys, and
// it's unlikely to get them after
// calling the method again. Let's bail out.
t.Fatalf("Room %s data missing required keys: %s, data: %v", roomID, err.Error(), discoveredRoomData)
}

// Keep track of all validation errors, rather than bailing out on the first one.
validationErrors := make([]error, 0)

// Verify canonical alias
canonicalAlias := discoveredRoomData.Get("canonical_alias").Str
if canonicalAlias != expectedCanonicalAlias {
err = fmt.Errorf("Room %s has canonical alias '%s', expected '%s'", roomID, canonicalAlias, expectedCanonicalAlias)
validationErrors = append(validationErrors, err)
}

// Verify member count
numMembers := discoveredRoomData.Get("num_joined_members").Int()
if numMembers != 1 {
err = fmt.Errorf("Room %s has %d members, expected 1", roomID, numMembers)
validationErrors = append(validationErrors, err)
}

// Verify name field, if there is one to verify
name := discoveredRoomData.Get("name").Str
if roomConfig.name != "" {
if name != roomConfig.name {
err = fmt.Errorf("Room %s has name '%s', expected '%s'", roomID, name, roomConfig.name)
validationErrors = append(validationErrors, err)
}
} else {
if name != "" {
err = fmt.Errorf("Room %s has unexpected name '%s', expected no name", roomID, name)
validationErrors = append(validationErrors, err)
}
}

// Verify topic field, if there is one to verify
topic := discoveredRoomData.Get("topic").Str
if roomConfig.topic != "" {
if topic != roomConfig.topic {
err = fmt.Errorf("Room %s has topic '%s', expected '%s'", roomID, topic, roomConfig.topic)
validationErrors = append(validationErrors, err)
}
} else {
if topic != "" {
err = fmt.Errorf("Room %s has unexpected topic '%s', expected no topic", roomID, topic)
validationErrors = append(validationErrors, err)
}
}

if len(validationErrors) > 0 {
for _, e := range validationErrors {
t.Errorf("Validation error for room %s: %s", roomID, e.Error())
}

t.Fail()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Using t.Fatalf(...) with some message looks better in the output.

Current t.Fail():

=== RUN   TestPublicRooms
2025/10/30 11:42:46 Sharing [SERVER_NAME=hs1 SYNAPSE_LOG_TESTING=1 SYNAPSE_COMPLEMENT_USE_WORKERS= SYNAPSE_COMPLEMENT_DATABASE=sqlite] host environment variables with container
=== RUN   TestPublicRooms/parallel
=== RUN   TestPublicRooms/parallel/Name/topic_keys_are_correct
=== PAUSE TestPublicRooms/parallel/Name/topic_keys_are_correct
=== CONT  TestPublicRooms/parallel/Name/topic_keys_are_correct
    client.go:784: [CSAPI] POST hs1/_matrix/client/v3/register => 200 OK (117.527638ms)
=== RUN   TestPublicRooms/parallel/Name/topic_keys_are_correct/Creating_room_with_alias_publicroomalias_no_name
=== NAME  TestPublicRooms/parallel/Name/topic_keys_are_correct
    client.go:784: [CSAPI] POST hs1/_matrix/client/v3/createRoom => 200 OK (411.951625ms)
=== NAME  TestPublicRooms/parallel/Name/topic_keys_are_correct/Creating_room_with_alias_publicroomalias_no_name
    public_rooms_test.go:103: Created room !RqNTiptKqCqmtWHWXC:hs1 with alias publicroomalias_no_name
=== NAME  TestPublicRooms/parallel/Name/topic_keys_are_correct
    client.go:784: [CSAPI] GET hs1/_matrix/client/v3/publicRooms => 200 OK (19.299404ms)
=== NAME  TestPublicRooms/parallel/Name/topic_keys_are_correct/Creating_room_with_alias_publicroomalias_no_name
    public_rooms_test.go:137: Warning: Found unexpected rooms in public rooms list: []
    public_rooms_test.go:200: Validation error for room !RqNTiptKqCqmtWHWXC:hs1: Room !RqNTiptKqCqmtWHWXC:hs1 has unexpected name '', expected no name
=== NAME  TestPublicRooms/parallel/Name/topic_keys_are_correct
    client.go:784: [CSAPI] PUT hs1/_matrix/client/v3/directory/list/room/!RqNTiptKqCqmtWHWXC:hs1 => 200 OK (21.216743ms)
2025/10/30 11:42:54 ============================================

Using t.Fatalf(...) instead:

=== RUN   TestPublicRooms
2025/10/30 11:41:17 Sharing [SERVER_NAME=hs1 SYNAPSE_LOG_TESTING=1 SYNAPSE_COMPLEMENT_USE_WORKERS= SYNAPSE_COMPLEMENT_DATABASE=sqlite] host environment variables with container
=== RUN   TestPublicRooms/parallel
=== RUN   TestPublicRooms/parallel/Name/topic_keys_are_correct
=== PAUSE TestPublicRooms/parallel/Name/topic_keys_are_correct
=== CONT  TestPublicRooms/parallel/Name/topic_keys_are_correct
    client.go:784: [CSAPI] POST hs1/_matrix/client/v3/register => 200 OK (121.73885ms)
=== RUN   TestPublicRooms/parallel/Name/topic_keys_are_correct/Creating_room_with_alias_publicroomalias_no_name
=== NAME  TestPublicRooms/parallel/Name/topic_keys_are_correct
    client.go:784: [CSAPI] POST hs1/_matrix/client/v3/createRoom => 200 OK (436.070145ms)
=== NAME  TestPublicRooms/parallel/Name/topic_keys_are_correct/Creating_room_with_alias_publicroomalias_no_name
    public_rooms_test.go:103: Created room !lKZeexiHPJLOnCUMOl:hs1 with alias publicroomalias_no_name
=== NAME  TestPublicRooms/parallel/Name/topic_keys_are_correct
    client.go:784: [CSAPI] GET hs1/_matrix/client/v3/publicRooms => 200 OK (20.858944ms)
=== NAME  TestPublicRooms/parallel/Name/topic_keys_are_correct/Creating_room_with_alias_publicroomalias_no_name
    public_rooms_test.go:137: Warning: Found unexpected rooms in public rooms list: []
    public_rooms_test.go:200: Validation error for room !lKZeexiHPJLOnCUMOl:hs1: Room !lKZeexiHPJLOnCUMOl:hs1 has unexpected name '', expected no name
    public_rooms_test.go:204: <your fatal message here>
2025/10/30 11:41:25 ============================================

}

// Remove the room from the public rooms list to avoid polluting other tests.
authedClient.MustDo(
t,
"PUT",
[]string{"_matrix", "client", "v3", "directory", "list", "room", roomID},
client.WithJSONBody(t, map[string]interface{}{
"visibility": "private",
}),
)
})
}
})
})
}

func parsePublicRoomsResponse(t *testing.T, res *http.Response) []gjson.Result {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
func parsePublicRoomsResponse(t *testing.T, res *http.Response) []gjson.Result {
func parsePublicRoomsResponse(t *testing.T, res *http.Response) []gjson.Result {
t.Helper()

body := must.ParseJSON(t, res.Body)

must.MatchGJSON(
t,
body,
match.JSONKeyPresent("chunk"),
match.JSONKeyTypeEqual("chunk", gjson.JSON),
)
Comment on lines +224 to +229
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we need this duplication with the checks below which have better assertion messages anyway


chunk := body.Get("chunk")
if !chunk.Exists() {
t.Fatalf("`chunk` field on public rooms response does not exist, got body: %v", body)
}
if !chunk.IsArray() {
t.Fatalf("`chunk` field on public rooms response is not an array, got: %v", chunk)
}

return chunk.Array()
}
Loading