Is your feature request related to a problem? Please describe.
Everything that only exists inside a real PostgreSQL is currently verified by a human starting the server and watching it come up: the 108 migrations, the role and GRANT generation in MyNpgsqlMigrationsSqlGenerator, the hand-written SQL in JsonQueryBuilder / GameConfigurationJsonQueryBuilder, the caching repository stack, and the 120 configuration update plug-ins.
State of testing today:
| Observation |
Where |
| Ten test projects, all pure unit tests |
tests/ |
| The only EF-against-a-real-database test is disabled |
tests/MUnique.OpenMU.Persistence.Initialization.Tests/TestInitializationWithEfCore.cs:30 ([Ignore]) |
| The JSON query builder test builds the SQL but never executes it |
tests/MUnique.OpenMU.Persistence.Initialization.Tests/JsonQueryBuilderTests.cs:36 |
| The configuration change test only inspects EF model metadata |
tests/MUnique.OpenMU.Persistence.Initialization.Tests/ConfigurationChangePublishingTests.cs:29 |
CI runs no tests at all — only dotnet publish of Startup |
.github/workflows/dotnetcore.yml:17 |
The last row matters most: until CI executes the suite that already exists, nothing added later can prevent a regression.
Describe the solution you'd like
Four tiers, of which the first two are worth doing now:
| Tier |
Covers |
Docker |
Budget |
| 0 |
The existing unit tests, finally executed in CI |
no |
~1 min |
| 1 |
Persistence against a real PostgreSQL (testcontainer) |
yes |
~3–6 min |
| 2 |
Login/connect/guild/friend/game server wired in-process, in-memory persistence, scripted network client |
no |
~1–2 min |
| 3 |
The tier 2 harness on tier 1 persistence — true end-to-end |
yes |
~2 min |
| 4 |
deploy/all-in-one compose smoke test, nightly |
yes |
~5 min |
The constraint that shapes the harness
ConnectionConfigurator is static and single-shot — a second Initialize throws — and every DbContext self-initializes it with the file provider on first use (EntityDataContext.cs:47, EntityDataContextFactory.cs:19, AdminAuth/AdminPanelContextFactory.cs:19, MyNpgsqlMigrationsSqlGenerator.cs:66). Three consequences:
- The container's connection settings must be installed before the first
DbContext is constructed in the process — in NUnit, an assembly-level [SetUpFixture] outside any namespace.
- One connection-string set per test assembly, therefore one container per assembly.
- A container assembly cannot also host tests that use
ConfigFileDatabaseConnectionStringProvider — which the existing persistence tests do. So this needs a new test project, not an extension of the existing one.
Connection settings for tests
A TestDatabaseConnectionSettingProvider loads the shipped ConnectionSettings.xml as its template and rewrites only host, port and database via NpgsqlConnectionStringBuilder. Keeping the production role names and passwords (account, config, guild, friend) means the migration generator's CREATE ROLE / GRANT statements run exactly as in production, which is what makes privilege tests possible at all. Since GetConnectionSetting is consulted on every OnConfiguring, the provider can serve a mutable database name, so switching databases between fixtures is a field assignment.
Container and database lifecycle
Seeding is the expensive part (CreateInitialDataAsync(1, true) builds the whole game configuration), so pay for it once:
once per assembly
start postgres:<pinned>-alpine -c fsync=off -c synchronous_commit=off
-c full_page_writes=off -c max_connections=200
migrate EntityDataContext + AdminPanelContext
run data initialization -> database "openmu_template"
per fixture (writing tests only)
NpgsqlConnection.ClearAllPools()
CREATE DATABASE "test_n" TEMPLATE "openmu_template" (a file copy)
point the provider at test_n
Read-only fixtures share one database and skip the copy. Four traps worth writing into the harness up front:
- Pooling —
CREATE DATABASE … TEMPLATE and DROP DATABASE fail while any connection is open; clear the pools first.
- Roles are cluster-wide, not per database —
CREATE ROLE is guarded by an existence check, but dropping a schema emits DROP ROLE IF EXISTS, so never run two migration runs concurrently in one container.
ReCreateDatabaseAsync(true) issues EnsureDeletedAsync; on a freshly created database pass false and let migrations build the schema.
- Parallelism — start the integration assembly
NonParallelizable; revisit once the template path is proven.
Required changes in src/ (both small)
ConnectionConfigurator: an Initialize(provider, overwrite: true) overload, or an internal Reset() plus InternalsVisibleTo (the repo already does this for other test assemblies). The file's own TODO: Make class non-static is the proper long-term fix.
ConfigFileDatabaseConnectionStringProvider: support DB_PORT (and optionally DB_NAME) next to the existing DB_HOST, DB_ADMIN_USER, DB_ADMIN_PW. Useful on its own — a non-default port is not configurable today.
Tier 1 test catalogue, by value
- Schema health — all migrations apply to an empty database and leave nothing pending; roles exist with the expected grants; re-migrating is a no-op. Plus a model drift check comparing the snapshot against the current model with
IMigrationsModelDiffer: PendingModelChangesWarning is currently suppressed (ConnectionConfigurator.cs:114), so "changed the model, forgot the migration" is invisible today.
- Data initialization round-trip for
Version075, Version095d, VersionSeasonSix — the [Ignore]d test revived: seed, reload through GameConfigurationJsonObjectLoader, compare against the in-memory result (collection counts plus spot checks on maps, items, skills, drop groups); the documented test accounts must authenticate.
- The JSON loading paths, actually executed —
JsonQueryBuilder and AccountRepository.LoadAccountByLoginNameByJsonQueryAsync (AccountRepository.cs:158).
- Role privileges — the config and account roles may not write configuration, the trade context is limited to items; assert
PostgresException with SqlState 42501.
- Player data round-trip — account and character with inventory, vault and item options: save, reload in a fresh context, compare structurally; then letters, guild membership and friends.
- Concurrency — the real-database counterpart to the in-memory
PersistenceLockTest.
- Configuration update plug-ins — apply all 120
IConfigurationUpdatePlugIns in version order to a freshly seeded database: each applies without error, is idempotent when applied twice, and the configuration still loads afterwards. Given how often updates are added, probably the strongest regression net here.
- Configuration change publishing — change an entity through a context, assert
ConfigurationChangeListener publishes it and the cache-aware repository provider serves the updated instance.
- Admin panel authentication against the real
admin schema — AdminUserRepository, bootstrap admin, BCrypt hashing, lockout.
Tier 2 and above
Tier 2 starts the servers in-process on ephemeral ports with in-memory persistence and drives them with a scripted client built from MUnique.OpenMU.Network (a Connection with the client-side SimpleModulus/XOR32 keys and the generated packet structs): connect handshake and server list, login with wrong and right credentials, character list/creation/deletion, enter world, walk, chat and whisper, logout and reconnect, the per-IP connection limit, disconnect cleanup. ChatServer.Tests/ChatClientTests.cs already shows the cheaper DuplexPipe variant; both are useful — pipes for protocol assertions, real loopback sockets for at least one smoke test of the listener and encryption wiring.
Tier 3 is that harness on the tier 1 persistence: play a short scripted session, then assert the persisted result (position, experience, inventory) — the test that catches "works in-memory, breaks on PostgreSQL".
CI
Keep the existing build job, add unit-tests (--filter TestCategory!=Integration) and integration-tests (--filter TestCategory=Integration), both on ubuntu-latest where Docker is available. Things that will otherwise bite:
- Do not build the whole solution on Linux —
ClientLauncher and Network.Analyzer.WinForms target net10.0-windows. Use a solution filter listing the test projects and their dependencies, or invoke dotnet test per project.
- Do not pass
-p:ci=true to the test jobs — the packet structure tests are generated in a PreBuild target that is skipped when it is set (tests/MUnique.OpenMU.Network.Packets.Tests/…csproj:43), so they would silently vanish.
- Skip policy — missing Docker ignores the fixture locally but fails when the
CI variable is set; a silently skipped suite in CI is worse than no suite.
- Cache NuGet, pre-pull the image, and consider adding a
pull_request trigger (the workflow is on: [push] only today).
Locally this needs Docker Desktop, Podman with DOCKER_HOST, or Rancher Desktop; --filter TestCategory!=Integration stays fast and container-free for everyone else, and Testcontainers reuse behind an environment switch keeps the inner loop short.
Suggested rollout
| Phase |
Content |
Size |
| 0 |
CI runs the existing unit tests (solution filter, no ci=true); no new tests |
small |
| 1 |
Spike: new project, container, provider, template database, one migrate-and-seed test, measured timings |
medium |
| 2 |
Schema health, data initialization round-trip, JSON loading (1–3) |
medium |
| 3 |
Repositories, privileges, concurrency, admin auth (4–6, 9) |
medium |
| 4 |
Configuration update plug-ins and change publishing (7–8) |
medium |
| 5 |
Integration job in CI, contributor documentation |
small |
| 6 |
Tier 2 server harness and the first flows |
large |
| 7 |
Tier 3 end-to-end, optionally tier 4 nightly |
medium |
Phases 0–2 already deliver most of the value; the rest can be picked up piece by piece.
Describe alternatives you've considered
- A shared, externally provisioned PostgreSQL (docker-compose or a CI service container) instead of Testcontainers: fewer moving parts in the test code, but no per-test database isolation, no automatic lifecycle, and contributors have to remember to start it.
- Only extending
ConfigFileDatabaseConnectionStringProvider with DB_PORT/DB_NAME and pointing it at a container started outside the test process: less new code, but the settings are read once, so per-test databases are impossible. Worth doing anyway as a production improvement, just not sufficient as the harness.
- Keeping everything on the in-memory provider: fast, but by construction it cannot cover migrations, the generated role SQL, the raw JSON queries or privilege enforcement — exactly the code that this proposal targets.
- Transaction rollback per test instead of template databases: does not work here, because a single scenario spans several contexts on separate connections (account, config, guild, friend, trade).
Additional context
Open questions before phase 1:
- Which PostgreSQL version should be the supported baseline? One pinned version is cheap, a matrix over two majors doubles the job time — and the deploy compose files use the unpinned
postgres tag today (deploy/all-in-one/docker-compose.yml:42).
- Should the integration job run on every push, or only on pull requests?
- If Season 6 seeding turns out to be slow, is it acceptable to run the repository-level fixtures on
Version075 data and keep the full seed for the initialization fixture only?
I'm happy to implement this phase by phase if the direction looks right.
Is your feature request related to a problem? Please describe.
Everything that only exists inside a real PostgreSQL is currently verified by a human starting the server and watching it come up: the 108 migrations, the role and
GRANTgeneration inMyNpgsqlMigrationsSqlGenerator, the hand-written SQL inJsonQueryBuilder/GameConfigurationJsonQueryBuilder, the caching repository stack, and the 120 configuration update plug-ins.State of testing today:
tests/tests/MUnique.OpenMU.Persistence.Initialization.Tests/TestInitializationWithEfCore.cs:30([Ignore])tests/MUnique.OpenMU.Persistence.Initialization.Tests/JsonQueryBuilderTests.cs:36tests/MUnique.OpenMU.Persistence.Initialization.Tests/ConfigurationChangePublishingTests.cs:29dotnet publishof Startup.github/workflows/dotnetcore.yml:17The last row matters most: until CI executes the suite that already exists, nothing added later can prevent a regression.
Describe the solution you'd like
Four tiers, of which the first two are worth doing now:
deploy/all-in-onecompose smoke test, nightlyThe constraint that shapes the harness
ConnectionConfiguratoris static and single-shot — a secondInitializethrows — and everyDbContextself-initializes it with the file provider on first use (EntityDataContext.cs:47,EntityDataContextFactory.cs:19,AdminAuth/AdminPanelContextFactory.cs:19,MyNpgsqlMigrationsSqlGenerator.cs:66). Three consequences:DbContextis constructed in the process — in NUnit, an assembly-level[SetUpFixture]outside any namespace.ConfigFileDatabaseConnectionStringProvider— which the existing persistence tests do. So this needs a new test project, not an extension of the existing one.Connection settings for tests
A
TestDatabaseConnectionSettingProviderloads the shippedConnectionSettings.xmlas its template and rewrites only host, port and database viaNpgsqlConnectionStringBuilder. Keeping the production role names and passwords (account,config,guild,friend) means the migration generator'sCREATE ROLE/GRANTstatements run exactly as in production, which is what makes privilege tests possible at all. SinceGetConnectionSettingis consulted on everyOnConfiguring, the provider can serve a mutable database name, so switching databases between fixtures is a field assignment.Container and database lifecycle
Seeding is the expensive part (
CreateInitialDataAsync(1, true)builds the whole game configuration), so pay for it once:Read-only fixtures share one database and skip the copy. Four traps worth writing into the harness up front:
CREATE DATABASE … TEMPLATEandDROP DATABASEfail while any connection is open; clear the pools first.CREATE ROLEis guarded by an existence check, but dropping a schema emitsDROP ROLE IF EXISTS, so never run two migration runs concurrently in one container.ReCreateDatabaseAsync(true)issuesEnsureDeletedAsync; on a freshly created database passfalseand let migrations build the schema.NonParallelizable; revisit once the template path is proven.Required changes in
src/(both small)ConnectionConfigurator: anInitialize(provider, overwrite: true)overload, or aninternal Reset()plusInternalsVisibleTo(the repo already does this for other test assemblies). The file's ownTODO: Make class non-staticis the proper long-term fix.ConfigFileDatabaseConnectionStringProvider: supportDB_PORT(and optionallyDB_NAME) next to the existingDB_HOST,DB_ADMIN_USER,DB_ADMIN_PW. Useful on its own — a non-default port is not configurable today.Tier 1 test catalogue, by value
IMigrationsModelDiffer:PendingModelChangesWarningis currently suppressed (ConnectionConfigurator.cs:114), so "changed the model, forgot the migration" is invisible today.Version075,Version095d,VersionSeasonSix— the[Ignore]d test revived: seed, reload throughGameConfigurationJsonObjectLoader, compare against the in-memory result (collection counts plus spot checks on maps, items, skills, drop groups); the documented test accounts must authenticate.JsonQueryBuilderandAccountRepository.LoadAccountByLoginNameByJsonQueryAsync(AccountRepository.cs:158).PostgresExceptionwithSqlState 42501.PersistenceLockTest.IConfigurationUpdatePlugIns in version order to a freshly seeded database: each applies without error, is idempotent when applied twice, and the configuration still loads afterwards. Given how often updates are added, probably the strongest regression net here.ConfigurationChangeListenerpublishes it and the cache-aware repository provider serves the updated instance.adminschema —AdminUserRepository, bootstrap admin, BCrypt hashing, lockout.Tier 2 and above
Tier 2 starts the servers in-process on ephemeral ports with in-memory persistence and drives them with a scripted client built from
MUnique.OpenMU.Network(aConnectionwith the client-side SimpleModulus/XOR32 keys and the generated packet structs): connect handshake and server list, login with wrong and right credentials, character list/creation/deletion, enter world, walk, chat and whisper, logout and reconnect, the per-IP connection limit, disconnect cleanup.ChatServer.Tests/ChatClientTests.csalready shows the cheaperDuplexPipevariant; both are useful — pipes for protocol assertions, real loopback sockets for at least one smoke test of the listener and encryption wiring.Tier 3 is that harness on the tier 1 persistence: play a short scripted session, then assert the persisted result (position, experience, inventory) — the test that catches "works in-memory, breaks on PostgreSQL".
CI
Keep the existing
buildjob, addunit-tests(--filter TestCategory!=Integration) andintegration-tests(--filter TestCategory=Integration), both onubuntu-latestwhere Docker is available. Things that will otherwise bite:ClientLauncherandNetwork.Analyzer.WinFormstargetnet10.0-windows. Use a solution filter listing the test projects and their dependencies, or invokedotnet testper project.-p:ci=trueto the test jobs — the packet structure tests are generated in aPreBuildtarget that is skipped when it is set (tests/MUnique.OpenMU.Network.Packets.Tests/…csproj:43), so they would silently vanish.CIvariable is set; a silently skipped suite in CI is worse than no suite.pull_requesttrigger (the workflow ison: [push]only today).Locally this needs Docker Desktop, Podman with
DOCKER_HOST, or Rancher Desktop;--filter TestCategory!=Integrationstays fast and container-free for everyone else, and Testcontainers reuse behind an environment switch keeps the inner loop short.Suggested rollout
ci=true); no new testsPhases 0–2 already deliver most of the value; the rest can be picked up piece by piece.
Describe alternatives you've considered
ConfigFileDatabaseConnectionStringProviderwithDB_PORT/DB_NAMEand pointing it at a container started outside the test process: less new code, but the settings are read once, so per-test databases are impossible. Worth doing anyway as a production improvement, just not sufficient as the harness.Additional context
Open questions before phase 1:
postgrestag today (deploy/all-in-one/docker-compose.yml:42).Version075data and keep the full seed for the initialization fixture only?I'm happy to implement this phase by phase if the direction looks right.