From 07387a04b213dea84619641660deaf25868ca9b2 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 17 Nov 2025 14:26:52 -0600 Subject: [PATCH 01/21] Add some tests for push rule behavior on room upgrade --- tests/csapi/room_upgrade_test.go | 135 +++++++++++++++++++++++++++++++ 1 file changed, 135 insertions(+) create mode 100644 tests/csapi/room_upgrade_test.go diff --git a/tests/csapi/room_upgrade_test.go b/tests/csapi/room_upgrade_test.go new file mode 100644 index 00000000..72e4dc03 --- /dev/null +++ b/tests/csapi/room_upgrade_test.go @@ -0,0 +1,135 @@ +package csapi_tests + +import ( + "testing" + + "github.com/matrix-org/complement" + "github.com/matrix-org/complement/client" + "github.com/matrix-org/complement/helpers" + "github.com/matrix-org/complement/match" + "github.com/matrix-org/complement/must" + "github.com/matrix-org/gomatrixserverlib/spec" + "github.com/tidwall/gjson" +) + +func TestPushRuleRoomUpgrade(t *testing.T) { + deployment := complement.Deploy(t, 2) + defer deployment.Destroy(t) + + alice := deployment.Register(t, "hs1", helpers.RegistrationOpts{}) + bob := deployment.Register(t, "hs2", helpers.RegistrationOpts{}) + + // When a homeserver becomes aware of a room upgrade (upgrade is done on local + // homeserver), it should copy over any push rules for all of its local users from the + // old room to the new room. + t.Run("upgrading a room carries over the push rules for local users", func(t *testing.T) { + // Create a room + roomID := alice.MustCreateRoom(t, map[string]interface{}{ + "preset": "public_chat", + "room_version": "10", + }) + + // Add some push rules + alice.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, + client.WithJSONBody(t, map[string]interface{}{ + "actions": []string{"dont_notify"}, + }), + ) + + // Sanity check the push rules are in the expected state before the upgrade + pushRulesBefore := alice.GetAllPushRules(t) + must.MatchGJSON(t, pushRulesBefore, + match.JSONCheckOff("global.room", []interface{}{ + roomID, + }, + match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), + match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { + return match.JSONKeyEqual("actions.0", "dont_notify")(result) + }), + ), + ) + + // Upgrade the room + upgradeRes := alice.MustDo(t, "POST", []string{"_matrix", "client", "v3", "rooms", roomID, "upgrade"}, + client.WithJSONBody(t, map[string]string{ + "new_version": "11", + }), + ) + body := must.ParseJSON(t, upgradeRes.Body) + newRoomID := must.GetJSONFieldStr(t, body, "replacement_room") + + // Sanity check the push rules are in the expected state before the upgrade + pushRulesAfter := alice.GetAllPushRules(t) + must.MatchGJSON(t, pushRulesAfter, + match.JSONCheckOff("global.room", []interface{}{ + roomID, + newRoomID, + }, + match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), + match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { + return match.JSONKeyEqual("actions.0", "dont_notify")(result) + }), + ), + ) + }) + + // When a homeserver becomes aware of a room upgrade (upgrade is done on remote + // homeserver), it should copy over any push rules for all of its local users from the + // old room to the new room. + t.Run("joining a remote upgraded room carries over the push rules", func(t *testing.T) { + // Alice create a room + roomID := alice.MustCreateRoom(t, map[string]interface{}{ + "preset": "public_chat", + "room_version": "10", + }) + // Remote bob joins the room + // alice.MustInviteRoom(t, roomID, bob.UserID) + bob.MustJoinRoom(t, roomID, []spec.ServerName{ + deployment.GetFullyQualifiedHomeserverName(t, "hs1"), + }) + + // Bob adds some push rules + bob.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, + client.WithJSONBody(t, map[string]interface{}{ + "actions": []string{"dont_notify"}, + }), + ) + + // Sanity check the push rules are in the expected state before the upgrade + pushRulesBefore := bob.GetAllPushRules(t) + must.MatchGJSON(t, pushRulesBefore, + match.JSONCheckOff("global.room", []interface{}{ + roomID, + }, + match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), + match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { + return match.JSONKeyEqual("actions.0", "dont_notify")(result) + }), + ), + ) + + // Upgrade the room + upgradeRes := alice.MustDo(t, "POST", []string{"_matrix", "client", "v3", "rooms", roomID, "upgrade"}, + client.WithJSONBody(t, map[string]string{ + "new_version": "11", + }), + ) + body := must.ParseJSON(t, upgradeRes.Body) + newRoomID := must.GetJSONFieldStr(t, body, "replacement_room") + + // Sanity check the push rules are in the expected state before the upgrade + pushRulesAfter := bob.GetAllPushRules(t) + must.MatchGJSON(t, pushRulesAfter, + match.JSONCheckOff("global.room", []interface{}{ + roomID, + newRoomID, + }, + match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), + match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { + return match.JSONKeyEqual("actions.0", "dont_notify")(result) + }), + ), + ) + }) + +} From 3fdbd02deabb4c7f687c519f2ad36a5da59f9d92 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 17 Nov 2025 15:48:20 -0600 Subject: [PATCH 02/21] Make the test generic with multiple push rules types --- tests/csapi/room_upgrade_test.go | 257 +++++++++++++++++++------------ 1 file changed, 157 insertions(+), 100 deletions(-) diff --git a/tests/csapi/room_upgrade_test.go b/tests/csapi/room_upgrade_test.go index 72e4dc03..5ce6c0fb 100644 --- a/tests/csapi/room_upgrade_test.go +++ b/tests/csapi/room_upgrade_test.go @@ -1,6 +1,7 @@ package csapi_tests import ( + "fmt" "testing" "github.com/matrix-org/complement" @@ -19,117 +20,173 @@ func TestPushRuleRoomUpgrade(t *testing.T) { alice := deployment.Register(t, "hs1", helpers.RegistrationOpts{}) bob := deployment.Register(t, "hs2", helpers.RegistrationOpts{}) - // When a homeserver becomes aware of a room upgrade (upgrade is done on local - // homeserver), it should copy over any push rules for all of its local users from the - // old room to the new room. - t.Run("upgrading a room carries over the push rules for local users", func(t *testing.T) { - // Create a room - roomID := alice.MustCreateRoom(t, map[string]interface{}{ - "preset": "public_chat", - "room_version": "10", - }) - - // Add some push rules - alice.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, - client.WithJSONBody(t, map[string]interface{}{ - "actions": []string{"dont_notify"}, - }), - ) - - // Sanity check the push rules are in the expected state before the upgrade - pushRulesBefore := alice.GetAllPushRules(t) - must.MatchGJSON(t, pushRulesBefore, - match.JSONCheckOff("global.room", []interface{}{ - roomID, + for _, testCase := range []struct { + name string + // `content`, `room`, `sender`, `underride`, etc + ruleset string + pushRuleJsonBuilder func(roomID string) map[string]interface{} + pushRuleMatchersBuilder func(roomID string) []match.JSON + }{ + { + name: "`room` ruleset", + ruleset: "room", + pushRuleJsonBuilder: func(roomID string) map[string]interface{} { + return map[string]interface{}{ + "actions": []string{"dont_notify"}, + } }, - match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), - match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { - return match.JSONKeyEqual("actions.0", "dont_notify")(result) - }), - ), - ) + pushRuleMatchersBuilder: func(roomID string) []match.JSON { + return []match.JSON{ + match.JSONKeyEqual("actions.0", "dont_notify"), + } + }, + }, + { + name: "`underride` ruleset", + ruleset: "underride", + pushRuleJsonBuilder: func(roomID string) map[string]interface{} { + return map[string]interface{}{ + "conditions": []map[string]interface{}{ + { + "key": "room", + "kind": "event_match", + "pattern": roomID, + }, + }, + "actions": []string{"dont_notify"}, + } + }, + pushRuleMatchersBuilder: func(roomID string) []match.JSON { + return []match.JSON{ + match.JSONKeyEqual("actions.0", "dont_notify"), + match.JSONKeyEqual("conditions.0.key", "room"), + match.JSONKeyEqual("conditions.0.pattern", roomID), + } + }, + }, + } { + // When a homeserver becomes aware of a room upgrade (upgrade is done on local + // homeserver), it should copy over any push rules for all of its local users from the + // old room to the new room. + t.Run(fmt.Sprintf("upgrading a room carries over the push rules for local users (%s)", testCase.name), func(t *testing.T) { + // Create a room + roomID := alice.MustCreateRoom(t, map[string]interface{}{ + "preset": "public_chat", + "room_version": "10", + }) - // Upgrade the room - upgradeRes := alice.MustDo(t, "POST", []string{"_matrix", "client", "v3", "rooms", roomID, "upgrade"}, - client.WithJSONBody(t, map[string]string{ - "new_version": "11", - }), - ) - body := must.ParseJSON(t, upgradeRes.Body) - newRoomID := must.GetJSONFieldStr(t, body, "replacement_room") + // Add some push rules + alice.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", testCase.ruleset, roomID}, + client.WithJSONBody(t, testCase.pushRuleJsonBuilder(roomID)), + ) - // Sanity check the push rules are in the expected state before the upgrade - pushRulesAfter := alice.GetAllPushRules(t) - must.MatchGJSON(t, pushRulesAfter, - match.JSONCheckOff("global.room", []interface{}{ - roomID, - newRoomID, - }, - match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), - match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { - return match.JSONKeyEqual("actions.0", "dont_notify")(result) + // Sanity check the push rules are in the expected state before the upgrade + pushRulesBefore := alice.GetAllPushRules(t) + must.MatchGJSON(t, pushRulesBefore, + match.JSONCheckOff(fmt.Sprintf("global.%s", testCase.ruleset), + []interface{}{ + roomID, + }, + match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), + match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { + for _, matcher := range testCase.pushRuleMatchersBuilder(roomIDFromPushRule.(string)) { + if err := matcher(result); err != nil { + return err + } + } + return nil + }), + ), + ) + + // Upgrade the room + upgradeRes := alice.MustDo(t, "POST", []string{"_matrix", "client", "v3", "rooms", roomID, "upgrade"}, + client.WithJSONBody(t, map[string]string{ + "new_version": "11", }), - ), - ) - }) + ) + body := must.ParseJSON(t, upgradeRes.Body) + newRoomID := must.GetJSONFieldStr(t, body, "replacement_room") - // When a homeserver becomes aware of a room upgrade (upgrade is done on remote - // homeserver), it should copy over any push rules for all of its local users from the - // old room to the new room. - t.Run("joining a remote upgraded room carries over the push rules", func(t *testing.T) { - // Alice create a room - roomID := alice.MustCreateRoom(t, map[string]interface{}{ - "preset": "public_chat", - "room_version": "10", - }) - // Remote bob joins the room - // alice.MustInviteRoom(t, roomID, bob.UserID) - bob.MustJoinRoom(t, roomID, []spec.ServerName{ - deployment.GetFullyQualifiedHomeserverName(t, "hs1"), + // Sanity check the push rules are in the expected state before the upgrade + pushRulesAfter := alice.GetAllPushRules(t) + must.MatchGJSON(t, pushRulesAfter, + match.JSONCheckOff(fmt.Sprintf("global.%s", testCase.ruleset), + []interface{}{ + roomID, + newRoomID, + }, + match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), + match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { + for _, matcher := range testCase.pushRuleMatchersBuilder(roomIDFromPushRule.(string)) { + if err := matcher(result); err != nil { + return err + } + } + return nil + }), + ), + ) }) - // Bob adds some push rules - bob.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, - client.WithJSONBody(t, map[string]interface{}{ - "actions": []string{"dont_notify"}, - }), - ) + // When a homeserver becomes aware of a room upgrade (upgrade is done on remote + // homeserver), it should copy over any push rules for all of its local users from the + // old room to the new room. + t.Run("joining a remote upgraded room carries over the push rules", func(t *testing.T) { + // Alice create a room + roomID := alice.MustCreateRoom(t, map[string]interface{}{ + "preset": "public_chat", + "room_version": "10", + }) + // Remote bob joins the room + // alice.MustInviteRoom(t, roomID, bob.UserID) + bob.MustJoinRoom(t, roomID, []spec.ServerName{ + deployment.GetFullyQualifiedHomeserverName(t, "hs1"), + }) - // Sanity check the push rules are in the expected state before the upgrade - pushRulesBefore := bob.GetAllPushRules(t) - must.MatchGJSON(t, pushRulesBefore, - match.JSONCheckOff("global.room", []interface{}{ - roomID, - }, - match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), - match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { - return match.JSONKeyEqual("actions.0", "dont_notify")(result) + // Bob adds some push rules + bob.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, + client.WithJSONBody(t, map[string]interface{}{ + "actions": []string{"dont_notify"}, }), - ), - ) + ) - // Upgrade the room - upgradeRes := alice.MustDo(t, "POST", []string{"_matrix", "client", "v3", "rooms", roomID, "upgrade"}, - client.WithJSONBody(t, map[string]string{ - "new_version": "11", - }), - ) - body := must.ParseJSON(t, upgradeRes.Body) - newRoomID := must.GetJSONFieldStr(t, body, "replacement_room") + // Sanity check the push rules are in the expected state before the upgrade + pushRulesBefore := bob.GetAllPushRules(t) + must.MatchGJSON(t, pushRulesBefore, + match.JSONCheckOff("global.room", []interface{}{ + roomID, + }, + match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), + match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { + return match.JSONKeyEqual("actions.0", "dont_notify")(result) + }), + ), + ) - // Sanity check the push rules are in the expected state before the upgrade - pushRulesAfter := bob.GetAllPushRules(t) - must.MatchGJSON(t, pushRulesAfter, - match.JSONCheckOff("global.room", []interface{}{ - roomID, - newRoomID, - }, - match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), - match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { - return match.JSONKeyEqual("actions.0", "dont_notify")(result) + // Upgrade the room + upgradeRes := alice.MustDo(t, "POST", []string{"_matrix", "client", "v3", "rooms", roomID, "upgrade"}, + client.WithJSONBody(t, map[string]string{ + "new_version": "11", }), - ), - ) - }) + ) + body := must.ParseJSON(t, upgradeRes.Body) + newRoomID := must.GetJSONFieldStr(t, body, "replacement_room") + + // Sanity check the push rules are in the expected state before the upgrade + pushRulesAfter := bob.GetAllPushRules(t) + must.MatchGJSON(t, pushRulesAfter, + match.JSONCheckOff("global.room", []interface{}{ + roomID, + newRoomID, + }, + match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), + match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { + return match.JSONKeyEqual("actions.0", "dont_notify")(result) + }), + ), + ) + }) + } } From cc58ec9b6a47107b6aae676e8161bf00722a92e3 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 17 Nov 2025 15:48:41 -0600 Subject: [PATCH 03/21] Revert "Make the test generic with multiple push rules types" This reverts commit 3fdbd02deabb4c7f687c519f2ad36a5da59f9d92. --- tests/csapi/room_upgrade_test.go | 257 ++++++++++++------------------- 1 file changed, 100 insertions(+), 157 deletions(-) diff --git a/tests/csapi/room_upgrade_test.go b/tests/csapi/room_upgrade_test.go index 5ce6c0fb..72e4dc03 100644 --- a/tests/csapi/room_upgrade_test.go +++ b/tests/csapi/room_upgrade_test.go @@ -1,7 +1,6 @@ package csapi_tests import ( - "fmt" "testing" "github.com/matrix-org/complement" @@ -20,173 +19,117 @@ func TestPushRuleRoomUpgrade(t *testing.T) { alice := deployment.Register(t, "hs1", helpers.RegistrationOpts{}) bob := deployment.Register(t, "hs2", helpers.RegistrationOpts{}) - for _, testCase := range []struct { - name string - // `content`, `room`, `sender`, `underride`, etc - ruleset string - pushRuleJsonBuilder func(roomID string) map[string]interface{} - pushRuleMatchersBuilder func(roomID string) []match.JSON - }{ - { - name: "`room` ruleset", - ruleset: "room", - pushRuleJsonBuilder: func(roomID string) map[string]interface{} { - return map[string]interface{}{ - "actions": []string{"dont_notify"}, - } - }, - pushRuleMatchersBuilder: func(roomID string) []match.JSON { - return []match.JSON{ - match.JSONKeyEqual("actions.0", "dont_notify"), - } - }, - }, - { - name: "`underride` ruleset", - ruleset: "underride", - pushRuleJsonBuilder: func(roomID string) map[string]interface{} { - return map[string]interface{}{ - "conditions": []map[string]interface{}{ - { - "key": "room", - "kind": "event_match", - "pattern": roomID, - }, - }, - "actions": []string{"dont_notify"}, - } - }, - pushRuleMatchersBuilder: func(roomID string) []match.JSON { - return []match.JSON{ - match.JSONKeyEqual("actions.0", "dont_notify"), - match.JSONKeyEqual("conditions.0.key", "room"), - match.JSONKeyEqual("conditions.0.pattern", roomID), - } - }, - }, - } { - // When a homeserver becomes aware of a room upgrade (upgrade is done on local - // homeserver), it should copy over any push rules for all of its local users from the - // old room to the new room. - t.Run(fmt.Sprintf("upgrading a room carries over the push rules for local users (%s)", testCase.name), func(t *testing.T) { - // Create a room - roomID := alice.MustCreateRoom(t, map[string]interface{}{ - "preset": "public_chat", - "room_version": "10", - }) + // When a homeserver becomes aware of a room upgrade (upgrade is done on local + // homeserver), it should copy over any push rules for all of its local users from the + // old room to the new room. + t.Run("upgrading a room carries over the push rules for local users", func(t *testing.T) { + // Create a room + roomID := alice.MustCreateRoom(t, map[string]interface{}{ + "preset": "public_chat", + "room_version": "10", + }) + + // Add some push rules + alice.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, + client.WithJSONBody(t, map[string]interface{}{ + "actions": []string{"dont_notify"}, + }), + ) - // Add some push rules - alice.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", testCase.ruleset, roomID}, - client.WithJSONBody(t, testCase.pushRuleJsonBuilder(roomID)), - ) + // Sanity check the push rules are in the expected state before the upgrade + pushRulesBefore := alice.GetAllPushRules(t) + must.MatchGJSON(t, pushRulesBefore, + match.JSONCheckOff("global.room", []interface{}{ + roomID, + }, + match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), + match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { + return match.JSONKeyEqual("actions.0", "dont_notify")(result) + }), + ), + ) - // Sanity check the push rules are in the expected state before the upgrade - pushRulesBefore := alice.GetAllPushRules(t) - must.MatchGJSON(t, pushRulesBefore, - match.JSONCheckOff(fmt.Sprintf("global.%s", testCase.ruleset), - []interface{}{ - roomID, - }, - match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), - match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { - for _, matcher := range testCase.pushRuleMatchersBuilder(roomIDFromPushRule.(string)) { - if err := matcher(result); err != nil { - return err - } - } - return nil - }), - ), - ) + // Upgrade the room + upgradeRes := alice.MustDo(t, "POST", []string{"_matrix", "client", "v3", "rooms", roomID, "upgrade"}, + client.WithJSONBody(t, map[string]string{ + "new_version": "11", + }), + ) + body := must.ParseJSON(t, upgradeRes.Body) + newRoomID := must.GetJSONFieldStr(t, body, "replacement_room") - // Upgrade the room - upgradeRes := alice.MustDo(t, "POST", []string{"_matrix", "client", "v3", "rooms", roomID, "upgrade"}, - client.WithJSONBody(t, map[string]string{ - "new_version": "11", + // Sanity check the push rules are in the expected state before the upgrade + pushRulesAfter := alice.GetAllPushRules(t) + must.MatchGJSON(t, pushRulesAfter, + match.JSONCheckOff("global.room", []interface{}{ + roomID, + newRoomID, + }, + match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), + match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { + return match.JSONKeyEqual("actions.0", "dont_notify")(result) }), - ) - body := must.ParseJSON(t, upgradeRes.Body) - newRoomID := must.GetJSONFieldStr(t, body, "replacement_room") + ), + ) + }) - // Sanity check the push rules are in the expected state before the upgrade - pushRulesAfter := alice.GetAllPushRules(t) - must.MatchGJSON(t, pushRulesAfter, - match.JSONCheckOff(fmt.Sprintf("global.%s", testCase.ruleset), - []interface{}{ - roomID, - newRoomID, - }, - match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), - match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { - for _, matcher := range testCase.pushRuleMatchersBuilder(roomIDFromPushRule.(string)) { - if err := matcher(result); err != nil { - return err - } - } - return nil - }), - ), - ) + // When a homeserver becomes aware of a room upgrade (upgrade is done on remote + // homeserver), it should copy over any push rules for all of its local users from the + // old room to the new room. + t.Run("joining a remote upgraded room carries over the push rules", func(t *testing.T) { + // Alice create a room + roomID := alice.MustCreateRoom(t, map[string]interface{}{ + "preset": "public_chat", + "room_version": "10", + }) + // Remote bob joins the room + // alice.MustInviteRoom(t, roomID, bob.UserID) + bob.MustJoinRoom(t, roomID, []spec.ServerName{ + deployment.GetFullyQualifiedHomeserverName(t, "hs1"), }) - // When a homeserver becomes aware of a room upgrade (upgrade is done on remote - // homeserver), it should copy over any push rules for all of its local users from the - // old room to the new room. - t.Run("joining a remote upgraded room carries over the push rules", func(t *testing.T) { - // Alice create a room - roomID := alice.MustCreateRoom(t, map[string]interface{}{ - "preset": "public_chat", - "room_version": "10", - }) - // Remote bob joins the room - // alice.MustInviteRoom(t, roomID, bob.UserID) - bob.MustJoinRoom(t, roomID, []spec.ServerName{ - deployment.GetFullyQualifiedHomeserverName(t, "hs1"), - }) + // Bob adds some push rules + bob.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, + client.WithJSONBody(t, map[string]interface{}{ + "actions": []string{"dont_notify"}, + }), + ) - // Bob adds some push rules - bob.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, - client.WithJSONBody(t, map[string]interface{}{ - "actions": []string{"dont_notify"}, + // Sanity check the push rules are in the expected state before the upgrade + pushRulesBefore := bob.GetAllPushRules(t) + must.MatchGJSON(t, pushRulesBefore, + match.JSONCheckOff("global.room", []interface{}{ + roomID, + }, + match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), + match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { + return match.JSONKeyEqual("actions.0", "dont_notify")(result) }), - ) + ), + ) - // Sanity check the push rules are in the expected state before the upgrade - pushRulesBefore := bob.GetAllPushRules(t) - must.MatchGJSON(t, pushRulesBefore, - match.JSONCheckOff("global.room", []interface{}{ - roomID, - }, - match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), - match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { - return match.JSONKeyEqual("actions.0", "dont_notify")(result) - }), - ), - ) + // Upgrade the room + upgradeRes := alice.MustDo(t, "POST", []string{"_matrix", "client", "v3", "rooms", roomID, "upgrade"}, + client.WithJSONBody(t, map[string]string{ + "new_version": "11", + }), + ) + body := must.ParseJSON(t, upgradeRes.Body) + newRoomID := must.GetJSONFieldStr(t, body, "replacement_room") - // Upgrade the room - upgradeRes := alice.MustDo(t, "POST", []string{"_matrix", "client", "v3", "rooms", roomID, "upgrade"}, - client.WithJSONBody(t, map[string]string{ - "new_version": "11", + // Sanity check the push rules are in the expected state before the upgrade + pushRulesAfter := bob.GetAllPushRules(t) + must.MatchGJSON(t, pushRulesAfter, + match.JSONCheckOff("global.room", []interface{}{ + roomID, + newRoomID, + }, + match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), + match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { + return match.JSONKeyEqual("actions.0", "dont_notify")(result) }), - ) - body := must.ParseJSON(t, upgradeRes.Body) - newRoomID := must.GetJSONFieldStr(t, body, "replacement_room") - - // Sanity check the push rules are in the expected state before the upgrade - pushRulesAfter := bob.GetAllPushRules(t) - must.MatchGJSON(t, pushRulesAfter, - match.JSONCheckOff("global.room", []interface{}{ - roomID, - newRoomID, - }, - match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), - match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { - return match.JSONKeyEqual("actions.0", "dont_notify")(result) - }), - ), - ) - }) - } + ), + ) + }) } From 1167cee7c6d8c4cedd334fdca3992aa09d2115b8 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 17 Nov 2025 15:58:23 -0600 Subject: [PATCH 04/21] Add `MustUpgradeRoom` --- client/client.go | 25 +++++++++++++++++++++++++ tests/csapi/room_upgrade_test.go | 17 ++--------------- 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/client/client.go b/client/client.go index e43ebce4..04d8f355 100644 --- a/client/client.go +++ b/client/client.go @@ -174,6 +174,31 @@ func (c *CSAPI) CreateRoom(t ct.TestLike, body map[string]interface{}) *http.Res return c.Do(t, "POST", []string{"_matrix", "client", "v3", "createRoom"}, WithJSONBody(t, body)) } +// MustUpgradeRoom upgrades a room to the newVersion. Fails the test on error. Returns the new room ID. +func (c *CSAPI) MustUpgradeRoom(t ct.TestLike, roomID string, newVersion string) string { + t.Helper() + res := c.UpgradeRoom(t, roomID, newVersion) + mustRespond2xx(t, res) + resBody := ParseJSON(t, res) + return GetJSONFieldStr(t, resBody, "replacement_room") +} + +// UpgradeRoom upgrades a room to the newVersion +func (c *CSAPI) UpgradeRoom(t ct.TestLike, roomID string, newVersion string) *http.Response { + t.Helper() + // Ensure we don't call create a room (upgrade creates a new room) from the same user + // in parallel, else we might try to make 2 rooms in the same millisecond (same + // `origin_server_ts`), causing v12 rooms to get the same room ID thus failing the + // test. + c.createRoomMutex.Lock() + defer c.createRoomMutex.Unlock() + return c.Do(t, "POST", []string{"_matrix", "client", "v3", "rooms", roomID, "upgrade"}, + WithJSONBody(t, map[string]string{ + "new_version": newVersion, + }), + ) +} + // MustJoinRoom joins the room ID or alias given, else fails the test. Returns the room ID. // // Args: diff --git a/tests/csapi/room_upgrade_test.go b/tests/csapi/room_upgrade_test.go index 72e4dc03..e973c791 100644 --- a/tests/csapi/room_upgrade_test.go +++ b/tests/csapi/room_upgrade_test.go @@ -50,13 +50,7 @@ func TestPushRuleRoomUpgrade(t *testing.T) { ) // Upgrade the room - upgradeRes := alice.MustDo(t, "POST", []string{"_matrix", "client", "v3", "rooms", roomID, "upgrade"}, - client.WithJSONBody(t, map[string]string{ - "new_version": "11", - }), - ) - body := must.ParseJSON(t, upgradeRes.Body) - newRoomID := must.GetJSONFieldStr(t, body, "replacement_room") + newRoomID := alice.MustUpgradeRoom(t, roomID, "11") // Sanity check the push rules are in the expected state before the upgrade pushRulesAfter := alice.GetAllPushRules(t) @@ -109,13 +103,7 @@ func TestPushRuleRoomUpgrade(t *testing.T) { ) // Upgrade the room - upgradeRes := alice.MustDo(t, "POST", []string{"_matrix", "client", "v3", "rooms", roomID, "upgrade"}, - client.WithJSONBody(t, map[string]string{ - "new_version": "11", - }), - ) - body := must.ParseJSON(t, upgradeRes.Body) - newRoomID := must.GetJSONFieldStr(t, body, "replacement_room") + newRoomID := alice.MustUpgradeRoom(t, roomID, "11") // Sanity check the push rules are in the expected state before the upgrade pushRulesAfter := bob.GetAllPushRules(t) @@ -131,5 +119,4 @@ func TestPushRuleRoomUpgrade(t *testing.T) { ), ) }) - } From d53a3079e0f7186e2b5aa940ab2d244c0577f48e Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 17 Nov 2025 16:06:13 -0600 Subject: [PATCH 05/21] Test multiple users --- tests/csapi/room_upgrade_test.go | 138 +++++++++++++++++++------------ 1 file changed, 86 insertions(+), 52 deletions(-) diff --git a/tests/csapi/room_upgrade_test.go b/tests/csapi/room_upgrade_test.go index e973c791..d8935522 100644 --- a/tests/csapi/room_upgrade_test.go +++ b/tests/csapi/room_upgrade_test.go @@ -17,17 +17,24 @@ func TestPushRuleRoomUpgrade(t *testing.T) { defer deployment.Destroy(t) alice := deployment.Register(t, "hs1", helpers.RegistrationOpts{}) + alice2 := deployment.Register(t, "hs1", helpers.RegistrationOpts{}) bob := deployment.Register(t, "hs2", helpers.RegistrationOpts{}) + bob2 := deployment.Register(t, "hs2", helpers.RegistrationOpts{}) // When a homeserver becomes aware of a room upgrade (upgrade is done on local - // homeserver), it should copy over any push rules for all of its local users from the - // old room to the new room. - t.Run("upgrading a room carries over the push rules for local users", func(t *testing.T) { + // homeserver), it should copy over any existing push rules for all of its local users + // from the old room to the new room at the time of upgrade. + t.Run("upgrading a room carries over existing push rules for local users", func(t *testing.T) { // Create a room roomID := alice.MustCreateRoom(t, map[string]interface{}{ "preset": "public_chat", "room_version": "10", }) + // Have alice2 join the room + // + // We use two users to ensure that all users get taken care of (not just the person + // upgrading the room). + alice2.MustJoinRoom(t, roomID, nil) // Add some push rules alice.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, @@ -35,41 +42,52 @@ func TestPushRuleRoomUpgrade(t *testing.T) { "actions": []string{"dont_notify"}, }), ) + alice2.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, + client.WithJSONBody(t, map[string]interface{}{ + "actions": []string{"dont_notify"}, + }), + ) // Sanity check the push rules are in the expected state before the upgrade - pushRulesBefore := alice.GetAllPushRules(t) - must.MatchGJSON(t, pushRulesBefore, - match.JSONCheckOff("global.room", []interface{}{ - roomID, - }, - match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), - match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { - return match.JSONKeyEqual("actions.0", "dont_notify")(result) - }), - ), - ) + for _, client := range []*client.CSAPI{alice, alice2} { + pushRulesBefore := client.GetAllPushRules(t) + must.MatchGJSON(t, pushRulesBefore, + match.JSONCheckOff("global.room", []interface{}{ + roomID, + }, + match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), + match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { + return match.JSONKeyEqual("actions.0", "dont_notify")(result) + }), + ), + ) + } // Upgrade the room newRoomID := alice.MustUpgradeRoom(t, roomID, "11") + // Alice2 joins the new room + alice2.MustJoinRoom(t, newRoomID, nil) // Sanity check the push rules are in the expected state before the upgrade - pushRulesAfter := alice.GetAllPushRules(t) - must.MatchGJSON(t, pushRulesAfter, - match.JSONCheckOff("global.room", []interface{}{ - roomID, - newRoomID, - }, - match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), - match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { - return match.JSONKeyEqual("actions.0", "dont_notify")(result) - }), - ), - ) + for _, client := range []*client.CSAPI{alice, alice2} { + pushRulesAfter := client.GetAllPushRules(t) + must.MatchGJSON(t, pushRulesAfter, + match.JSONCheckOff("global.room", []interface{}{ + roomID, + newRoomID, + }, + match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), + match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { + return match.JSONKeyEqual("actions.0", "dont_notify")(result) + }), + ), + ) + } }) // When a homeserver becomes aware of a room upgrade (upgrade is done on remote - // homeserver), it should copy over any push rules for all of its local users from the - // old room to the new room. + // homeserver), it should copy over any existing push rules for all of its local users + // from the old room to the new room at the time of upgrade. t.Run("joining a remote upgraded room carries over the push rules", func(t *testing.T) { // Alice create a room roomID := alice.MustCreateRoom(t, map[string]interface{}{ @@ -81,42 +99,58 @@ func TestPushRuleRoomUpgrade(t *testing.T) { bob.MustJoinRoom(t, roomID, []spec.ServerName{ deployment.GetFullyQualifiedHomeserverName(t, "hs1"), }) + // Remote bob2 joins the room + // + // We use two users to ensure that all users get taken care of (not just the first + // user on the homeserver). + bob2.MustJoinRoom(t, roomID, []spec.ServerName{ + deployment.GetFullyQualifiedHomeserverName(t, "hs1"), + }) - // Bob adds some push rules + // Add some push rules bob.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, client.WithJSONBody(t, map[string]interface{}{ "actions": []string{"dont_notify"}, }), ) + bob2.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, + client.WithJSONBody(t, map[string]interface{}{ + "actions": []string{"dont_notify"}, + }), + ) // Sanity check the push rules are in the expected state before the upgrade - pushRulesBefore := bob.GetAllPushRules(t) - must.MatchGJSON(t, pushRulesBefore, - match.JSONCheckOff("global.room", []interface{}{ - roomID, - }, - match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), - match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { - return match.JSONKeyEqual("actions.0", "dont_notify")(result) - }), - ), - ) + for _, client := range []*client.CSAPI{bob, bob2} { + pushRulesBefore := client.GetAllPushRules(t) + must.MatchGJSON(t, pushRulesBefore, + match.JSONCheckOff("global.room", []interface{}{ + roomID, + }, + match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), + match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { + return match.JSONKeyEqual("actions.0", "dont_notify")(result) + }), + ), + ) + } // Upgrade the room newRoomID := alice.MustUpgradeRoom(t, roomID, "11") // Sanity check the push rules are in the expected state before the upgrade - pushRulesAfter := bob.GetAllPushRules(t) - must.MatchGJSON(t, pushRulesAfter, - match.JSONCheckOff("global.room", []interface{}{ - roomID, - newRoomID, - }, - match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), - match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { - return match.JSONKeyEqual("actions.0", "dont_notify")(result) - }), - ), - ) + for _, client := range []*client.CSAPI{bob, bob2} { + pushRulesAfter := client.GetAllPushRules(t) + must.MatchGJSON(t, pushRulesAfter, + match.JSONCheckOff("global.room", []interface{}{ + roomID, + newRoomID, + }, + match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), + match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { + return match.JSONKeyEqual("actions.0", "dont_notify")(result) + }), + ), + ) + } }) } From e00c1c04b5214ce549f65cfb6eafed96d3d4c95f Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 17 Nov 2025 16:10:16 -0600 Subject: [PATCH 06/21] Better remote tests --- tests/csapi/room_upgrade_test.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/tests/csapi/room_upgrade_test.go b/tests/csapi/room_upgrade_test.go index d8935522..311945b7 100644 --- a/tests/csapi/room_upgrade_test.go +++ b/tests/csapi/room_upgrade_test.go @@ -65,10 +65,11 @@ func TestPushRuleRoomUpgrade(t *testing.T) { // Upgrade the room newRoomID := alice.MustUpgradeRoom(t, roomID, "11") + // Alice2 joins the new room alice2.MustJoinRoom(t, newRoomID, nil) - // Sanity check the push rules are in the expected state before the upgrade + // Sanity check the push rules are in the expected state after the upgrade for _, client := range []*client.CSAPI{alice, alice2} { pushRulesAfter := client.GetAllPushRules(t) must.MatchGJSON(t, pushRulesAfter, @@ -88,14 +89,13 @@ func TestPushRuleRoomUpgrade(t *testing.T) { // When a homeserver becomes aware of a room upgrade (upgrade is done on remote // homeserver), it should copy over any existing push rules for all of its local users // from the old room to the new room at the time of upgrade. - t.Run("joining a remote upgraded room carries over the push rules", func(t *testing.T) { + t.Run("joining a remote upgraded room carries over existing push rules", func(t *testing.T) { // Alice create a room roomID := alice.MustCreateRoom(t, map[string]interface{}{ "preset": "public_chat", "room_version": "10", }) // Remote bob joins the room - // alice.MustInviteRoom(t, roomID, bob.UserID) bob.MustJoinRoom(t, roomID, []spec.ServerName{ deployment.GetFullyQualifiedHomeserverName(t, "hs1"), }) @@ -137,7 +137,16 @@ func TestPushRuleRoomUpgrade(t *testing.T) { // Upgrade the room newRoomID := alice.MustUpgradeRoom(t, roomID, "11") - // Sanity check the push rules are in the expected state before the upgrade + // Remote bob joins the new room + bob.MustJoinRoom(t, newRoomID, []spec.ServerName{ + deployment.GetFullyQualifiedHomeserverName(t, "hs1"), + }) + // Remote bob2 joins the new room + bob2.MustJoinRoom(t, newRoomID, []spec.ServerName{ + deployment.GetFullyQualifiedHomeserverName(t, "hs1"), + }) + + // Sanity check the push rules are in the expected state after the upgrade for _, client := range []*client.CSAPI{bob, bob2} { pushRulesAfter := client.GetAllPushRules(t) must.MatchGJSON(t, pushRulesAfter, From fd90c90507309162e14bc7f18bbe3cb2cf9a46fd Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 17 Nov 2025 16:25:12 -0600 Subject: [PATCH 07/21] Run tests in parallel --- tests/csapi/room_upgrade_test.go | 270 ++++++++++++++++--------------- 1 file changed, 138 insertions(+), 132 deletions(-) diff --git a/tests/csapi/room_upgrade_test.go b/tests/csapi/room_upgrade_test.go index 311945b7..4e06ea1f 100644 --- a/tests/csapi/room_upgrade_test.go +++ b/tests/csapi/room_upgrade_test.go @@ -21,145 +21,151 @@ func TestPushRuleRoomUpgrade(t *testing.T) { bob := deployment.Register(t, "hs2", helpers.RegistrationOpts{}) bob2 := deployment.Register(t, "hs2", helpers.RegistrationOpts{}) - // When a homeserver becomes aware of a room upgrade (upgrade is done on local - // homeserver), it should copy over any existing push rules for all of its local users - // from the old room to the new room at the time of upgrade. - t.Run("upgrading a room carries over existing push rules for local users", func(t *testing.T) { - // Create a room - roomID := alice.MustCreateRoom(t, map[string]interface{}{ - "preset": "public_chat", - "room_version": "10", - }) - // Have alice2 join the room - // - // We use two users to ensure that all users get taken care of (not just the person - // upgrading the room). - alice2.MustJoinRoom(t, roomID, nil) - - // Add some push rules - alice.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, - client.WithJSONBody(t, map[string]interface{}{ - "actions": []string{"dont_notify"}, - }), - ) - alice2.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, - client.WithJSONBody(t, map[string]interface{}{ - "actions": []string{"dont_notify"}, - }), - ) - - // Sanity check the push rules are in the expected state before the upgrade - for _, client := range []*client.CSAPI{alice, alice2} { - pushRulesBefore := client.GetAllPushRules(t) - must.MatchGJSON(t, pushRulesBefore, - match.JSONCheckOff("global.room", []interface{}{ - roomID, - }, - match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), - match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { - return match.JSONKeyEqual("actions.0", "dont_notify")(result) - }), - ), + t.Run("parallel", func(t *testing.T) { + // When a homeserver becomes aware of a room upgrade (upgrade is done on local + // homeserver), it should copy over any existing push rules for all of its local users + // from the old room to the new room at the time of upgrade. + t.Run("upgrading a room carries over existing push rules for local users", func(t *testing.T) { + t.Parallel() + + // Create a room + roomID := alice.MustCreateRoom(t, map[string]interface{}{ + "preset": "public_chat", + "room_version": "10", + }) + // Have alice2 join the room + // + // We use two users to ensure that all users get taken care of (not just the person + // upgrading the room). + alice2.MustJoinRoom(t, roomID, nil) + + // Add some push rules + alice.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, + client.WithJSONBody(t, map[string]interface{}{ + "actions": []string{"dont_notify"}, + }), ) - } - - // Upgrade the room - newRoomID := alice.MustUpgradeRoom(t, roomID, "11") - - // Alice2 joins the new room - alice2.MustJoinRoom(t, newRoomID, nil) - - // Sanity check the push rules are in the expected state after the upgrade - for _, client := range []*client.CSAPI{alice, alice2} { - pushRulesAfter := client.GetAllPushRules(t) - must.MatchGJSON(t, pushRulesAfter, - match.JSONCheckOff("global.room", []interface{}{ - roomID, - newRoomID, - }, - match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), - match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { - return match.JSONKeyEqual("actions.0", "dont_notify")(result) - }), - ), + alice2.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, + client.WithJSONBody(t, map[string]interface{}{ + "actions": []string{"dont_notify"}, + }), ) - } - }) - // When a homeserver becomes aware of a room upgrade (upgrade is done on remote - // homeserver), it should copy over any existing push rules for all of its local users - // from the old room to the new room at the time of upgrade. - t.Run("joining a remote upgraded room carries over existing push rules", func(t *testing.T) { - // Alice create a room - roomID := alice.MustCreateRoom(t, map[string]interface{}{ - "preset": "public_chat", - "room_version": "10", - }) - // Remote bob joins the room - bob.MustJoinRoom(t, roomID, []spec.ServerName{ - deployment.GetFullyQualifiedHomeserverName(t, "hs1"), - }) - // Remote bob2 joins the room - // - // We use two users to ensure that all users get taken care of (not just the first - // user on the homeserver). - bob2.MustJoinRoom(t, roomID, []spec.ServerName{ - deployment.GetFullyQualifiedHomeserverName(t, "hs1"), + // Sanity check the push rules are in the expected state before the upgrade + for _, client := range []*client.CSAPI{alice, alice2} { + pushRulesBefore := client.GetAllPushRules(t) + must.MatchGJSON(t, pushRulesBefore, + match.JSONCheckOff("global.room", []interface{}{ + roomID, + }, + match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), + match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { + return match.JSONKeyEqual("actions.0", "dont_notify")(result) + }), + ), + ) + } + + // Upgrade the room + newRoomID := alice.MustUpgradeRoom(t, roomID, "11") + + // Alice2 joins the new room + alice2.MustJoinRoom(t, newRoomID, nil) + + // Sanity check the push rules are in the expected state after the upgrade + for _, client := range []*client.CSAPI{alice, alice2} { + pushRulesAfter := client.GetAllPushRules(t) + must.MatchGJSON(t, pushRulesAfter, + match.JSONCheckOff("global.room", []interface{}{ + roomID, + newRoomID, + }, + match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), + match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { + return match.JSONKeyEqual("actions.0", "dont_notify")(result) + }), + ), + ) + } }) - // Add some push rules - bob.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, - client.WithJSONBody(t, map[string]interface{}{ - "actions": []string{"dont_notify"}, - }), - ) - bob2.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, - client.WithJSONBody(t, map[string]interface{}{ - "actions": []string{"dont_notify"}, - }), - ) - - // Sanity check the push rules are in the expected state before the upgrade - for _, client := range []*client.CSAPI{bob, bob2} { - pushRulesBefore := client.GetAllPushRules(t) - must.MatchGJSON(t, pushRulesBefore, - match.JSONCheckOff("global.room", []interface{}{ - roomID, - }, - match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), - match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { - return match.JSONKeyEqual("actions.0", "dont_notify")(result) - }), - ), + // When a homeserver becomes aware of a room upgrade (upgrade is done on remote + // homeserver), it should copy over any existing push rules for all of its local users + // from the old room to the new room at the time of upgrade. + t.Run("joining a remote upgraded room carries over existing push rules", func(t *testing.T) { + t.Parallel() + + // Alice create a room + roomID := alice.MustCreateRoom(t, map[string]interface{}{ + "preset": "public_chat", + "room_version": "10", + }) + // Remote bob joins the room + bob.MustJoinRoom(t, roomID, []spec.ServerName{ + deployment.GetFullyQualifiedHomeserverName(t, "hs1"), + }) + // Remote bob2 joins the room + // + // We use two users to ensure that all users get taken care of (not just the first + // user on the homeserver). + bob2.MustJoinRoom(t, roomID, []spec.ServerName{ + deployment.GetFullyQualifiedHomeserverName(t, "hs1"), + }) + + // Add some push rules + bob.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, + client.WithJSONBody(t, map[string]interface{}{ + "actions": []string{"dont_notify"}, + }), + ) + bob2.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, + client.WithJSONBody(t, map[string]interface{}{ + "actions": []string{"dont_notify"}, + }), ) - } - - // Upgrade the room - newRoomID := alice.MustUpgradeRoom(t, roomID, "11") - // Remote bob joins the new room - bob.MustJoinRoom(t, newRoomID, []spec.ServerName{ - deployment.GetFullyQualifiedHomeserverName(t, "hs1"), - }) - // Remote bob2 joins the new room - bob2.MustJoinRoom(t, newRoomID, []spec.ServerName{ - deployment.GetFullyQualifiedHomeserverName(t, "hs1"), + // Sanity check the push rules are in the expected state before the upgrade + for _, client := range []*client.CSAPI{bob, bob2} { + pushRulesBefore := client.GetAllPushRules(t) + must.MatchGJSON(t, pushRulesBefore, + match.JSONCheckOff("global.room", []interface{}{ + roomID, + }, + match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), + match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { + return match.JSONKeyEqual("actions.0", "dont_notify")(result) + }), + ), + ) + } + + // Upgrade the room + newRoomID := alice.MustUpgradeRoom(t, roomID, "11") + + // Remote bob joins the new room + bob.MustJoinRoom(t, newRoomID, []spec.ServerName{ + deployment.GetFullyQualifiedHomeserverName(t, "hs1"), + }) + // Remote bob2 joins the new room + bob2.MustJoinRoom(t, newRoomID, []spec.ServerName{ + deployment.GetFullyQualifiedHomeserverName(t, "hs1"), + }) + + // Sanity check the push rules are in the expected state after the upgrade + for _, client := range []*client.CSAPI{bob, bob2} { + pushRulesAfter := client.GetAllPushRules(t) + must.MatchGJSON(t, pushRulesAfter, + match.JSONCheckOff("global.room", []interface{}{ + roomID, + newRoomID, + }, + match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), + match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { + return match.JSONKeyEqual("actions.0", "dont_notify")(result) + }), + ), + ) + } }) - - // Sanity check the push rules are in the expected state after the upgrade - for _, client := range []*client.CSAPI{bob, bob2} { - pushRulesAfter := client.GetAllPushRules(t) - must.MatchGJSON(t, pushRulesAfter, - match.JSONCheckOff("global.room", []interface{}{ - roomID, - newRoomID, - }, - match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), - match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { - return match.JSONKeyEqual("actions.0", "dont_notify")(result) - }), - ), - ) - } }) } From ad2a7eabb02960cadf22bd4db07299d70a8edc65 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 17 Nov 2025 17:40:41 -0600 Subject: [PATCH 08/21] More robust test (`MustAwaitPartialStateJoinCompletion`) See https://github.com/matrix-org/complement/pull/819#discussion_r2535770826 --- client/client.go | 15 +++++++++++++++ tests/csapi/room_upgrade_test.go | 12 ++++++++++++ 2 files changed, 27 insertions(+) diff --git a/client/client.go b/client/client.go index 04d8f355..0a1eb5ea 100644 --- a/client/client.go +++ b/client/client.go @@ -239,6 +239,21 @@ func (c *CSAPI) JoinRoom(t ct.TestLike, roomIDOrAlias string, serverNames []spec ) } +// MustAwaitPartialStateJoinCompletion waits until the joined room is no longer partial-stated +func (c *CSAPI) MustAwaitPartialStateJoinCompletion(t ct.TestLike, room_id string) { + t.Helper() + + // Use a `/members` request to wait for the room to be un-partial stated. + // We avoid using `/sync`, as it only waits (or used to wait) for full state at + // particular events, rather than the whole room. + c.MustDo( + t, + "GET", + []string{"_matrix", "client", "v3", "rooms", room_id, "members"}, + ) + t.Logf("%s's partial state join to %s completed.", c.UserID, room_id) +} + // MustLeaveRoom leaves the room ID, else fails the test. func (c *CSAPI) MustLeaveRoom(t ct.TestLike, roomID string) { res := c.LeaveRoom(t, roomID) diff --git a/tests/csapi/room_upgrade_test.go b/tests/csapi/room_upgrade_test.go index 4e06ea1f..9db304f4 100644 --- a/tests/csapi/room_upgrade_test.go +++ b/tests/csapi/room_upgrade_test.go @@ -95,6 +95,9 @@ func TestPushRuleRoomUpgrade(t *testing.T) { t.Run("joining a remote upgraded room carries over existing push rules", func(t *testing.T) { t.Parallel() + // Start a sync loop + _, bobSince := bob.MustSync(t, client.SyncReq{TimeoutMillis: "0"}) + // Alice create a room roomID := alice.MustCreateRoom(t, map[string]interface{}{ "preset": "public_chat", @@ -104,6 +107,15 @@ func TestPushRuleRoomUpgrade(t *testing.T) { bob.MustJoinRoom(t, roomID, []spec.ServerName{ deployment.GetFullyQualifiedHomeserverName(t, "hs1"), }) + // Wait until we know the first bob is joined for sure. We want to make sure bob2 + // doesn't also race us to remotely join the room as bob2 should be able to + // locally join and then send a join over federation (because the first bob is + // already joined to the room). + bobSince = bob.MustSyncUntil(t, client.SyncReq{Since: bobSince}, client.SyncJoinedTo(bob.UserID, roomID)) + // Wait until the homeserver is fully participating in the room so that we can + // double-check the subsequent joins also work (sanity check participating vs + // non-participating logic in the homeserver) + bob.MustAwaitPartialStateJoinCompletion(t, roomID) // Remote bob2 joins the room // // We use two users to ensure that all users get taken care of (not just the first From 5ebcc9fc00ee8349e569616fd98610c7807e58e8 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 17 Nov 2025 17:45:38 -0600 Subject: [PATCH 09/21] Even better test --- tests/csapi/room_upgrade_test.go | 33 ++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/tests/csapi/room_upgrade_test.go b/tests/csapi/room_upgrade_test.go index 9db304f4..6c94ed1f 100644 --- a/tests/csapi/room_upgrade_test.go +++ b/tests/csapi/room_upgrade_test.go @@ -120,9 +120,15 @@ func TestPushRuleRoomUpgrade(t *testing.T) { // // We use two users to ensure that all users get taken care of (not just the first // user on the homeserver). - bob2.MustJoinRoom(t, roomID, []spec.ServerName{ - deployment.GetFullyQualifiedHomeserverName(t, "hs1"), - }) + bob2.MustJoinRoom(t, roomID, + // bob2 can do a local join since bob is already in the room. No need to specify + // via servers here. + // + // []spec.ServerName{ + // deployment.GetFullyQualifiedHomeserverName(t, "hs1"), + // }, + nil, + ) // Add some push rules bob.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, @@ -158,10 +164,25 @@ func TestPushRuleRoomUpgrade(t *testing.T) { bob.MustJoinRoom(t, newRoomID, []spec.ServerName{ deployment.GetFullyQualifiedHomeserverName(t, "hs1"), }) + // Wait until we know the first bob is joined for sure. We want to make sure bob2 + // doesn't also race us to remotely join the room as bob2 should be able to + // locally join and then send a join over federation (because the first bob is + // already joined to the room). + bobSince = bob.MustSyncUntil(t, client.SyncReq{Since: bobSince}, client.SyncJoinedTo(bob.UserID, newRoomID)) + // Wait until the homeserver is fully participating in the room so that we can + // double-check the subsequent joins also work (sanity check participating vs + // non-participating logic in the homeserver) + bob.MustAwaitPartialStateJoinCompletion(t, newRoomID) // Remote bob2 joins the new room - bob2.MustJoinRoom(t, newRoomID, []spec.ServerName{ - deployment.GetFullyQualifiedHomeserverName(t, "hs1"), - }) + bob2.MustJoinRoom(t, newRoomID, + // bob2 can do a local join since bob is already in the room. No need to specify + // via servers here. + // + // []spec.ServerName{ + // deployment.GetFullyQualifiedHomeserverName(t, "hs1"), + // }, + nil, + ) // Sanity check the push rules are in the expected state after the upgrade for _, client := range []*client.CSAPI{bob, bob2} { From e71b03e5567471a39c0176eb879c3e57e19ee1cb Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 17 Nov 2025 18:28:00 -0600 Subject: [PATCH 10/21] Add manually upgraded variant and more robust --- tests/csapi/room_upgrade_test.go | 402 ++++++++++++++++++------------- 1 file changed, 232 insertions(+), 170 deletions(-) diff --git a/tests/csapi/room_upgrade_test.go b/tests/csapi/room_upgrade_test.go index 6c94ed1f..27a095d0 100644 --- a/tests/csapi/room_upgrade_test.go +++ b/tests/csapi/room_upgrade_test.go @@ -16,189 +16,251 @@ func TestPushRuleRoomUpgrade(t *testing.T) { deployment := complement.Deploy(t, 2) defer deployment.Destroy(t) - alice := deployment.Register(t, "hs1", helpers.RegistrationOpts{}) - alice2 := deployment.Register(t, "hs1", helpers.RegistrationOpts{}) - bob := deployment.Register(t, "hs2", helpers.RegistrationOpts{}) - bob2 := deployment.Register(t, "hs2", helpers.RegistrationOpts{}) + alice := deployment.Register(t, "hs1", helpers.RegistrationOpts{ + LocalpartSuffix: "alice", + }) + alice2 := deployment.Register(t, "hs1", helpers.RegistrationOpts{ + LocalpartSuffix: "alice2", + }) + bob := deployment.Register(t, "hs2", helpers.RegistrationOpts{ + LocalpartSuffix: "bob", + }) + bob2 := deployment.Register(t, "hs2", helpers.RegistrationOpts{ + LocalpartSuffix: "bob2", + }) t.Run("parallel", func(t *testing.T) { - // When a homeserver becomes aware of a room upgrade (upgrade is done on local - // homeserver), it should copy over any existing push rules for all of its local users - // from the old room to the new room at the time of upgrade. - t.Run("upgrading a room carries over existing push rules for local users", func(t *testing.T) { - t.Parallel() - - // Create a room - roomID := alice.MustCreateRoom(t, map[string]interface{}{ - "preset": "public_chat", - "room_version": "10", - }) - // Have alice2 join the room - // - // We use two users to ensure that all users get taken care of (not just the person - // upgrading the room). - alice2.MustJoinRoom(t, roomID, nil) - - // Add some push rules - alice.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, - client.WithJSONBody(t, map[string]interface{}{ - "actions": []string{"dont_notify"}, - }), - ) - alice2.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, - client.WithJSONBody(t, map[string]interface{}{ - "actions": []string{"dont_notify"}, - }), - ) - - // Sanity check the push rules are in the expected state before the upgrade - for _, client := range []*client.CSAPI{alice, alice2} { - pushRulesBefore := client.GetAllPushRules(t) - must.MatchGJSON(t, pushRulesBefore, - match.JSONCheckOff("global.room", []interface{}{ - roomID, - }, - match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), - match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { - return match.JSONKeyEqual("actions.0", "dont_notify")(result) - }), - ), - ) + for _, useManualRoomUpgrade := range []bool{true, false} { + upgradeDescritorPrefix := "" + if useManualRoomUpgrade { + upgradeDescritorPrefix = "manually " } - // Upgrade the room - newRoomID := alice.MustUpgradeRoom(t, roomID, "11") - - // Alice2 joins the new room - alice2.MustJoinRoom(t, newRoomID, nil) - - // Sanity check the push rules are in the expected state after the upgrade - for _, client := range []*client.CSAPI{alice, alice2} { - pushRulesAfter := client.GetAllPushRules(t) - must.MatchGJSON(t, pushRulesAfter, - match.JSONCheckOff("global.room", []interface{}{ - roomID, - newRoomID, - }, - match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), - match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { - return match.JSONKeyEqual("actions.0", "dont_notify")(result) - }), - ), + // When a homeserver becomes aware of a room upgrade (upgrade is done on local + // homeserver), it should copy over any existing push rules for all of its local users + // from the old room to the new room at the time of upgrade. + t.Run(upgradeDescritorPrefix+"upgrading a room carries over existing push rules for local users", func(t *testing.T) { + t.Parallel() + + // Create a room + roomID := alice.MustCreateRoom(t, map[string]interface{}{ + "preset": "public_chat", + "room_version": "10", + }) + // Have alice2 join the room + // + // We use two users to ensure that all users get taken care of (not just the person + // upgrading the room). + alice2.MustJoinRoom(t, roomID, nil) + + // Add some push rules + alice.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, + client.WithJSONBody(t, map[string]interface{}{ + "actions": []string{"dont_notify"}, + }), + ) + alice2.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, + client.WithJSONBody(t, map[string]interface{}{ + "actions": []string{"dont_notify"}, + }), ) - } - }) - // When a homeserver becomes aware of a room upgrade (upgrade is done on remote - // homeserver), it should copy over any existing push rules for all of its local users - // from the old room to the new room at the time of upgrade. - t.Run("joining a remote upgraded room carries over existing push rules", func(t *testing.T) { - t.Parallel() + // Sanity check the push rules are in the expected state before the upgrade + for _, client := range []*client.CSAPI{alice, alice2} { + t.Logf("Checking push rules (before upgrade) for %s", client.UserID) + pushRulesBefore := client.GetAllPushRules(t) + must.MatchGJSON(t, pushRulesBefore, + match.JSONCheckOff("global.room", + []interface{}{ + roomID, + }, + match.CheckOffAllowUnwanted(), + match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), + match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { + return match.JSONKeyEqual("actions.0", "dont_notify")(result) + }), + ), + ) + } - // Start a sync loop - _, bobSince := bob.MustSync(t, client.SyncReq{TimeoutMillis: "0"}) + // Upgrade the room + var newRoomID string + if useManualRoomUpgrade { + newRoomID = mustManualUpgradeRoom(t, alice, roomID, "11") + } else { + newRoomID = alice.MustUpgradeRoom(t, roomID, "11") + } - // Alice create a room - roomID := alice.MustCreateRoom(t, map[string]interface{}{ - "preset": "public_chat", - "room_version": "10", - }) - // Remote bob joins the room - bob.MustJoinRoom(t, roomID, []spec.ServerName{ - deployment.GetFullyQualifiedHomeserverName(t, "hs1"), + // Alice2 joins the new room + alice2.MustJoinRoom(t, newRoomID, nil) + + // Sanity check the push rules are in the expected state after the upgrade + for _, client := range []*client.CSAPI{alice, alice2} { + t.Logf("Checking push rules (after upgrade) for %s", client.UserID) + pushRulesAfter := client.GetAllPushRules(t) + must.MatchGJSON(t, pushRulesAfter, + match.JSONCheckOff("global.room", + []interface{}{ + roomID, + newRoomID, + }, + match.CheckOffAllowUnwanted(), + match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), + match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { + return match.JSONKeyEqual("actions.0", "dont_notify")(result) + }), + ), + ) + } }) - // Wait until we know the first bob is joined for sure. We want to make sure bob2 - // doesn't also race us to remotely join the room as bob2 should be able to - // locally join and then send a join over federation (because the first bob is - // already joined to the room). - bobSince = bob.MustSyncUntil(t, client.SyncReq{Since: bobSince}, client.SyncJoinedTo(bob.UserID, roomID)) - // Wait until the homeserver is fully participating in the room so that we can - // double-check the subsequent joins also work (sanity check participating vs - // non-participating logic in the homeserver) - bob.MustAwaitPartialStateJoinCompletion(t, roomID) - // Remote bob2 joins the room - // - // We use two users to ensure that all users get taken care of (not just the first - // user on the homeserver). - bob2.MustJoinRoom(t, roomID, - // bob2 can do a local join since bob is already in the room. No need to specify - // via servers here. + + // When a homeserver becomes aware of a room upgrade (upgrade is done on remote + // homeserver), it should copy over any existing push rules for all of its local users + // from the old room to the new room at the time of upgrade. + t.Run("joining a remote "+upgradeDescritorPrefix+"upgraded room carries over existing push rules", func(t *testing.T) { + t.Parallel() + + // Start a sync loop + _, bobSince := bob.MustSync(t, client.SyncReq{TimeoutMillis: "0"}) + + // Alice create a room + roomID := alice.MustCreateRoom(t, map[string]interface{}{ + "preset": "public_chat", + "room_version": "10", + }) + // Remote bob joins the room + bob.MustJoinRoom(t, roomID, []spec.ServerName{ + deployment.GetFullyQualifiedHomeserverName(t, "hs1"), + }) + // Wait until the homeserver is fully participating in the room so that we can + // double-check the subsequent joins also work (sanity check participating vs + // non-participating logic in the homeserver) + bob.MustAwaitPartialStateJoinCompletion(t, roomID) + // Wait until we know the first bob is joined for sure. We want to make sure bob2 + // doesn't also race us to remotely join the room as bob2 should be able to + // locally join and then send a join over federation (because the first bob is + // already joined to the room). + bobSince = bob.MustSyncUntil(t, client.SyncReq{Since: bobSince}, client.SyncJoinedTo(bob.UserID, roomID)) + // Remote bob2 joins the room // - // []spec.ServerName{ - // deployment.GetFullyQualifiedHomeserverName(t, "hs1"), - // }, - nil, - ) - - // Add some push rules - bob.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, - client.WithJSONBody(t, map[string]interface{}{ - "actions": []string{"dont_notify"}, - }), - ) - bob2.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, - client.WithJSONBody(t, map[string]interface{}{ - "actions": []string{"dont_notify"}, - }), - ) - - // Sanity check the push rules are in the expected state before the upgrade - for _, client := range []*client.CSAPI{bob, bob2} { - pushRulesBefore := client.GetAllPushRules(t) - must.MatchGJSON(t, pushRulesBefore, - match.JSONCheckOff("global.room", []interface{}{ - roomID, - }, - match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), - match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { - return match.JSONKeyEqual("actions.0", "dont_notify")(result) - }), - ), + // We use two users to ensure that all users get taken care of (not just the first + // user on the homeserver). + bob2.MustJoinRoom(t, roomID, + // bob2 can do a local join since bob is already in the room. No need to specify + // via servers here. + // + // []spec.ServerName{ + // deployment.GetFullyQualifiedHomeserverName(t, "hs1"), + // }, + nil, ) - } - // Upgrade the room - newRoomID := alice.MustUpgradeRoom(t, roomID, "11") + // Add some push rules + bob.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, + client.WithJSONBody(t, map[string]interface{}{ + "actions": []string{"dont_notify"}, + }), + ) + bob2.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, + client.WithJSONBody(t, map[string]interface{}{ + "actions": []string{"dont_notify"}, + }), + ) - // Remote bob joins the new room - bob.MustJoinRoom(t, newRoomID, []spec.ServerName{ - deployment.GetFullyQualifiedHomeserverName(t, "hs1"), - }) - // Wait until we know the first bob is joined for sure. We want to make sure bob2 - // doesn't also race us to remotely join the room as bob2 should be able to - // locally join and then send a join over federation (because the first bob is - // already joined to the room). - bobSince = bob.MustSyncUntil(t, client.SyncReq{Since: bobSince}, client.SyncJoinedTo(bob.UserID, newRoomID)) - // Wait until the homeserver is fully participating in the room so that we can - // double-check the subsequent joins also work (sanity check participating vs - // non-participating logic in the homeserver) - bob.MustAwaitPartialStateJoinCompletion(t, newRoomID) - // Remote bob2 joins the new room - bob2.MustJoinRoom(t, newRoomID, - // bob2 can do a local join since bob is already in the room. No need to specify - // via servers here. - // - // []spec.ServerName{ - // deployment.GetFullyQualifiedHomeserverName(t, "hs1"), - // }, - nil, - ) - - // Sanity check the push rules are in the expected state after the upgrade - for _, client := range []*client.CSAPI{bob, bob2} { - pushRulesAfter := client.GetAllPushRules(t) - must.MatchGJSON(t, pushRulesAfter, - match.JSONCheckOff("global.room", []interface{}{ - roomID, - newRoomID, - }, - match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), - match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { - return match.JSONKeyEqual("actions.0", "dont_notify")(result) - }), - ), + // Sanity check the push rules are in the expected state before the upgrade + for _, client := range []*client.CSAPI{bob, bob2} { + t.Logf("Checking push rules (before upgrade) for %s", client.UserID) + pushRulesBefore := client.GetAllPushRules(t) + must.MatchGJSON(t, pushRulesBefore, + match.JSONCheckOff("global.room", + []interface{}{ + roomID, + }, + match.CheckOffAllowUnwanted(), + match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), + match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { + return match.JSONKeyEqual("actions.0", "dont_notify")(result) + }), + ), + ) + } + + // Upgrade the room + var newRoomID string + if useManualRoomUpgrade { + newRoomID = mustManualUpgradeRoom(t, alice, roomID, "11") + } else { + newRoomID = alice.MustUpgradeRoom(t, roomID, "11") + } + + // Ensure that the remote server sees the tombstone in the old room before + // joining the new room (avoid races and the client woudn't know where to go + // without this hint anyway) + bobSince = bob.MustSyncUntil(t, client.SyncReq{Since: bobSince}, client.SyncTimelineHas(roomID, func(ev gjson.Result) bool { + return ev.Get("type").Str == "m.room.tombstone" && ev.Get("state_key").Str == "" + })) + + // Remote bob joins the new room + bob.MustJoinRoom(t, newRoomID, []spec.ServerName{ + deployment.GetFullyQualifiedHomeserverName(t, "hs1"), + }) + // Wait until the homeserver is fully participating in the room so that we can + // double-check the subsequent joins also work (sanity check participating vs + // non-participating logic in the homeserver) + bob.MustAwaitPartialStateJoinCompletion(t, newRoomID) + // Wait until we know the first bob is joined for sure. We want to make sure bob2 + // doesn't also race us to remotely join the room as bob2 should be able to + // locally join and then send a join over federation (because the first bob is + // already joined to the room). + bobSince = bob.MustSyncUntil(t, client.SyncReq{Since: bobSince}, client.SyncJoinedTo(bob.UserID, newRoomID)) + // Remote bob2 joins the new room + bob2.MustJoinRoom(t, newRoomID, + // bob2 can do a local join since bob is already in the room. No need to specify + // via servers here. + // + // []spec.ServerName{ + // deployment.GetFullyQualifiedHomeserverName(t, "hs1"), + // }, + nil, ) - } - }) + + // Sanity check the push rules are in the expected state after the upgrade + for _, client := range []*client.CSAPI{bob, bob2} { + pushRulesAfter := client.GetAllPushRules(t) + t.Logf("Checking push rules (after upgrade) for %s", client.UserID) + must.MatchGJSON(t, pushRulesAfter, + match.JSONCheckOff("global.room", + []interface{}{ + roomID, + newRoomID, + }, + match.CheckOffAllowUnwanted(), + match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), + match.CheckOffForEach(func(roomIDFromPushRule interface{}, result gjson.Result) error { + return match.JSONKeyEqual("actions.0", "dont_notify")(result) + }), + ), + ) + } + }) + } + }) +} + +func mustManualUpgradeRoom(t *testing.T, c *client.CSAPI, oldRoomID string, newRoomVersion string) string { + t.Helper() + + // Create a new room + newRoomID := c.MustCreateRoom(t, map[string]interface{}{ + "preset": "public_chat", + "room_version": newRoomVersion, }) + + // Send the m.room.tombstone event to the old room + c.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "rooms", oldRoomID, "state", "m.room.tombstone", ""}, client.WithJSONBody(t, map[string]interface{}{ + "body": "This room has been replaced", + "replacement_room": newRoomID, + })) + + return newRoomID } From 4fb255c4d409c7e7c0f2fde78f3b32c0cecd32fd Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 17 Nov 2025 18:40:36 -0600 Subject: [PATCH 11/21] Specify predecessor when manually upgrading room Synapse looks for the connection in order to run the upgrade logic for remote room joins --- tests/csapi/room_upgrade_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/csapi/room_upgrade_test.go b/tests/csapi/room_upgrade_test.go index 27a095d0..310862b2 100644 --- a/tests/csapi/room_upgrade_test.go +++ b/tests/csapi/room_upgrade_test.go @@ -90,6 +90,7 @@ func TestPushRuleRoomUpgrade(t *testing.T) { } else { newRoomID = alice.MustUpgradeRoom(t, roomID, "11") } + t.Logf("Upgraded room %s to %s", roomID, newRoomID) // Alice2 joins the new room alice2.MustJoinRoom(t, newRoomID, nil) @@ -192,6 +193,7 @@ func TestPushRuleRoomUpgrade(t *testing.T) { } else { newRoomID = alice.MustUpgradeRoom(t, roomID, "11") } + t.Logf("Upgraded room %s to %s", roomID, newRoomID) // Ensure that the remote server sees the tombstone in the old room before // joining the new room (avoid races and the client woudn't know where to go @@ -254,6 +256,12 @@ func mustManualUpgradeRoom(t *testing.T, c *client.CSAPI, oldRoomID string, newR newRoomID := c.MustCreateRoom(t, map[string]interface{}{ "preset": "public_chat", "room_version": newRoomVersion, + "creation_content": map[string]interface{}{ + // Specify the old room as the predecessor + "predecessor": map[string]interface{}{ + "room_id": oldRoomID, + }, + }, }) // Send the m.room.tombstone event to the old room From 369250ae2d13f248e873b30a59a368a6efe66ba7 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 17 Nov 2025 18:51:17 -0600 Subject: [PATCH 12/21] Skip test until Synapse is fixed See https://github.com/element-hq/synapse/issues/19199 --- tests/csapi/room_upgrade_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/csapi/room_upgrade_test.go b/tests/csapi/room_upgrade_test.go index 310862b2..0f9a628f 100644 --- a/tests/csapi/room_upgrade_test.go +++ b/tests/csapi/room_upgrade_test.go @@ -8,6 +8,7 @@ import ( "github.com/matrix-org/complement/helpers" "github.com/matrix-org/complement/match" "github.com/matrix-org/complement/must" + "github.com/matrix-org/complement/runtime" "github.com/matrix-org/gomatrixserverlib/spec" "github.com/tidwall/gjson" ) @@ -42,6 +43,12 @@ func TestPushRuleRoomUpgrade(t *testing.T) { t.Run(upgradeDescritorPrefix+"upgrading a room carries over existing push rules for local users", func(t *testing.T) { t.Parallel() + // FIXME: We have to skip this test on Synapse until + // https://github.com/element-hq/synapse/issues/19199 is resolved. + if useManualRoomUpgrade { + runtime.SkipIf(t, runtime.Synapse) + } + // Create a room roomID := alice.MustCreateRoom(t, map[string]interface{}{ "preset": "public_chat", From 3b4ca0b215f6c3be6d4c488688aaabeb19a8acd9 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 18 Nov 2025 12:06:37 -0600 Subject: [PATCH 13/21] Use `SetPushRule` --- tests/csapi/room_upgrade_test.go | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/tests/csapi/room_upgrade_test.go b/tests/csapi/room_upgrade_test.go index 0f9a628f..06985f22 100644 --- a/tests/csapi/room_upgrade_test.go +++ b/tests/csapi/room_upgrade_test.go @@ -61,16 +61,12 @@ func TestPushRuleRoomUpgrade(t *testing.T) { alice2.MustJoinRoom(t, roomID, nil) // Add some push rules - alice.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, - client.WithJSONBody(t, map[string]interface{}{ - "actions": []string{"dont_notify"}, - }), - ) - alice2.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, - client.WithJSONBody(t, map[string]interface{}{ - "actions": []string{"dont_notify"}, - }), - ) + alice.SetPushRule(t, "global", "room", roomID, map[string]interface{}{ + "actions": []string{"dont_notify"}, + }, "", "") + alice2.SetPushRule(t, "global", "room", roomID, map[string]interface{}{ + "actions": []string{"dont_notify"}, + }, "", "") // Sanity check the push rules are in the expected state before the upgrade for _, client := range []*client.CSAPI{alice, alice2} { @@ -164,16 +160,12 @@ func TestPushRuleRoomUpgrade(t *testing.T) { ) // Add some push rules - bob.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, - client.WithJSONBody(t, map[string]interface{}{ - "actions": []string{"dont_notify"}, - }), - ) - bob2.MustDo(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", roomID}, - client.WithJSONBody(t, map[string]interface{}{ - "actions": []string{"dont_notify"}, - }), - ) + bob.SetPushRule(t, "global", "room", roomID, map[string]interface{}{ + "actions": []string{"dont_notify"}, + }, "", "") + bob2.SetPushRule(t, "global", "room", roomID, map[string]interface{}{ + "actions": []string{"dont_notify"}, + }, "", "") // Sanity check the push rules are in the expected state before the upgrade for _, client := range []*client.CSAPI{bob, bob2} { From 6e2c6f2b84769a16403dcf5a3e73829f6a2c0f83 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 18 Nov 2025 12:57:35 -0600 Subject: [PATCH 14/21] Wait/sync until push rules show up See https://github.com/matrix-org/complement/pull/819#discussion_r2536014635 --- tests/csapi/room_upgrade_test.go | 96 ++++++++++++++++++++++++++++---- 1 file changed, 84 insertions(+), 12 deletions(-) diff --git a/tests/csapi/room_upgrade_test.go b/tests/csapi/room_upgrade_test.go index 06985f22..aa38a5ec 100644 --- a/tests/csapi/room_upgrade_test.go +++ b/tests/csapi/room_upgrade_test.go @@ -43,6 +43,10 @@ func TestPushRuleRoomUpgrade(t *testing.T) { t.Run(upgradeDescritorPrefix+"upgrading a room carries over existing push rules for local users", func(t *testing.T) { t.Parallel() + // Start a sync loop + _, aliceSince := alice.MustSync(t, client.SyncReq{TimeoutMillis: "0"}) + _, alice2Since := alice2.MustSync(t, client.SyncReq{TimeoutMillis: "0"}) + // FIXME: We have to skip this test on Synapse until // https://github.com/element-hq/synapse/issues/19199 is resolved. if useManualRoomUpgrade { @@ -69,9 +73,18 @@ func TestPushRuleRoomUpgrade(t *testing.T) { }, "", "") // Sanity check the push rules are in the expected state before the upgrade - for _, client := range []*client.CSAPI{alice, alice2} { - t.Logf("Checking push rules (before upgrade) for %s", client.UserID) - pushRulesBefore := client.GetAllPushRules(t) + for _, clientTokenPair := range []struct { + *client.CSAPI + *string + }{{alice, &aliceSince}, {alice2, &alice2Since}} { + userClient := clientTokenPair.CSAPI + sinceToken := clientTokenPair.string + t.Logf("Checking push rules (before upgrade) for %s", userClient.UserID) + // Wait until the new push rules show up in /sync + *sinceToken = userClient.MustSyncUntil(t, client.SyncReq{Since: *sinceToken}, syncGlobalAccountDataHasPushRuleForRoomID(roomID)) + + // Verify that the push rules are as expected + pushRulesBefore := userClient.GetAllPushRules(t) must.MatchGJSON(t, pushRulesBefore, match.JSONCheckOff("global.room", []interface{}{ @@ -99,9 +112,21 @@ func TestPushRuleRoomUpgrade(t *testing.T) { alice2.MustJoinRoom(t, newRoomID, nil) // Sanity check the push rules are in the expected state after the upgrade - for _, client := range []*client.CSAPI{alice, alice2} { - t.Logf("Checking push rules (after upgrade) for %s", client.UserID) - pushRulesAfter := client.GetAllPushRules(t) + for _, clientTokenPair := range []struct { + *client.CSAPI + *string + }{{alice, &aliceSince}, {alice2, &alice2Since}} { + userClient := clientTokenPair.CSAPI + sinceToken := clientTokenPair.string + t.Logf("Checking push rules (after upgrade) for %s", userClient.UserID) + // Wait until the new push rules show up in /sync + *sinceToken = userClient.MustSyncUntil(t, client.SyncReq{Since: *sinceToken}, + syncGlobalAccountDataHasPushRuleForRoomID(roomID), + syncGlobalAccountDataHasPushRuleForRoomID(newRoomID), + ) + + // Verify that the push rules are as expected + pushRulesAfter := userClient.GetAllPushRules(t) must.MatchGJSON(t, pushRulesAfter, match.JSONCheckOff("global.room", []interface{}{ @@ -126,6 +151,7 @@ func TestPushRuleRoomUpgrade(t *testing.T) { // Start a sync loop _, bobSince := bob.MustSync(t, client.SyncReq{TimeoutMillis: "0"}) + _, bob2Since := bob2.MustSync(t, client.SyncReq{TimeoutMillis: "0"}) // Alice create a room roomID := alice.MustCreateRoom(t, map[string]interface{}{ @@ -168,9 +194,18 @@ func TestPushRuleRoomUpgrade(t *testing.T) { }, "", "") // Sanity check the push rules are in the expected state before the upgrade - for _, client := range []*client.CSAPI{bob, bob2} { - t.Logf("Checking push rules (before upgrade) for %s", client.UserID) - pushRulesBefore := client.GetAllPushRules(t) + for _, clientTokenPair := range []struct { + *client.CSAPI + *string + }{{bob, &bobSince}, {bob2, &bob2Since}} { + userClient := clientTokenPair.CSAPI + sinceToken := clientTokenPair.string + t.Logf("Checking push rules (before upgrade) for %s", userClient.UserID) + // Wait until the new push rules show up in /sync + *sinceToken = userClient.MustSyncUntil(t, client.SyncReq{Since: *sinceToken}, syncGlobalAccountDataHasPushRuleForRoomID(roomID)) + + // Verify that the push rules are as expected + pushRulesBefore := userClient.GetAllPushRules(t) must.MatchGJSON(t, pushRulesBefore, match.JSONCheckOff("global.room", []interface{}{ @@ -226,9 +261,21 @@ func TestPushRuleRoomUpgrade(t *testing.T) { ) // Sanity check the push rules are in the expected state after the upgrade - for _, client := range []*client.CSAPI{bob, bob2} { - pushRulesAfter := client.GetAllPushRules(t) - t.Logf("Checking push rules (after upgrade) for %s", client.UserID) + for _, clientTokenPair := range []struct { + *client.CSAPI + *string + }{{bob, &bobSince}, {bob2, &bob2Since}} { + userClient := clientTokenPair.CSAPI + sinceToken := clientTokenPair.string + t.Logf("Checking push rules (after upgrade) for %s", userClient.UserID) + // Wait until the new push rules show up in /sync + *sinceToken = userClient.MustSyncUntil(t, client.SyncReq{Since: *sinceToken}, + syncGlobalAccountDataHasPushRuleForRoomID(roomID), + syncGlobalAccountDataHasPushRuleForRoomID(newRoomID), + ) + + // Verify that the push rules are as expected + pushRulesAfter := userClient.GetAllPushRules(t) must.MatchGJSON(t, pushRulesAfter, match.JSONCheckOff("global.room", []interface{}{ @@ -271,3 +318,28 @@ func mustManualUpgradeRoom(t *testing.T, c *client.CSAPI, oldRoomID string, newR return newRoomID } + +// syncGlobalAccountDataHasPushRuleForRoomID waits for the global account data to +// include a push rule for the given room ID +func syncGlobalAccountDataHasPushRuleForRoomID(roomID string) client.SyncCheckOpt { + return client.SyncGlobalAccountDataHas(func(globalAccountDataEvent gjson.Result) bool { + if globalAccountDataEvent.Get("type").Str != "m.push_rules" { + return false + } + pushRules := globalAccountDataEvent + + matcher := match.JSONCheckOff("content.global.room", + []interface{}{ + roomID, + }, + match.CheckOffAllowUnwanted(), + match.CheckOffMapper(func(r gjson.Result) interface{} { return r.Get("rule_id").Str }), + ) + + if err := matcher(pushRules); err != nil { + return false + } + + return true + }) +} From 10755c222a9bf3ee8d95e608bcc5c5ca50e93762 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 18 Nov 2025 14:59:11 -0600 Subject: [PATCH 15/21] Fix waiting since the correct spot --- tests/csapi/room_upgrade_test.go | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/tests/csapi/room_upgrade_test.go b/tests/csapi/room_upgrade_test.go index aa38a5ec..54182d42 100644 --- a/tests/csapi/room_upgrade_test.go +++ b/tests/csapi/room_upgrade_test.go @@ -99,6 +99,15 @@ func TestPushRuleRoomUpgrade(t *testing.T) { ) } + // Because the push rules can be copied over as part of the "upgrade", we need a + // sync token just before so we can check since that point. + // + // Note: In this test, this is redundant because we don't sync after the upgrade + // until our check happens but I'm just keeping the shape similar to other test + // below. + aliceSinceBeforeUpgrade := aliceSince + alice2SinceBeforeUpgrade := alice2Since + // Upgrade the room var newRoomID string if useManualRoomUpgrade { @@ -115,7 +124,11 @@ func TestPushRuleRoomUpgrade(t *testing.T) { for _, clientTokenPair := range []struct { *client.CSAPI *string - }{{alice, &aliceSince}, {alice2, &alice2Since}} { + }{ + // Since the upgrade, wait until the new push rules show up in /sync + {alice, &aliceSinceBeforeUpgrade}, + {alice2, &alice2SinceBeforeUpgrade}, + } { userClient := clientTokenPair.CSAPI sinceToken := clientTokenPair.string t.Logf("Checking push rules (after upgrade) for %s", userClient.UserID) @@ -220,6 +233,11 @@ func TestPushRuleRoomUpgrade(t *testing.T) { ) } + // Because the push rules can be copied over as part of the "upgrade", we need a + // sync token just before so we can check since that point. + bobSinceBeforeUpgrade := bobSince + bob2SinceBeforeUpgrade := bob2Since + // Upgrade the room var newRoomID string if useManualRoomUpgrade { @@ -264,7 +282,11 @@ func TestPushRuleRoomUpgrade(t *testing.T) { for _, clientTokenPair := range []struct { *client.CSAPI *string - }{{bob, &bobSince}, {bob2, &bob2Since}} { + }{ + // Since the upgrade, wait until the new push rules show up in /sync + {bob, &bobSinceBeforeUpgrade}, + {bob2, &bob2SinceBeforeUpgrade}, + } { userClient := clientTokenPair.CSAPI sinceToken := clientTokenPair.string t.Logf("Checking push rules (after upgrade) for %s", userClient.UserID) From 7a601fa83278d119e2ee162d0b1b958510933a1c Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 18 Nov 2025 16:34:51 -0600 Subject: [PATCH 16/21] Skip earlier --- tests/csapi/room_upgrade_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/csapi/room_upgrade_test.go b/tests/csapi/room_upgrade_test.go index 54182d42..c291e821 100644 --- a/tests/csapi/room_upgrade_test.go +++ b/tests/csapi/room_upgrade_test.go @@ -43,16 +43,16 @@ func TestPushRuleRoomUpgrade(t *testing.T) { t.Run(upgradeDescritorPrefix+"upgrading a room carries over existing push rules for local users", func(t *testing.T) { t.Parallel() - // Start a sync loop - _, aliceSince := alice.MustSync(t, client.SyncReq{TimeoutMillis: "0"}) - _, alice2Since := alice2.MustSync(t, client.SyncReq{TimeoutMillis: "0"}) - // FIXME: We have to skip this test on Synapse until // https://github.com/element-hq/synapse/issues/19199 is resolved. if useManualRoomUpgrade { runtime.SkipIf(t, runtime.Synapse) } + // Start a sync loop + _, aliceSince := alice.MustSync(t, client.SyncReq{TimeoutMillis: "0"}) + _, alice2Since := alice2.MustSync(t, client.SyncReq{TimeoutMillis: "0"}) + // Create a room roomID := alice.MustCreateRoom(t, map[string]interface{}{ "preset": "public_chat", From 201a93f920326688347f0f69fded5ed1fbf21d49 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 18 Nov 2025 16:36:39 -0600 Subject: [PATCH 17/21] Skip on Dendrite See https://github.com/matrix-org/complement/pull/819#discussion_r2536014635 --- tests/csapi/room_upgrade_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/csapi/room_upgrade_test.go b/tests/csapi/room_upgrade_test.go index c291e821..034ec039 100644 --- a/tests/csapi/room_upgrade_test.go +++ b/tests/csapi/room_upgrade_test.go @@ -43,6 +43,10 @@ func TestPushRuleRoomUpgrade(t *testing.T) { t.Run(upgradeDescritorPrefix+"upgrading a room carries over existing push rules for local users", func(t *testing.T) { t.Parallel() + // FIXME: We have to skip this test on Dendrite because it doesn't seem to send + // the new push rules down sync in many scenarios. + runtime.SkipIf(t, runtime.Dendrite) + // FIXME: We have to skip this test on Synapse until // https://github.com/element-hq/synapse/issues/19199 is resolved. if useManualRoomUpgrade { @@ -162,6 +166,12 @@ func TestPushRuleRoomUpgrade(t *testing.T) { t.Run("joining a remote "+upgradeDescritorPrefix+"upgraded room carries over existing push rules", func(t *testing.T) { t.Parallel() + // FIXME: We have to skip this test on Dendrite because it doesn't seem to send + // the new push rules down sync in many scenarios. + if useManualRoomUpgrade { + runtime.SkipIf(t, runtime.Dendrite) + } + // Start a sync loop _, bobSince := bob.MustSync(t, client.SyncReq{TimeoutMillis: "0"}) _, bob2Since := bob2.MustSync(t, client.SyncReq{TimeoutMillis: "0"}) From 126cc9b850f6d5ea0854221f2dcc2ba1fd44fc9b Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 18 Nov 2025 16:44:44 -0600 Subject: [PATCH 18/21] This does sometimes pass but skip since not reliable --- tests/csapi/room_upgrade_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/csapi/room_upgrade_test.go b/tests/csapi/room_upgrade_test.go index 034ec039..4ca665bf 100644 --- a/tests/csapi/room_upgrade_test.go +++ b/tests/csapi/room_upgrade_test.go @@ -168,9 +168,7 @@ func TestPushRuleRoomUpgrade(t *testing.T) { // FIXME: We have to skip this test on Dendrite because it doesn't seem to send // the new push rules down sync in many scenarios. - if useManualRoomUpgrade { - runtime.SkipIf(t, runtime.Dendrite) - } + runtime.SkipIf(t, runtime.Dendrite) // Start a sync loop _, bobSince := bob.MustSync(t, client.SyncReq{TimeoutMillis: "0"}) From 32aff46740c1e59840d44325b6cbc57531becc53 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 20 Nov 2025 11:55:06 -0600 Subject: [PATCH 19/21] Accurate plural language See https://github.com/matrix-org/complement/pull/819#discussion_r2545535746 --- tests/csapi/room_upgrade_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/csapi/room_upgrade_test.go b/tests/csapi/room_upgrade_test.go index 4ca665bf..6936bec0 100644 --- a/tests/csapi/room_upgrade_test.go +++ b/tests/csapi/room_upgrade_test.go @@ -107,7 +107,7 @@ func TestPushRuleRoomUpgrade(t *testing.T) { // sync token just before so we can check since that point. // // Note: In this test, this is redundant because we don't sync after the upgrade - // until our check happens but I'm just keeping the shape similar to other test + // until our check happens but I'm just keeping the shape similar to other tests // below. aliceSinceBeforeUpgrade := aliceSince alice2SinceBeforeUpgrade := alice2Since From bae2b742fef68906806dafdf1971fde66cb7f8ef Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 20 Nov 2025 11:55:47 -0600 Subject: [PATCH 20/21] `upgradeDescriptorPrefix` typo See https://github.com/matrix-org/complement/pull/819#discussion_r2545528875 --- tests/csapi/room_upgrade_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/csapi/room_upgrade_test.go b/tests/csapi/room_upgrade_test.go index 6936bec0..4a649a61 100644 --- a/tests/csapi/room_upgrade_test.go +++ b/tests/csapi/room_upgrade_test.go @@ -32,15 +32,15 @@ func TestPushRuleRoomUpgrade(t *testing.T) { t.Run("parallel", func(t *testing.T) { for _, useManualRoomUpgrade := range []bool{true, false} { - upgradeDescritorPrefix := "" + upgradeDescriptorPrefix := "" if useManualRoomUpgrade { - upgradeDescritorPrefix = "manually " + upgradeDescriptorPrefix = "manually " } // When a homeserver becomes aware of a room upgrade (upgrade is done on local // homeserver), it should copy over any existing push rules for all of its local users // from the old room to the new room at the time of upgrade. - t.Run(upgradeDescritorPrefix+"upgrading a room carries over existing push rules for local users", func(t *testing.T) { + t.Run(upgradeDescriptorPrefix+"upgrading a room carries over existing push rules for local users", func(t *testing.T) { t.Parallel() // FIXME: We have to skip this test on Dendrite because it doesn't seem to send @@ -163,7 +163,7 @@ func TestPushRuleRoomUpgrade(t *testing.T) { // When a homeserver becomes aware of a room upgrade (upgrade is done on remote // homeserver), it should copy over any existing push rules for all of its local users // from the old room to the new room at the time of upgrade. - t.Run("joining a remote "+upgradeDescritorPrefix+"upgraded room carries over existing push rules", func(t *testing.T) { + t.Run("joining a remote "+upgradeDescriptorPrefix+"upgraded room carries over existing push rules", func(t *testing.T) { t.Parallel() // FIXME: We have to skip this test on Dendrite because it doesn't seem to send From 87d6ef0688ff457961ac6da52ffd2e8cc15ac00f Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 20 Nov 2025 12:04:27 -0600 Subject: [PATCH 21/21] Explain state of the spec --- tests/csapi/room_upgrade_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/csapi/room_upgrade_test.go b/tests/csapi/room_upgrade_test.go index 4a649a61..17cd8686 100644 --- a/tests/csapi/room_upgrade_test.go +++ b/tests/csapi/room_upgrade_test.go @@ -13,6 +13,13 @@ import ( "github.com/tidwall/gjson" ) +// When a homeserver becomes aware of a room upgrade, it should copy over any existing +// push rules for all of its local users from the old room. +// +// This behavior is not explained in the [Matrix +// Spec](https://spec.matrix.org/v1.16/client-server-api/#server-behaviour-19) but +// perhaps that just needs a clarification/MSC to document the state of things. Synapse +// and Dendrite do this for example. func TestPushRuleRoomUpgrade(t *testing.T) { deployment := complement.Deploy(t, 2) defer deployment.Destroy(t)