Test and CI related changes (#2896)

In an attempt to:
- make on-boarding a bit easier (`go test ./...` should now not need
additional postgres setup)
- get code coverage faster, not only scheduled at night
- test the `create-account` binary

.github/workflows/dendrite.yml

@@ -68,7 +68,7 @@ jobs:
 
   # run go test with different go versions
   test:
-    timeout-minutes: 5
+    timeout-minutes: 10
     name: Unit tests (Go ${{ matrix.go }})
     runs-on: ubuntu-latest
     # Service containers to run with `container-job`
@@ -94,14 +94,22 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        go: ["1.18", "1.19"]
+        go: ["1.19"]
     steps:
       - uses: actions/checkout@v3
       - name: Setup go
         uses: actions/setup-go@v3
         with:
           go-version: ${{ matrix.go }}
-          cache: true
+      - uses: actions/cache@v3
+        # manually set up caches, as they otherwise clash with different steps using setup-go with cache=true
+        with:
+          path: |
+            ~/.cache/go-build
+            ~/go/pkg/mod
+          key: ${{ runner.os }}-go${{ matrix.go }}-unit-${{ hashFiles('**/go.sum') }}
+          restore-keys: |
+            ${{ runner.os }}-go${{ matrix.go }}-unit-
       - name: Set up gotestfmt
         uses: gotesttools/gotestfmt-action@v2
         with:
@@ -194,6 +202,66 @@ jobs:
         with:
           jobs: ${{ toJSON(needs) }}
 
+  # run go test with different go versions
+  integration:
+    timeout-minutes: 20
+    needs: initial-tests-done
+    name: Integration tests (Go ${{ matrix.go }})
+    runs-on: ubuntu-latest
+    # Service containers to run with `container-job`
+    services:
+      # Label used to access the service container
+      postgres:
+        # Docker Hub image
+        image: postgres:13-alpine
+        # Provide the password for postgres
+        env:
+          POSTGRES_USER: postgres
+          POSTGRES_PASSWORD: postgres
+          POSTGRES_DB: dendrite
+        ports:
+          # Maps tcp port 5432 on service container to the host
+          - 5432:5432
+        # Set health checks to wait until postgres has started
+        options: >-
+          --health-cmd pg_isready
+          --health-interval 10s
+          --health-timeout 5s
+          --health-retries 5
+    strategy:
+      fail-fast: false
+      matrix:
+        go: ["1.19"]
+    steps:
+      - uses: actions/checkout@v3
+      - name: Setup go
+        uses: actions/setup-go@v3
+        with:
+          go-version: ${{ matrix.go }}
+      - name: Set up gotestfmt
+        uses: gotesttools/gotestfmt-action@v2
+        with:
+          # Optional: pass GITHUB_TOKEN to avoid rate limiting.
+          token: ${{ secrets.GITHUB_TOKEN }}
+      - uses: actions/cache@v3
+        with:
+          path: |
+            ~/.cache/go-build
+            ~/go/pkg/mod
+          key: ${{ runner.os }}-go${{ matrix.go }}-test-race-${{ hashFiles('**/go.sum') }}
+          restore-keys: |
+            ${{ runner.os }}-go${{ matrix.go }}-test-race-
+      - run: go test -race -json -v -coverpkg=./... -coverprofile=cover.out $(go list ./... | grep -v /cmd/dendrite*) 2>&1 | gotestfmt
+        env:
+          POSTGRES_HOST: localhost
+          POSTGRES_USER: postgres
+          POSTGRES_PASSWORD: postgres
+          POSTGRES_DB: dendrite
+      - name: Upload coverage to Codecov
+        uses: codecov/codecov-action@v3
+        with:
+          flags: unittests
+
   # run database upgrade tests
   upgrade_test:
     name: Upgrade tests
@@ -404,6 +472,7 @@ jobs:
         upgrade_test_direct,
         sytest,
         complement,
+        integration
       ]
     runs-on: ubuntu-latest
     if: ${{ !cancelled() }} # Run this even if prior jobs were skipped

.github/workflows/schedules.yaml

@@ -10,79 +10,9 @@ concurrency:
   cancel-in-progress: true
 
 jobs:
-  # run go test with different go versions
-  test:
-    timeout-minutes: 20
-    name: Unit tests (Go ${{ matrix.go }})
-    runs-on: ubuntu-latest
-    # Service containers to run with `container-job`
-    services:
-      # Label used to access the service container
-      postgres:
-        # Docker Hub image
-        image: postgres:13-alpine
-        # Provide the password for postgres
-        env:
-          POSTGRES_USER: postgres
-          POSTGRES_PASSWORD: postgres
-          POSTGRES_DB: dendrite
-        ports:
-          # Maps tcp port 5432 on service container to the host
-          - 5432:5432
-        # Set health checks to wait until postgres has started
-        options: >-
-          --health-cmd pg_isready
-          --health-interval 10s
-          --health-timeout 5s
-          --health-retries 5
-    strategy:
-      fail-fast: false
-      matrix:
-        go: ["1.18", "1.19"]
-    steps:
-      - uses: actions/checkout@v3
-      - name: Setup go
-        uses: actions/setup-go@v3
-        with:
-          go-version: ${{ matrix.go }}
-      - name: Set up gotestfmt
-        uses: gotesttools/gotestfmt-action@v2
-        with:
-          # Optional: pass GITHUB_TOKEN to avoid rate limiting.
-          token: ${{ secrets.GITHUB_TOKEN }}
-      - uses: actions/cache@v3
-        with:
-          path: |
-            ~/.cache/go-build
-            ~/go/pkg/mod
-          key: ${{ runner.os }}-go${{ matrix.go }}-test-race-${{ hashFiles('**/go.sum') }}
-          restore-keys: |
-            ${{ runner.os }}-go${{ matrix.go }}-test-race-
-      - run: go test -race -json -v -coverpkg=./... -coverprofile=cover.out $(go list ./... | grep -v /cmd/dendrite*) 2>&1 | gotestfmt
-        env:
-          POSTGRES_HOST: localhost
-          POSTGRES_USER: postgres
-          POSTGRES_PASSWORD: postgres
-          POSTGRES_DB: dendrite
-      - name: Upload coverage to Codecov
-        uses: codecov/codecov-action@v3
-
-  # Dummy step to gate other tests on without repeating the whole list
-  initial-tests-done:
-    name: Initial tests passed
-    needs: [test]
-    runs-on: ubuntu-latest
-    if: ${{ !cancelled() }} # Run this even if prior jobs were skipped
-    steps:
-      - name: Check initial tests passed
-        uses: re-actors/alls-green@release/v1
-        with:
-          jobs: ${{ toJSON(needs) }}
-
   # run Sytest in different variations
   sytest:
     timeout-minutes: 60
-    needs: initial-tests-done
     name: "Sytest (${{ matrix.label }})"
     runs-on: ubuntu-latest
     strategy:
@@ -104,13 +34,23 @@ jobs:
       image: matrixdotorg/sytest-dendrite:latest
       volumes:
         - ${{ github.workspace }}:/src
+        - /root/.cache/go-build:/github/home/.cache/go-build
+        - /root/.cache/go-mod:/gopath/pkg/mod
       env:
         POSTGRES: ${{ matrix.postgres && 1}}
         API: ${{ matrix.api && 1 }}
         SYTEST_BRANCH: ${{ github.head_ref }}
         RACE_DETECTION: 1
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
+      - uses: actions/cache@v3
+        with:
+          path: |
+            ~/.cache/go-build
+            /gopath/pkg/mod
+          key: ${{ runner.os }}-go-sytest-${{ hashFiles('**/go.sum') }}
+          restore-keys: |
+            ${{ runner.os }}-go-sytest-
       - name: Run Sytest
         run: /bootstrap.sh dendrite
         working-directory: /src

cmd/dendrite-upgrade-tests/main.go

@@ -7,6 +7,7 @@ import (
 	"flag"
 	"fmt"
 	"io"
+	"io/ioutil"
 	"log"
 	"net/http"
 	"os"
@@ -61,6 +62,7 @@ COPY . .
 RUN go build ./cmd/dendrite-monolith-server
 RUN go build ./cmd/generate-keys
 RUN go build ./cmd/generate-config
+RUN go build ./cmd/create-account
 RUN ./generate-config --ci > dendrite.yaml
 RUN ./generate-keys --private-key matrix_key.pem --tls-cert server.crt --tls-key server.key
 
@@ -104,6 +106,7 @@ COPY . .
 RUN go build ./cmd/dendrite-monolith-server
 RUN go build ./cmd/generate-keys
 RUN go build ./cmd/generate-config
+RUN go build ./cmd/create-account
 RUN ./generate-config --ci > dendrite.yaml
 RUN ./generate-keys --private-key matrix_key.pem --tls-cert server.crt --tls-key server.key
 
@@ -458,6 +461,46 @@ func loadAndRunTests(dockerClient *client.Client, volumeName, v string, branchTo
 	if err = runTests(csAPIURL, v); err != nil {
 		return fmt.Errorf("failed to run tests on version %s: %s", v, err)
 	}
+
+	err = testCreateAccount(dockerClient, v, containerID)
+	if err != nil {
+		return err
+	}
+	return nil
+}
+
+// test that create-account is working
+func testCreateAccount(dockerClient *client.Client, v string, containerID string) error {
+	createUser := strings.ToLower("createaccountuser-" + v)
+	log.Printf("%s: Creating account %s with create-account\n", v, createUser)
+
+	respID, err := dockerClient.ContainerExecCreate(context.Background(), containerID, types.ExecConfig{
+		AttachStderr: true,
+		AttachStdout: true,
+		Cmd: []string{
+			"/build/create-account",
+			"-username", createUser,
+			"-password", "someRandomPassword",
+		},
+	})
+	if err != nil {
+		return fmt.Errorf("failed to ContainerExecCreate: %w", err)
+	}
+
+	response, err := dockerClient.ContainerExecAttach(context.Background(), respID.ID, types.ExecStartCheck{})
+	if err != nil {
+		return fmt.Errorf("failed to attach to container: %w", err)
+	}
+	defer response.Close()
+
+	data, err := ioutil.ReadAll(response.Reader)
+	if err != nil {
+		return err
+	}
+
+	if !bytes.Contains(data, []byte("AccessToken")) {
+		return fmt.Errorf("failed to create-account: %s", string(data))
+	}
 	return nil
 }
 

docs/CONTRIBUTING.md

@@ -75,7 +75,20 @@ comment. Please avoid doing this if you can.
 We also have unit tests which we run via:
 
 ```bash
-go test --race ./...
+DENDRITE_TEST_SKIP_NODB=1 go test --race ./...
+```
+
+This only runs SQLite database tests. If you wish to execute Postgres tests as well, you'll either need to 
+have Postgres installed locally (`createdb` will be used) or have a remote/containerized Postgres instance
+available.
+
+To configure the connection to a remote Postgres, you can use the following enviroment variables:
+
+```bash
+POSTGRES_USER=postgres
+POSTGERS_PASSWORD=yourPostgresPassword
+POSTGRES_HOST=localhost
+POSTGRES_DB=postgres # the superuser database to use
 ```
 
 In general, we like submissions that come with tests. Anything that proves that the

internal/pushgateway/client.go

@@ -9,6 +9,8 @@ import (
 	"net/http"
 	"time"
 
+	"github.com/matrix-org/dendrite/internal"
+
 	"github.com/opentracing/opentracing-go"
 )
 
@@ -50,8 +52,7 @@ func (h *httpClient) Notify(ctx context.Context, url string, req *NotifyRequest,
 		return err
 	}
 
-	//nolint:errcheck
+	defer internal.CloseAndLogIfError(ctx, hresp.Body, "failed to close response body")
-	defer hresp.Body.Close()
 
 	if hresp.StatusCode == http.StatusOK {
 		return json.NewDecoder(hresp.Body).Decode(resp)

internal/pushgateway/client_test.go

@@ -0,0 +1,54 @@
+package pushgateway
+
+import (
+	"context"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"reflect"
+	"testing"
+)
+
+func TestNotify(t *testing.T) {
+	wantResponse := NotifyResponse{
+		Rejected: []string{"testing"},
+	}
+
+	var i = 0
+
+	svr := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// /notify only accepts POST requests
+		if r.Method != http.MethodPost {
+			w.WriteHeader(http.StatusNotImplemented)
+			return
+		}
+
+		if i != 0 { // error path
+			w.WriteHeader(http.StatusBadRequest)
+			return
+		}
+
+		// happy path
+		json.NewEncoder(w).Encode(wantResponse)
+	}))
+	defer svr.Close()
+
+	cl := NewHTTPClient(true)
+	gotResponse := NotifyResponse{}
+
+	// Test happy path
+	err := cl.Notify(context.Background(), svr.URL, &NotifyRequest{}, &gotResponse)
+	if err != nil {
+		t.Errorf("failed to notify client")
+	}
+	if !reflect.DeepEqual(gotResponse, wantResponse) {
+		t.Errorf("expected response %+v, got %+v", wantResponse, gotResponse)
+	}
+
+	// Test error path
+	i++
+	err = cl.Notify(context.Background(), svr.URL, &NotifyRequest{}, &gotResponse)
+	if err == nil {
+		t.Errorf("expected notifying the pushgateway to fail, but it succeeded")
+	}
+}