From c8d573821795ae36f4fec829162dcdcb0717740c Mon Sep 17 00:00:00 2001 From: Till Faelligen <2353100+S7evinK@users.noreply.github.com> Date: Tue, 4 Oct 2022 08:13:47 +0200 Subject: [PATCH 1/6] Add tests/61push/06_push_rules_in_sync.pl --- tests/csapi/push_test.go | 90 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/tests/csapi/push_test.go b/tests/csapi/push_test.go index b6dda3ea..dfa3b3bd 100644 --- a/tests/csapi/push_test.go +++ b/tests/csapi/push_test.go @@ -1,8 +1,11 @@ package csapi_tests import ( + "fmt" "testing" + "github.com/tidwall/gjson" + "github.com/matrix-org/complement/internal/b" "github.com/matrix-org/complement/internal/client" "github.com/matrix-org/complement/internal/match" @@ -41,3 +44,90 @@ func TestPushRuleCacheHealth(t *testing.T) { }, }) } + +func TestPushSync(t *testing.T) { + deployment := Deploy(t, b.BlueprintAlice) + defer deployment.Destroy(t) + + alice := deployment.Client(t, "hs1", "@alice:hs1") + deployment.Config.AlwaysPrintServerLogs = true + + var syncResp gjson.Result + var nextBatch string + + // sytest: Push rules come down in an initial /sync + t.Run("Push rules come down in an initial /sync", func(t *testing.T) { + syncResp, nextBatch = alice.MustSync(t, client.SyncReq{}) + // get the first result where the type is m.push_rules + pushrules := syncResp.Get(`account_data.events.#(type=="m.push_rules").content.global`) + if !pushrules.Exists() { + t.Fatalf("Expected account_data to contain push_rules: %s", syncResp.Raw) + } + }) + + // sytest: Adding a push rule wakes up an incremental /sync + t.Run("Adding a push rule wakes up an incremental /sync", func(t *testing.T) { + nextBatch = checkWokenUp(t, alice, client.SyncReq{Since: nextBatch, TimeoutMillis: "10000"}, func() { + body := client.WithJSONBody(t, map[string]interface{}{ + "actions": []string{"notify"}, + }) + + alice.MustDoFunc(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", "!foo:example.com"}, body) + }) + }) + + // sytest: Disabling a push rule wakes up an incremental /sync + t.Run("Disabling a push rule wakes up an incremental /sync", func(t *testing.T) { + nextBatch = checkWokenUp(t, alice, client.SyncReq{Since: nextBatch, TimeoutMillis: "10000"}, func() { + body := client.WithJSONBody(t, map[string]interface{}{ + "enabled": false, + }) + + alice.MustDoFunc(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", "!foo:example.com", "enabled"}, body) + }) + }) + + // sytest: Enabling a push rule wakes up an incremental /sync + t.Run("Enabling a push rule wakes up an incremental /sync", func(t *testing.T) { + nextBatch = checkWokenUp(t, alice, client.SyncReq{Since: nextBatch, TimeoutMillis: "10000"}, func() { + body := client.WithJSONBody(t, map[string]interface{}{ + "enabled": true, + }) + + alice.MustDoFunc(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", "!foo:example.com", "enabled"}, body) + }) + }) + + // sytest: Setting actions for a push rule wakes up an incremental /sync + t.Run("Setting actions for a push rule wakes up an incremental /sync", func(t *testing.T) { + checkWokenUp(t, alice, client.SyncReq{Since: nextBatch, TimeoutMillis: "10000"}, func() { + body := client.WithJSONBody(t, map[string]interface{}{ + "actions": []string{"notify"}, + }) + + alice.MustDoFunc(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", "!foo:example.com", "actions"}, body) + }) + }) +} + +func checkWokenUp(t *testing.T, csapi *client.CSAPI, syncReq client.SyncReq, fn func()) (nextBatch string) { + t.Helper() + errChan := make(chan error, 1) + go func() { + var syncResp gjson.Result + syncResp, nextBatch = csapi.MustSync(t, syncReq) + pushrules := syncResp.Get(`account_data.events.#(type=="m.push_rules").content.global`) + if !pushrules.Exists() { + errChan <- fmt.Errorf("no pushrules found in sync response: %s", syncResp.Raw) + return + } + errChan <- nil + }() + + fn() + + if err := <-errChan; err != nil { + t.Fatal(err) + } + return nextBatch +} From bafaadcddae76276e586a3ccfc562350e5fa1bd2 Mon Sep 17 00:00:00 2001 From: Till Faelligen <2353100+S7evinK@users.noreply.github.com> Date: Tue, 4 Oct 2022 08:20:48 +0200 Subject: [PATCH 2/6] Error messages; change actions --- tests/csapi/push_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/csapi/push_test.go b/tests/csapi/push_test.go index dfa3b3bd..ab233d08 100644 --- a/tests/csapi/push_test.go +++ b/tests/csapi/push_test.go @@ -61,7 +61,7 @@ func TestPushSync(t *testing.T) { // get the first result where the type is m.push_rules pushrules := syncResp.Get(`account_data.events.#(type=="m.push_rules").content.global`) if !pushrules.Exists() { - t.Fatalf("Expected account_data to contain push_rules: %s", syncResp.Raw) + t.Fatalf("no pushrules found in sync response: %s", syncResp.Raw) } }) @@ -102,7 +102,7 @@ func TestPushSync(t *testing.T) { t.Run("Setting actions for a push rule wakes up an incremental /sync", func(t *testing.T) { checkWokenUp(t, alice, client.SyncReq{Since: nextBatch, TimeoutMillis: "10000"}, func() { body := client.WithJSONBody(t, map[string]interface{}{ - "actions": []string{"notify"}, + "actions": []string{"dont_notify"}, }) alice.MustDoFunc(t, "PUT", []string{"_matrix", "client", "v3", "pushrules", "global", "room", "!foo:example.com", "actions"}, body) @@ -116,6 +116,7 @@ func checkWokenUp(t *testing.T, csapi *client.CSAPI, syncReq client.SyncReq, fn go func() { var syncResp gjson.Result syncResp, nextBatch = csapi.MustSync(t, syncReq) + // get the first result where the type is m.push_rules pushrules := syncResp.Get(`account_data.events.#(type=="m.push_rules").content.global`) if !pushrules.Exists() { errChan <- fmt.Errorf("no pushrules found in sync response: %s", syncResp.Raw) From 491ba653e13b27fc13bb4ab84b6186bc753c17b8 Mon Sep 17 00:00:00 2001 From: Till Faelligen <2353100+S7evinK@users.noreply.github.com> Date: Tue, 4 Oct 2022 08:29:19 +0200 Subject: [PATCH 3/6] Remove debug logs --- tests/csapi/push_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/csapi/push_test.go b/tests/csapi/push_test.go index ab233d08..14f2c5fa 100644 --- a/tests/csapi/push_test.go +++ b/tests/csapi/push_test.go @@ -50,7 +50,6 @@ func TestPushSync(t *testing.T) { defer deployment.Destroy(t) alice := deployment.Client(t, "hs1", "@alice:hs1") - deployment.Config.AlwaysPrintServerLogs = true var syncResp gjson.Result var nextBatch string From bf1e0298e1ca193b992ddff4f209149ab489ad84 Mon Sep 17 00:00:00 2001 From: Till Faelligen <2353100+S7evinK@users.noreply.github.com> Date: Tue, 4 Oct 2022 09:27:27 +0200 Subject: [PATCH 4/6] Wait for the sync to start to avoid responding instantly --- tests/csapi/push_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/csapi/push_test.go b/tests/csapi/push_test.go index 14f2c5fa..e53a756b 100644 --- a/tests/csapi/push_test.go +++ b/tests/csapi/push_test.go @@ -112,8 +112,10 @@ func TestPushSync(t *testing.T) { func checkWokenUp(t *testing.T, csapi *client.CSAPI, syncReq client.SyncReq, fn func()) (nextBatch string) { t.Helper() errChan := make(chan error, 1) + syncStarted := make(chan struct{}) go func() { var syncResp gjson.Result + syncStarted <- struct{}{} syncResp, nextBatch = csapi.MustSync(t, syncReq) // get the first result where the type is m.push_rules pushrules := syncResp.Get(`account_data.events.#(type=="m.push_rules").content.global`) @@ -123,7 +125,8 @@ func checkWokenUp(t *testing.T, csapi *client.CSAPI, syncReq client.SyncReq, fn } errChan <- nil }() - + // Wait for the sync to actually start, to avoid responding immediately + <-syncStarted fn() if err := <-errChan; err != nil { From 0ff946f8501e56816f2914d72f79698257567410 Mon Sep 17 00:00:00 2001 From: Till Faelligen <2353100+S7evinK@users.noreply.github.com> Date: Fri, 7 Oct 2022 15:49:36 +0200 Subject: [PATCH 5/6] PR review comments; Tweaks around timeouts --- tests/csapi/push_test.go | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/tests/csapi/push_test.go b/tests/csapi/push_test.go index e53a756b..6a8fc2ff 100644 --- a/tests/csapi/push_test.go +++ b/tests/csapi/push_test.go @@ -3,6 +3,7 @@ package csapi_tests import ( "fmt" "testing" + "time" "github.com/tidwall/gjson" @@ -125,12 +126,27 @@ func checkWokenUp(t *testing.T, csapi *client.CSAPI, syncReq client.SyncReq, fn } errChan <- nil }() - // Wait for the sync to actually start, to avoid responding immediately - <-syncStarted - fn() + // Try to wait for the sync to actually start, so that we test wakeups + select { + case <-syncStarted: + fn() + case <-time.After(time.Second * 5): + // even though this should mostly be impossible, make sure we have a timeout + t.Fatalf("go routine didn't start") + } - if err := <-errChan; err != nil { - t.Fatal(err) + // Try to wait for the sync to return or timeout after 15 seconds, + // as the above tests are using a timeout of 10 seconds + select { + case err := <-errChan: + if err != nil { + t.Fatal(err) + } + case <-time.After(time.Second * 15): + t.Errorf("sync failed to return") } + + close(errChan) + close(syncStarted) return nextBatch } From 5a15796bf3b3a006a2f7d9c877040f681fa1c2f1 Mon Sep 17 00:00:00 2001 From: Till <2353100+S7evinK@users.noreply.github.com> Date: Fri, 7 Oct 2022 16:10:48 +0200 Subject: [PATCH 6/6] Update tests/csapi/push_test.go Co-authored-by: Sean Quah <8349537+squahtx@users.noreply.github.com> --- tests/csapi/push_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/csapi/push_test.go b/tests/csapi/push_test.go index 6a8fc2ff..6e359b8a 100644 --- a/tests/csapi/push_test.go +++ b/tests/csapi/push_test.go @@ -132,7 +132,7 @@ func checkWokenUp(t *testing.T, csapi *client.CSAPI, syncReq client.SyncReq, fn fn() case <-time.After(time.Second * 5): // even though this should mostly be impossible, make sure we have a timeout - t.Fatalf("go routine didn't start") + t.Fatalf("goroutine didn't start") } // Try to wait for the sync to return or timeout after 15 seconds,