Support functional options in TestMain #750
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Rather than having `TestMainWithCleanup` and `TestMain`, use functional options. Add support for a custom deployment mechanism at the same time.
anoadragon453
approved these changes
Dec 17, 2024
S7evinK
reviewed
Dec 17, 2024
|
Can we merge this now ? I've tested it successfully. |
|
I've checked the private implementation and it looks to work. Thanks! |
1 task
MadLittleMods
added a commit
that referenced
this pull request
May 16, 2025
…778) Spawning from a real use-case with a custom `Deployment`/`Deployer` (Element-internal). I think this is the first working example of a custom deployment so it makes sense that we ran into a couple oversights. This PR introduces some quality of life for working with a custom `Deployment`. For reference, custom deployments were introduced in #750. - Pass `t` to allow the custom deployment creation callback (`complement.WithDeployment(callback)`) to handle errors gracefully - Also pass in `complement.Config` so the custom Deployment can align behavior to what's configured
12 tasks
MadLittleMods
added a commit
that referenced
this pull request
May 19, 2025
…t custom `Deployment` (#780) *Split off from #778 per [discussion](#778 (comment) Spawning from a real use-case with a custom `Deployment`/`Deployer` (Element-internal). Introduce `complement.Deployment.GetFullyQualifiedHomeserverName(hsName)` to allow the per-deployment short homeserver aliases like `hs1` to be mapped to something else that's resolvable in your custom deployments context. Example: `hs1` -> `hs1.shard1:8481`. This is useful for situations like the following where you have to specify the via servers in a federated join request during a test: ```go alice.MustJoinRoom(t, bobRoomID, []string{ deployment.GetFullyQualifiedHomeserverName(t, "hs2"), }) ``` ### But why does this have to be part of the `complement.Deployment` interface instead of your own custom deployment? - Tests only have access to the generic `complement.Deployment` interface - We can't derive fully-qualified homeserver names from the existing `complement.Deployment` interface - While we could cheekily cast the generic `complement.Deployment` back to `CustomDeployment` in our own tests (and have the helper in the `CustomDeployment` instead), if we start using something exotic in our out-of-repo Complement tests, the suite of existing Complement tests in this repo will not be compatible. (also see below) ### Motivating custom `Deployment` use case [`complement.Deployment`](https://github.com/matrix-org/complement/blob/d2e04c995666fbeb0948e6a4ed52d3fbb45fbdf7/test_package.go#L21-L69) is an interface that can be backed by anything. For reference, custom deployments were introduced in #750. The [default `Deployment` naming scheme in Complement is `hs1`, `hs2`, etc](https://github.com/matrix-org/complement/blob/6b63eff50804beb334ca215650f5027ddf02ae9a/test_package.go#L198). It's really nice and convenient to be able to simply refer to homeservers as `hs1`, etc within a deployment. And using consistent names like this makes tests compatible with each other regardless of which `Deployment` is being used. The built-in `Deployment` in Complement has each homeserver in a Docker container which already has network aliases like `hs1`, `hs2`, etc so no translation is needed from friendly name to resolvable address. When one homeserver needs to federate with the other, it can simply make a request to `https://hs1:8448/...` per [spec on resolving server names](https://spec.matrix.org/v1.13/server-server-api/#resolving-server-names). Right-now, we hard-code `hs1` across the tests when we specify ["via" servers in join requests](https://spec.matrix.org/v1.13/client-server-api/#post_matrixclientv3joinroomidoralias) but that only works if you follow the strict single-deployment naming scheme. https://github.com/matrix-org/complement/blob/6b63eff50804beb334ca215650f5027ddf02ae9a/tests/federation_rooms_invite_test.go#L112 In the current setup of our custom `Deployment`, each `Deployment` is a "shard" application that can deploy multiple homeserver "tenants". We specifically want to test that homeservers between multiple shards can federate with each other as a sanity check (make sure our shards can deploy homeserver tenants correctly). If we keep using the consistent `hs1`, `hs2` naming scheme for each `Deployment` we're going to have conflicts. This is where `deployment.GetFullyQualifiedHomeserverName(t, hsName)` comes in handy. We can call `deployment1.GetFullyQualifiedHomeserverName(t, "hs1")` -> `hs1.shard1` and also `deployment2.GetFullyQualifiedHomeserverName(t, "hs1")` -> `hs1.shard2` to get their unique resolvable addresses in the network. Additionally, the helper removes the constraint of needing the network to strictly resolve `hs1`, `hs2` hostnames to their respective homeservers. Whenever you need to refer to another homeserver, use `deployment.GetFullyQualifiedHomeserverName(hsName)` to take care of the nuance of environment that the given `Deployment` creates. Example of a cross-deployment test: ```go func TestMain(m *testing.M) { complement.TestMain(m, "custom_tests", complement.WithDeployment(internal.MakeCustomDeployment()), ) } func TestCrossShardFederation(t *testing.T) { // Create two shards with their own homeserver tenants shardDeployment1 := complement.Deploy(t, 1) defer shardDeployment1.Destroy(t) shardDeployment2 := complement.Deploy(t, 1) defer shardDeployment2.Destroy(t) alice := shardDeployment1.Register(t, "hs1", helpers.RegistrationOpts{}) bob := shardDeployment2.Register(t, "hs1", helpers.RegistrationOpts{}) aliceRoomID := alice.MustCreateRoom(t, map[string]any{ "preset": "public_chat", }) bobRoomID := bob.MustCreateRoom(t, map[string]any{ "preset": "public_chat", }) t.Run("parallel", func(t *testing.T) { t.Run("shard1 -> shard2", func(t *testing.T) { // Since these tests use the same config, they can be run in parallel t.Parallel() alice.MustJoinRoom(t, bobRoomID, []string{ shardDeployment2.GetFullyQualifiedHomeserverName(t, "hs1"), }) bob.MustSyncUntil(t, client.SyncReq{}, client.SyncJoinedTo(alice.UserID, bobRoomID)) }) t.Run("shard2 -> shard1", func(t *testing.T) { // Since these tests use the same config, they can be run in parallel t.Parallel() bob.MustJoinRoom(t, aliceRoomID, []string{ shardDeployment1.GetFullyQualifiedHomeserverName(t, "hs1"), }) alice.MustSyncUntil(t, client.SyncReq{}, client.SyncJoinedTo(bob.UserID, aliceRoomID)) }) }) } ``` Per the discussion in #780 (comment), multiple-deployments per test doesn't work with Complement's `Deployment` implementation yet and the `Deployment` is meant to encapsulate an _entire_ deployment, all servers and network links between them. This was the motivating use case but use at your own discretion until further guidance is given.
jevolk
pushed a commit
to matrix-construct/complement
that referenced
this pull request
Jul 22, 2025
…atrix-org#778) Spawning from a real use-case with a custom `Deployment`/`Deployer` (Element-internal). I think this is the first working example of a custom deployment so it makes sense that we ran into a couple oversights. This PR introduces some quality of life for working with a custom `Deployment`. For reference, custom deployments were introduced in matrix-org#750. - Pass `t` to allow the custom deployment creation callback (`complement.WithDeployment(callback)`) to handle errors gracefully - Also pass in `complement.Config` so the custom Deployment can align behavior to what's configured
jevolk
pushed a commit
to matrix-construct/complement
that referenced
this pull request
Jul 22, 2025
…t custom `Deployment` (matrix-org#780) *Split off from matrix-org#778 per [discussion](matrix-org#778 (comment) Spawning from a real use-case with a custom `Deployment`/`Deployer` (Element-internal). Introduce `complement.Deployment.GetFullyQualifiedHomeserverName(hsName)` to allow the per-deployment short homeserver aliases like `hs1` to be mapped to something else that's resolvable in your custom deployments context. Example: `hs1` -> `hs1.shard1:8481`. This is useful for situations like the following where you have to specify the via servers in a federated join request during a test: ```go alice.MustJoinRoom(t, bobRoomID, []string{ deployment.GetFullyQualifiedHomeserverName(t, "hs2"), }) ``` ### But why does this have to be part of the `complement.Deployment` interface instead of your own custom deployment? - Tests only have access to the generic `complement.Deployment` interface - We can't derive fully-qualified homeserver names from the existing `complement.Deployment` interface - While we could cheekily cast the generic `complement.Deployment` back to `CustomDeployment` in our own tests (and have the helper in the `CustomDeployment` instead), if we start using something exotic in our out-of-repo Complement tests, the suite of existing Complement tests in this repo will not be compatible. (also see below) ### Motivating custom `Deployment` use case [`complement.Deployment`](https://github.com/matrix-org/complement/blob/d2e04c995666fbeb0948e6a4ed52d3fbb45fbdf7/test_package.go#L21-L69) is an interface that can be backed by anything. For reference, custom deployments were introduced in matrix-org#750. The [default `Deployment` naming scheme in Complement is `hs1`, `hs2`, etc](https://github.com/matrix-org/complement/blob/6b63eff50804beb334ca215650f5027ddf02ae9a/test_package.go#L198). It's really nice and convenient to be able to simply refer to homeservers as `hs1`, etc within a deployment. And using consistent names like this makes tests compatible with each other regardless of which `Deployment` is being used. The built-in `Deployment` in Complement has each homeserver in a Docker container which already has network aliases like `hs1`, `hs2`, etc so no translation is needed from friendly name to resolvable address. When one homeserver needs to federate with the other, it can simply make a request to `https://hs1:8448/...` per [spec on resolving server names](https://spec.matrix.org/v1.13/server-server-api/#resolving-server-names). Right-now, we hard-code `hs1` across the tests when we specify ["via" servers in join requests](https://spec.matrix.org/v1.13/client-server-api/#post_matrixclientv3joinroomidoralias) but that only works if you follow the strict single-deployment naming scheme. https://github.com/matrix-org/complement/blob/6b63eff50804beb334ca215650f5027ddf02ae9a/tests/federation_rooms_invite_test.go#L112 In the current setup of our custom `Deployment`, each `Deployment` is a "shard" application that can deploy multiple homeserver "tenants". We specifically want to test that homeservers between multiple shards can federate with each other as a sanity check (make sure our shards can deploy homeserver tenants correctly). If we keep using the consistent `hs1`, `hs2` naming scheme for each `Deployment` we're going to have conflicts. This is where `deployment.GetFullyQualifiedHomeserverName(t, hsName)` comes in handy. We can call `deployment1.GetFullyQualifiedHomeserverName(t, "hs1")` -> `hs1.shard1` and also `deployment2.GetFullyQualifiedHomeserverName(t, "hs1")` -> `hs1.shard2` to get their unique resolvable addresses in the network. Additionally, the helper removes the constraint of needing the network to strictly resolve `hs1`, `hs2` hostnames to their respective homeservers. Whenever you need to refer to another homeserver, use `deployment.GetFullyQualifiedHomeserverName(hsName)` to take care of the nuance of environment that the given `Deployment` creates. Example of a cross-deployment test: ```go func TestMain(m *testing.M) { complement.TestMain(m, "custom_tests", complement.WithDeployment(internal.MakeCustomDeployment()), ) } func TestCrossShardFederation(t *testing.T) { // Create two shards with their own homeserver tenants shardDeployment1 := complement.Deploy(t, 1) defer shardDeployment1.Destroy(t) shardDeployment2 := complement.Deploy(t, 1) defer shardDeployment2.Destroy(t) alice := shardDeployment1.Register(t, "hs1", helpers.RegistrationOpts{}) bob := shardDeployment2.Register(t, "hs1", helpers.RegistrationOpts{}) aliceRoomID := alice.MustCreateRoom(t, map[string]any{ "preset": "public_chat", }) bobRoomID := bob.MustCreateRoom(t, map[string]any{ "preset": "public_chat", }) t.Run("parallel", func(t *testing.T) { t.Run("shard1 -> shard2", func(t *testing.T) { // Since these tests use the same config, they can be run in parallel t.Parallel() alice.MustJoinRoom(t, bobRoomID, []string{ shardDeployment2.GetFullyQualifiedHomeserverName(t, "hs1"), }) bob.MustSyncUntil(t, client.SyncReq{}, client.SyncJoinedTo(alice.UserID, bobRoomID)) }) t.Run("shard2 -> shard1", func(t *testing.T) { // Since these tests use the same config, they can be run in parallel t.Parallel() bob.MustJoinRoom(t, aliceRoomID, []string{ shardDeployment1.GetFullyQualifiedHomeserverName(t, "hs1"), }) alice.MustSyncUntil(t, client.SyncReq{}, client.SyncJoinedTo(bob.UserID, aliceRoomID)) }) }) } ``` Per the discussion in matrix-org#780 (comment), multiple-deployments per test doesn't work with Complement's `Deployment` implementation yet and the `Deployment` is meant to encapsulate an _entire_ deployment, all servers and network links between them. This was the motivating use case but use at your own discretion until further guidance is given.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rather than having
TestMainWithCleanupandTestMain, use functional options. Add support for a custom deployment mechanism at the same time.This is a breaking change for users who use
TestMainWithCleanup(m, namespace, cleanupFn)who now need to doTestMain(m, namespace, complement.WithCleanup(cleanupFn)).