From 9a46d8d95c937bdb356f9e373424e1a37aa37f04 Mon Sep 17 00:00:00 2001 From: Till <2353100+S7evinK@users.noreply.github.com> Date: Fri, 2 Dec 2022 11:44:20 +0100 Subject: [PATCH] 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 | 75 ++++++++++++++++++++++++-- .github/workflows/schedules.yaml | 82 ++++------------------------- cmd/dendrite-upgrade-tests/main.go | 43 +++++++++++++++ docs/CONTRIBUTING.md | 15 +++++- internal/pushgateway/client.go | 5 +- internal/pushgateway/client_test.go | 54 +++++++++++++++++++ 6 files changed, 197 insertions(+), 77 deletions(-) create mode 100644 internal/pushgateway/client_test.go diff --git a/.github/workflows/dendrite.yml b/.github/workflows/dendrite.yml index fa428238..593012ef 100644 --- a/.github/workflows/dendrite.yml +++ b/.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 diff --git a/.github/workflows/schedules.yaml b/.github/workflows/schedules.yaml index ff4d4718..d2a1f6e1 100644 --- a/.github/workflows/schedules.yaml +++ b/.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 diff --git a/cmd/dendrite-upgrade-tests/main.go b/cmd/dendrite-upgrade-tests/main.go index 75446d18..39b9320c 100644 --- a/cmd/dendrite-upgrade-tests/main.go +++ b/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 } diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index 6ba05f46..262a93a7 100644 --- a/docs/CONTRIBUTING.md +++ b/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 diff --git a/internal/pushgateway/client.go b/internal/pushgateway/client.go index 95f5afd9..259239b8 100644 --- a/internal/pushgateway/client.go +++ b/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 hresp.Body.Close() + defer internal.CloseAndLogIfError(ctx, hresp.Body, "failed to close response body") if hresp.StatusCode == http.StatusOK { return json.NewDecoder(hresp.Body).Decode(resp) diff --git a/internal/pushgateway/client_test.go b/internal/pushgateway/client_test.go new file mode 100644 index 00000000..bd0dca47 --- /dev/null +++ b/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") + } +}