From 48fa869fa3578741d1d5775d30f24f6b097ab995 Mon Sep 17 00:00:00 2001 From: Till <2353100+S7evinK@users.noreply.github.com> Date: Mon, 23 Jan 2023 13:17:15 +0100 Subject: [PATCH] Use `t.TempDir` for SQLite databases, so tests don't rip out each others databases (#2950) This should hopefully finally fix issues about `disk I/O error` as seen [here](https://gitlab.alpinelinux.org/alpine/aports/-/jobs/955030/raw) Hopefully this will also fix `SSL accept attempt failed` issues by disabling HTTP keep alives when generating a config for CI. --- cmd/generate-config/main.go | 1 + internal/log.go | 2 ++ internal/log_unix.go | 2 ++ setup/jetstream/helpers.go | 5 +++++ sytest-blacklist | 1 + sytest-whitelist | 7 ++++++- test/db.go | 12 +++++------- test/testrig/base.go | 37 ++++++++++++++----------------------- 8 files changed, 36 insertions(+), 31 deletions(-) diff --git a/cmd/generate-config/main.go b/cmd/generate-config/main.go index 5f75f5e4..56a14565 100644 --- a/cmd/generate-config/main.go +++ b/cmd/generate-config/main.go @@ -70,6 +70,7 @@ func main() { cfg.AppServiceAPI.DisableTLSValidation = true cfg.ClientAPI.RateLimiting.Enabled = false cfg.FederationAPI.DisableTLSValidation = false + cfg.FederationAPI.DisableHTTPKeepalives = true // don't hit matrix.org when running tests!!! cfg.FederationAPI.KeyPerspectives = config.KeyPerspectives{} cfg.MediaAPI.BasePath = config.Path(filepath.Join(*dirPath, "media")) diff --git a/internal/log.go b/internal/log.go index d7e852c8..da6e2041 100644 --- a/internal/log.go +++ b/internal/log.go @@ -24,6 +24,7 @@ import ( "path/filepath" "runtime" "strings" + "sync" "github.com/matrix-org/util" @@ -37,6 +38,7 @@ import ( // this unfortunately results in us adding the same hook multiple times. // This map ensures we only ever add one level hook. var stdLevelLogAdded = make(map[logrus.Level]bool) +var levelLogAddedMu = &sync.Mutex{} type utcFormatter struct { logrus.Formatter diff --git a/internal/log_unix.go b/internal/log_unix.go index b38e7c2e..8f34c320 100644 --- a/internal/log_unix.go +++ b/internal/log_unix.go @@ -85,6 +85,8 @@ func checkSyslogHookParams(params map[string]interface{}) { } func setupStdLogHook(level logrus.Level) { + levelLogAddedMu.Lock() + defer levelLogAddedMu.Unlock() if stdLevelLogAdded[level] { return } diff --git a/setup/jetstream/helpers.go b/setup/jetstream/helpers.go index c1ce9583..53365216 100644 --- a/setup/jetstream/helpers.go +++ b/setup/jetstream/helpers.go @@ -77,6 +77,11 @@ func JetStreamConsumer( // The consumer was deleted so stop. return } else { + // Unfortunately, there's no ErrServerShutdown or similar, so we need to compare the string + if err.Error() == "nats: Server Shutdown" { + logrus.WithContext(ctx).Warn("nats server shutting down") + return + } // Something else went wrong, so we'll panic. sentry.CaptureException(err) logrus.WithContext(ctx).WithField("subject", subj).Fatal(err) diff --git a/sytest-blacklist b/sytest-blacklist index bb0ee368..49a3cc87 100644 --- a/sytest-blacklist +++ b/sytest-blacklist @@ -7,6 +7,7 @@ AS-ghosted users can use rooms via AS Events in rooms with AS-hosted room aliases are sent to AS server Inviting an AS-hosted user asks the AS server Accesing an AS-hosted room alias asks the AS server +If user leaves room, remote user changes device and rejoins we see update in /sync and /keys/changes # This will fail in HTTP API mode, so blacklisted for now If a device list update goes missing, the server resyncs on the next one diff --git a/sytest-whitelist b/sytest-whitelist index 1f6ecc29..c61e0bc3 100644 --- a/sytest-whitelist +++ b/sytest-whitelist @@ -778,4 +778,9 @@ Can receive redactions from regular users over federation in room version 10 New federated private chats get full presence information (SYN-115) /state returns M_NOT_FOUND for an outlier /state_ids returns M_NOT_FOUND for an outlier -Outbound federation requests missing prev_events and then asks for /state_ids and resolves the state \ No newline at end of file +Outbound federation requests missing prev_events and then asks for /state_ids and resolves the state +Invited user can reject invite for empty room +Invited user can reject local invite after originator leaves +Guest users can join guest_access rooms +Forgotten room messages cannot be paginated +Local device key changes get to remote servers with correct prev_id \ No newline at end of file diff --git a/test/db.go b/test/db.go index 17f637e1..54ded6ad 100644 --- a/test/db.go +++ b/test/db.go @@ -22,6 +22,7 @@ import ( "os" "os/exec" "os/user" + "path/filepath" "testing" "github.com/lib/pq" @@ -103,13 +104,10 @@ func currentUser() string { // TODO: namespace for concurrent package tests func PrepareDBConnectionString(t *testing.T, dbType DBType) (connStr string, close func()) { if dbType == DBTypeSQLite { - // this will be made in the current working directory which namespaces concurrent package runs correctly - dbname := "dendrite_test.db" + // this will be made in the t.TempDir, which is unique per test + dbname := filepath.Join(t.TempDir(), "dendrite_test.db") return fmt.Sprintf("file:%s", dbname), func() { - err := os.Remove(dbname) - if err != nil { - t.Fatalf("failed to cleanup sqlite db '%s': %s", dbname, err) - } + t.Cleanup(func() {}) // removes the t.TempDir } } @@ -176,7 +174,7 @@ func WithAllDatabases(t *testing.T, testFn func(t *testing.T, db DBType)) { for dbName, dbType := range dbs { dbt := dbType t.Run(dbName, func(tt *testing.T) { - //tt.Parallel() + tt.Parallel() testFn(tt, dbt) }) } diff --git a/test/testrig/base.go b/test/testrig/base.go index 52e6ef5f..9773da22 100644 --- a/test/testrig/base.go +++ b/test/testrig/base.go @@ -15,18 +15,14 @@ package testrig import ( - "errors" "fmt" - "io/fs" - "os" - "strings" + "path/filepath" "testing" - "github.com/nats-io/nats.go" - "github.com/matrix-org/dendrite/setup/base" "github.com/matrix-org/dendrite/setup/config" "github.com/matrix-org/dendrite/test" + "github.com/nats-io/nats.go" ) func CreateBaseDendrite(t *testing.T, dbType test.DBType) (*base.BaseDendrite, func()) { @@ -77,27 +73,22 @@ func CreateBaseDendrite(t *testing.T, dbType test.DBType) (*base.BaseDendrite, f // use a distinct prefix else concurrent postgres/sqlite runs will clash since NATS will use // the file system event with InMemory=true :( cfg.Global.JetStream.TopicPrefix = fmt.Sprintf("Test_%d_", dbType) + + // Use a temp dir provided by go for tests, this will be cleanup by a call to t.CleanUp() + tempDir := t.TempDir() + cfg.FederationAPI.Database.ConnectionString = config.DataSource(filepath.Join("file://", tempDir, "federationapi.db")) + cfg.KeyServer.Database.ConnectionString = config.DataSource(filepath.Join("file://", tempDir, "keyserver.db")) + cfg.MSCs.Database.ConnectionString = config.DataSource(filepath.Join("file://", tempDir, "mscs.db")) + cfg.MediaAPI.Database.ConnectionString = config.DataSource(filepath.Join("file://", tempDir, "mediaapi.db")) + cfg.RoomServer.Database.ConnectionString = config.DataSource(filepath.Join("file://", tempDir, "roomserver.db")) + cfg.SyncAPI.Database.ConnectionString = config.DataSource(filepath.Join("file://", tempDir, "syncapi.db")) + cfg.UserAPI.AccountDatabase.ConnectionString = config.DataSource(filepath.Join("file://", tempDir, "userapi.db")) + base := base.NewBaseDendrite(&cfg, "Test", base.DisableMetrics) return base, func() { base.ShutdownDendrite() base.WaitForShutdown() - // cleanup db files. This risks getting out of sync as we add more database strings :( - dbFiles := []config.DataSource{ - cfg.FederationAPI.Database.ConnectionString, - cfg.KeyServer.Database.ConnectionString, - cfg.MSCs.Database.ConnectionString, - cfg.MediaAPI.Database.ConnectionString, - cfg.RoomServer.Database.ConnectionString, - cfg.SyncAPI.Database.ConnectionString, - cfg.UserAPI.AccountDatabase.ConnectionString, - } - for _, fileURI := range dbFiles { - path := strings.TrimPrefix(string(fileURI), "file:") - err := os.Remove(path) - if err != nil && !errors.Is(err, fs.ErrNotExist) { - t.Fatalf("failed to cleanup sqlite db '%s': %s", fileURI, err) - } - } + t.Cleanup(func() {}) // removes t.TempDir, where all database files are created } default: t.Fatalf("unknown db type: %v", dbType)