Merge pull request #45 from matrix-org/kegan/rm-templates

Convert a templated test to multiprocess

.github/workflows/single_sdk_tests.yml

@@ -20,16 +20,28 @@ on:
         required: false
         default: ''
         type: string
+      use_complement_crypto:
+        description: 'tag/commit/branch of Complement Crypto to test against. If "." then the caller checkout is used.'
+        required: false
+        default: 'main'
+        type: string
 jobs:
   tests:
     runs-on: ubuntu-latest
     steps:
         - name: Checkout repo
           uses: actions/checkout@v3
         - name: Checkout complement-crypto
+          if: ${{ inputs.use_complement_crypto != '.'}}
+          env:
+            COMPLEMENT_CRYPTO_SHA: ${{ inputs.use_complement_crypto }}
           run: |
             mkdir complement-crypto
-            (wget -O - "https://github.com/matrix-org/complement-crypto/archive/main.tar.gz" | tar -xz --strip-components=1 -C complement-crypto)
+            (wget -O - "https://github.com/matrix-org/complement-crypto/archive/$COMPLEMENT_CRYPTO_SHA.tar.gz" | tar -xz --strip-components=1 -C complement-crypto)
+        - name: Symlink complement-crypto
+          if: ${{ inputs.use_complement_crypto == '.'}}
+          run: |
+            ln -s . complement-crypto
         # Setup code we always need
         - name: Pull synapse service v1.94.0 and mitmproxy
           shell: bash
@@ -84,11 +96,25 @@ jobs:
           run: |
             cd complement-crypto && ./rebuild_rust_sdk.sh $RUST_SDK_DIR
 
+        - name: Build RPC client (rust)
+          if: ${{ inputs.use_rust_sdk != '' }}
+          env:
+            RUST_SDK_LIB_RELATIVE: ${{ inputs.use_rust_sdk == '.' &&  '/target/debug' || '/complement-crypto/rust-sdk/target/debug'}}
+          run: |
+            export LIBRARY_PATH="$(pwd)$RUST_SDK_LIB_RELATIVE"
+            export LD_LIBRARY_PATH="$(pwd)$RUST_SDK_LIB_RELATIVE"
+            cd complement-crypto && go build -tags=rust ./cmd/rpc
+        - name: Build RPC client (js)
+          if: ${{ inputs.use_js_sdk != '' }}
+          run: |
+            cd complement-crypto && go build -tags=jssdk ./cmd/rpc
+
         # Run the tests
         - name: Run Complement Crypto Tests
           run: |
             export LIBRARY_PATH="$(pwd)$RUST_SDK_LIB_RELATIVE"
             export LD_LIBRARY_PATH="$(pwd)$RUST_SDK_LIB_RELATIVE"
+            export COMPLEMENT_CRYPTO_RPC_BINARY="$(pwd)/complement-crypto/rpc"
             cd complement-crypto &&
             set -o pipefail &&
             go test -v -json -tags=$GO_TAGS -timeout 15m ./tests | gotestfmt

.github/workflows/tests.yaml

@@ -16,12 +16,14 @@ jobs:
     uses: ./.github/workflows/single_sdk_tests.yml
     with:
       use_js_sdk: 'develop'
+      use_complement_crypto: '.'
 
   rust-latest-main:
     name: Tests (Rust only, latest main)
     uses: ./.github/workflows/single_sdk_tests.yml
     with:
       use_rust_sdk: '628374b8d86653e733649a506f5ae70385cd4de1' # TODO: go back to main when it works with uniffi 0.25 again
+      use_complement_crypto: '.'
 
   complement:
     name: Tests
@@ -47,7 +49,9 @@ jobs:
       - name: Checkout matrix-rust-sdk
         run: |
           mkdir rust-sdk
-          (wget -O - "https://github.com/matrix-org/matrix-rust-sdk/archive/kegan/complement-crypto.tar.gz" | tar -xz --strip-components=1 -C rust-sdk)
+          wget -O archive.tar.gz "https://github.com/matrix-org/matrix-rust-sdk/archive/kegan/complement-crypto.tar.gz"
+          zcat < archive.tar.gz | git get-tar-commit-id # useful for debugging
+          tar -xz --strip-components=1 -C rust-sdk < archive.tar.gz
       - uses: Swatinem/rust-cache@v2
         with:
           workspaces: "rust-sdk"
@@ -141,6 +145,7 @@ jobs:
       - run: |
           export LIBRARY_PATH="$(pwd)/rust-sdk/target/debug"
           export LD_LIBRARY_PATH="$(pwd)/rust-sdk/target/debug"
+          export COMPLEMENT_CRYPTO_RPC_BINARY="$(pwd)/rpc"
           set -o pipefail &&
           go test -v -json -tags='jssdk,rust' -timeout 15m ./tests | gotestfmt
         shell: bash # required for pipefail to be A Thing. pipefail is required to stop gotestfmt swallowing non-zero exit codes

ENVIRONMENT.md

@@ -4,6 +4,11 @@
 Complement-Crypto is configured exclusively through the use of environment variables. These variables are described below. Additional environment variables can be used, and are outlined at https://github.com/matrix-org/complement/blob/main/ENVIRONMENT.md 
 Complement-Crypto always runs in dirty mode (homeservers exist for the entire duration of the test suite) for performance reasons.
 
+#### `COMPLEMENT_CRYPTO_RPC_BINARY`
+The absolute path to the pre-built rpc binary file. This binary is generated via `go build -tags=jssdk,rust ./cmd/rpc`. This binary is used when running multiprocess tests. If this environment variable is not supplied, tests which try to use multiprocess clients will be skipped, making this environment variable optional.  
+- Type: `string`
+- Default: ""
+
 #### `COMPLEMENT_CRYPTO_TCPDUMP`
 If 1, automatically attempts to run `tcpdump` when the containers are running. Stops dumping when tests complete. This will probably require you to run `go test` with `sudo -E`. The `.pcap` file is written to `tests/test.pcap`.  
 - Type: `bool`

internal/config/config.go

@@ -41,6 +41,13 @@ type ComplementCrypto struct {
 	// tests complete. This will probably require you to run `go test` with `sudo -E`. The `.pcap` file is written to
 	// `tests/test.pcap`.
 	TCPDump bool
+
+	// Name: COMPLEMENT_CRYPTO_RPC_BINARY
+	// Default: ""
+	// Description: The absolute path to the pre-built rpc binary file. This binary is generated via `go build -tags=jssdk,rust ./cmd/rpc`.
+	// This binary is used when running multiprocess tests. If this environment variable is not supplied, tests which try to use multiprocess
+	// clients will be skipped, making this environment variable optional.
+	RPCBinaryPath string
 }
 
 func (c *ComplementCrypto) ShouldTest(lang api.ClientTypeLang) bool {
@@ -109,8 +116,15 @@ func NewComplementCryptoConfigFromEnvVars() *ComplementCrypto {
 	if len(testClientMatrix) == 0 {
 		panic("COMPLEMENT_CRYPTO_TEST_CLIENT_MATRIX: no tests will run as no matrix values are set")
 	}
+	rpcBinaryPath := os.Getenv("COMPLEMENT_CRYPTO_RPC_BINARY")
+	if rpcBinaryPath != "" {
+		if _, err := os.Stat(rpcBinaryPath); err != nil {
+			panic("COMPLEMENT_CRYPTO_RPC_BINARY must be the absolute path to a binary file: " + err.Error())
+		}
+	}
 	return &ComplementCrypto{
 		TCPDump:          os.Getenv("COMPLEMENT_CRYPTO_TCPDUMP") == "1",
+		RPCBinaryPath:    rpcBinaryPath,
 		TestClientMatrix: testClientMatrix,
 		clientLangs:      clientLangs,
 	}

tests/main_test.go

@@ -121,7 +121,8 @@ func WithPersistentStorage() func(*api.ClientCreationOpts) {
 
 // TestContext provides a consistent set of variables which most tests will need access to.
 type TestContext struct {
-	Deployment *deploy.SlidingSyncDeployment
+	Deployment    *deploy.SlidingSyncDeployment
+	RPCBinaryPath string
 	// Alice is defined if at least 1 clientType is provided to CreateTestContext.
 	Alice           *client.CSAPI
 	AliceClientType api.ClientType
@@ -145,7 +146,8 @@ type TestContext struct {
 func CreateTestContext(t *testing.T, clientType ...api.ClientType) *TestContext {
 	deployment := Deploy(t)
 	tc := &TestContext{
-		Deployment: deployment,
+		Deployment:    deployment,
+		RPCBinaryPath: complementCryptoConfig.RPCBinaryPath,
 	}
 	// pre-register alice and bob, if told
 	if len(clientType) > 0 {
@@ -184,6 +186,26 @@ func (c *TestContext) WithClientSyncing(t *testing.T, clientType api.ClientType,
 	callback(clientUnderTest)
 }
 
+// WithMultiprocessClientSyncing is the same as WithClientSyncing but it spins up the client in a separate process.
+// Communication is done via net/rpc internally.
+func (c *TestContext) WithMultiprocessClientSyncing(t *testing.T, lang api.ClientTypeLang, opts api.ClientCreationOpts, callback func(cli api.Client)) {
+	t.Helper()
+	if c.RPCBinaryPath == "" {
+		t.Skipf("RPC binary path not provided, skipping multiprocess test")
+		return
+	}
+	remoteBindings, err := deploy.NewRPCLanguageBindings(c.RPCBinaryPath, lang)
+	if err != nil {
+		t.Fatalf("Failed to create new RPC language bindings: %s", err)
+	}
+	remoteClient := remoteBindings.MustCreateClient(t, opts)
+	must.NotError(t, "failed to login client", remoteClient.Login(t, remoteClient.Opts()))
+	defer remoteClient.Close(t)
+	stopSyncing := remoteClient.MustStartSyncing(t)
+	defer stopSyncing()
+	callback(remoteClient)
+}
+
 // WithAliceSyncing is a helper function which creates a rust/js client and automatically logs in Alice and starts
 // a sync loop for her.
 //
@@ -329,20 +351,6 @@ func (encRoomOptions) RotationPeriodMsgs(numMsgs int) EncRoomOption {
 	}
 }
 
-// OptsFromClient converts a Complement client into a set of options which can be used to create an api.Client.
-func (c *TestContext) OptsFromClient(t *testing.T, existing *client.CSAPI, options ...func(*api.ClientCreationOpts)) api.ClientCreationOpts {
-	o := &api.ClientCreationOpts{
-		BaseURL:  existing.BaseURL,
-		UserID:   existing.UserID,
-		DeviceID: existing.DeviceID,
-		Password: existing.Password,
-	}
-	for _, opt := range options {
-		opt(o)
-	}
-	return *o
-}
-
 // MustRegisterNewDevice logs in a new device for this client, else fails the test.
 func (c *TestContext) MustRegisterNewDevice(t *testing.T, cli *client.CSAPI, hsName, newDeviceID string) *client.CSAPI {
 	return c.Deployment.Login(t, hsName, cli, helpers.LoginOpts{
@@ -351,16 +359,23 @@ func (c *TestContext) MustRegisterNewDevice(t *testing.T, cli *client.CSAPI, hsN
 	})
 }
 
+// ClientCreationOpts converts a Complement client into a set of real client options. Real client options are required in order to create
+// real rust/js clients.
+func (c *TestContext) ClientCreationOpts(t *testing.T, cli *client.CSAPI, hsName string, options ...func(*api.ClientCreationOpts)) api.ClientCreationOpts {
+	opts := api.NewClientCreationOpts(cli)
+	for _, opt := range options {
+		opt(&opts)
+	}
+	opts.SlidingSyncURL = c.Deployment.SlidingSyncURLForHS(t, hsName)
+	return opts
+}
+
 // MustCreateClient creates an api.Client from an existing Complement client and the specified client type. Additional options
 // can be set to configure the client beyond that of the Complement client e.g to add persistent storage.
 func (c *TestContext) MustCreateClient(t *testing.T, cli *client.CSAPI, clientType api.ClientType, options ...func(*api.ClientCreationOpts)) api.Client {
 	t.Helper()
-	cfg := api.NewClientCreationOpts(cli)
-	for _, opt := range options {
-		opt(&cfg)
-	}
-	cfg.SlidingSyncURL = c.Deployment.SlidingSyncURLForHS(t, clientType.HS)
-	client := MustCreateClient(t, clientType, cfg)
+	opts := c.ClientCreationOpts(t, cli, clientType.HS, options...)
+	client := MustCreateClient(t, clientType, opts)
 	return client
 }
 

tests/room_keys_test.go

@@ -3,15 +3,13 @@ package tests
 import (
 	"encoding/json"
 	"fmt"
-	"os"
 	"strings"
 	"testing"
 	"time"
 
 	"github.com/matrix-org/complement"
 	"github.com/matrix-org/complement-crypto/internal/api"
 	"github.com/matrix-org/complement-crypto/internal/deploy"
-	templates "github.com/matrix-org/complement-crypto/tests/go_templates"
 	"github.com/matrix-org/complement/client"
 	"github.com/matrix-org/complement/ct"
 	"github.com/matrix-org/complement/must"
@@ -366,7 +364,7 @@ func TestRoomKeyIsNotCycled(t *testing.T) {
 // in the room. This is important to ensure that we don't cycle m.room_keys too frequently, which increases
 // the chances of seeing undecryptable events.
 func TestRoomKeyIsNotCycledOnClientRestart(t *testing.T) {
-	ForEachClientType(t, func(tt *testing.T, a api.ClientType) {
+	ForEachClientType(t, func(t *testing.T, a api.ClientType) {
 		switch a.Lang {
 		case api.ClientTypeRust:
 			testRoomKeyIsNotCycledOnClientRestartRust(t, a)
@@ -389,40 +387,19 @@ func testRoomKeyIsNotCycledOnClientRestartRust(t *testing.T, clientType api.Clie
 	tc.Bob.MustJoinRoom(t, roomID, []string{clientType.HS})
 
 	tc.WithClientSyncing(t, clientType, tc.Bob, func(bob api.Client) {
-		wantMsgBody := "test from the script"
-
-		// run a script which will login as alice and then send an event in the room.
-		// We will wait on that event as Bob to know when the script got to that point.
-		cmd, close := templates.PrepareGoScript(t, "testRoomKeyIsNotCycledOnClientRestartRust/test.go",
-			struct {
-				UserID            string
-				DeviceID          string
-				Password          string
-				BaseURL           string
-				SSURL             string
-				PersistentStorage bool
-				Body              string
-				RoomID            string
-			}{
-				UserID:            tc.Alice.UserID,
-				Password:          tc.Alice.Password,
-				DeviceID:          tc.Alice.DeviceID,
-				BaseURL:           tc.Alice.BaseURL,
-				PersistentStorage: true,
-				SSURL:             bob.Opts().SlidingSyncURL,
-				Body:              wantMsgBody,
-				RoomID:            roomID,
-			})
-		cmd.WaitDelay = 3 * time.Second
-		defer close()
-		cmd.Stdout = os.Stdout
-		cmd.Stderr = os.Stderr
-		must.NotError(t, "failed to run script", cmd.Run())
+		wantMsgBody := "test from another process"
+		// send a message as Alice in a different process
+		tc.WithMultiprocessClientSyncing(t, clientType.Lang, tc.ClientCreationOpts(t, tc.Alice, clientType.HS, WithPersistentStorage()),
+			func(remoteAlice api.Client) {
+				eventID := remoteAlice.SendMessage(t, roomID, wantMsgBody)
+				waiter := remoteAlice.WaitUntilEventInRoom(t, roomID, api.CheckEventHasEventID(eventID))
+				waiter.Waitf(t, 5*time.Second, "client did not see event %s", eventID)
+			},
+		)
 
 		waiter := bob.WaitUntilEventInRoom(t, roomID, api.CheckEventHasBody(wantMsgBody))
 		waiter.Waitf(t, 8*time.Second, "bob did not see alice's message")
 
-		// the script sent the msg and exited cleanly.
 		// Now recreate the same client and make sure we don't send new room keys.
 
 		// we're going to sniff calls to /sendToDevice to ensure we do NOT see a new room key being sent.