From d976d8c5792454e04ab19b01db53a01f38131429 Mon Sep 17 00:00:00 2001 From: Alexey Palazhchenko Date: Fri, 22 Sep 2023 09:22:33 +0400 Subject: [PATCH] Run new PostgreSQL backend tests (#3407) Closes #3126. --- .github/workflows/call-test-integration.yml | 115 ++++++++++++++++++++ .github/workflows/go-cron.yml | 42 +++++++ .github/workflows/go.yml | 105 ++---------------- Taskfile.yml | 23 ++++ internal/backends/backend.go | 1 - internal/backends/collection.go | 1 - internal/backends/database.go | 1 - internal/backends/doc.go | 6 +- internal/handlers/sqlite/sqlite.go | 10 +- 9 files changed, 199 insertions(+), 105 deletions(-) create mode 100644 .github/workflows/call-test-integration.yml create mode 100644 .github/workflows/go-cron.yml diff --git a/.github/workflows/call-test-integration.yml b/.github/workflows/call-test-integration.yml new file mode 100644 index 000000000000..37b12f966d7e --- /dev/null +++ b/.github/workflows/call-test-integration.yml @@ -0,0 +1,115 @@ +--- +on: + workflow_call: + inputs: + task: + required: true + type: string + shard_index: + required: true + type: number + shard_total: + required: true + type: number + disable_filter_pushdown: + required: false + type: boolean + default: false + enable_sort_pushdown: + required: false + type: boolean + default: false + +env: + GOPATH: /home/runner/go + GOCACHE: /home/runner/go/cache + GOLANGCI_LINT_CACHE: /home/runner/go/cache/lint + GOMODCACHE: /home/runner/go/mod + GOPROXY: https://proxy.golang.org + GOTOOLCHAIN: local + +jobs: + test-integration: + # make it short to fit in GitHub UI; all parameters are already in the caller's name + name: Run + + runs-on: + group: paid + + timeout-minutes: 20 + + steps: + - name: Setup permissions monitoring + uses: GitHubSecurityLab/actions-permissions/monitor@v1 + if: false + + - name: Checkout code + uses: actions/checkout@v3 + with: + fetch-depth: 0 # for `git describe` to work + lfs: false # LFS is used only by website + + - name: Setup Go + uses: FerretDB/github-actions/setup-go@main + with: + cache-key: integration + + - name: Install Task + run: go generate -x + working-directory: tools + + - name: Start environment + run: bin/task env-up-detach + + - name: Run init + run: bin/task init + + - name: Wait for and setup environment + run: bin/task env-setup + + - name: > + Run ${{ inputs.task }} tests + (${{ inputs.shard_index }}/${{ inputs.shard_total }}, + filter=${{ !inputs.disable_filter_pushdown }}, sort=${{ inputs.enable_sort_pushdown }}) + run: > + bin/task test-integration-${{ inputs.task }} + SHARD_INDEX=${{ inputs.shard_index }} + SHARD_TOTAL=${{ inputs.shard_total }} + DISABLE_FILTER_PUSHDOWN=${{ inputs.disable_filter_pushdown }} + ENABLE_SORT_PUSHDOWN=${{ inputs.enable_sort_pushdown }} + env: + GOFLAGS: ${{ runner.debug == '1' && '-v' || '' }} + + # The token is not required but should make uploads more stable. + # If secrets are unavailable (for example, for a pull request from a fork), it fallbacks to the tokenless uploads. + # + # Unfortunately, it seems that tokenless uploads fail too often. + # See https://github.com/codecov/codecov-action/issues/837. + # + # We also can't use ${{ vars.CODECOV_TOKEN }}: https://github.com/orgs/community/discussions/44322 + - name: Upload coverage information to codecov + if: always() + uses: codecov/codecov-action@v3 + with: + token: 22159d7c-856d-4fe9-8fdb-5d9ecff35514 + files: ./integration/integration-${{ inputs.task }}.txt + flags: integration,${{ inputs.task }}-${{ inputs.shard_index }},filter-${{ !inputs.disable_filter_pushdown }},sort-${{ inputs.enable_sort_pushdown }} + fail_ci_if_error: true + verbose: true + + - name: Upload coverage information to coveralls + uses: coverallsapp/github-action@v2 + with: + file: ./integration/integration-${{ inputs.task }}.txt + flag-name: integration-${{ inputs.task }}-${{ inputs.shard_index }}-filter-${{ !inputs.disable_filter_pushdown }}-sort-${{ inputs.enable_sort_pushdown }} + parallel: true + + # we don't want them on CI + - name: Clean test and fuzz caches + if: always() + run: go clean -testcache -fuzzcache + + - name: Check dirty + run: | + git status + git diff --exit-code diff --git a/.github/workflows/go-cron.yml b/.github/workflows/go-cron.yml new file mode 100644 index 000000000000..646d618843cd --- /dev/null +++ b/.github/workflows/go-cron.yml @@ -0,0 +1,42 @@ +--- +name: Go Cron +on: + schedule: + - cron: "12 0 * * *" + +env: + GOPATH: /home/runner/go + GOCACHE: /home/runner/go/cache + GOLANGCI_LINT_CACHE: /home/runner/go/cache/lint + GOMODCACHE: /home/runner/go/mod + GOPROXY: https://proxy.golang.org + GOTOOLCHAIN: local + +jobs: + integration: + # job name must be unique; make it unique and nice + name: > + ${{ matrix.task }} ${{ matrix.shard_index }}/${{ matrix.shard_total }} + (filter=${{ matrix.disable_filter_pushdown }}, sort=${{ matrix.enable_sort_pushdown }}) + + # To avoid conflict with go.yml. + concurrency: + group: ${{ github.workflow }}-integration-${{ matrix.task }}-${{ matrix.shard_index }}-${{ matrix.disable_filter_pushdown }}-${{ matrix.enable_sort_pushdown }}-${{ github.head_ref || github.ref_name }} + cancel-in-progress: true + + strategy: + fail-fast: false + matrix: + task: [pg, postgresql, sqlite] + shard_index: [1, 2, 3] + shard_total: [3] + disable_filter_pushdown: [false, true] + enable_sort_pushdown: [false, true] + + uses: ./.github/workflows/call-test-integration.yml + with: + task: ${{ matrix.task }} + shard_index: ${{ matrix.shard_index }} + shard_total: ${{ matrix.shard_total }} + disable_filter_pushdown: ${{ matrix.disable_filter_pushdown }} + enable_sort_pushdown: ${{ matrix.enable_sort_pushdown }} diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 2430c400f17d..3efbbc11b648 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -160,17 +160,13 @@ jobs: git diff --exit-code integration: - name: > - Integration ${{ matrix.name }} ${{ matrix.shard_index }}/${{ matrix.shard_total }} - (pushdown=${{ !(matrix.disable_filter_pushdown || false) }}) - runs-on: - group: paid - timeout-minutes: 20 + # job name must be unique; make it unique and nice + name: ${{ matrix.task }} ${{ matrix.shard_index }}/${{ matrix.shard_total }} # Do not run this job in parallel for any PR change or branch push # to save some resources. concurrency: - group: ${{ github.workflow }}-integration-${{ matrix.name }}-${{ matrix.shard_index }}-${{ matrix.disable_filter_pushdown || false }}-${{ github.head_ref || github.ref_name }} + group: ${{ github.workflow }}-integration-${{ matrix.task }}-${{ matrix.shard_index }}-${{ github.head_ref || github.ref_name }} cancel-in-progress: true if: github.event_name != 'pull_request' || !contains(github.event.pull_request.labels.*.name, 'not ready') @@ -178,94 +174,17 @@ jobs: strategy: fail-fast: false matrix: + task: [pg, postgresql, sqlite] + shard_index: [1, 2, 3] + shard_total: [3] include: - - { name: "MongoDB", task: "mongodb", shard_index: 1, shard_total: 1 } - - { name: "PostgreSQL", task: "pg", shard_index: 1, shard_total: 3 } - - { name: "PostgreSQL", task: "pg", shard_index: 2, shard_total: 3 } - - { name: "PostgreSQL", task: "pg", shard_index: 3, shard_total: 3 } - - { name: "SQLite", task: "sqlite", shard_index: 1, shard_total: 3 } - - { name: "SQLite", task: "sqlite", shard_index: 2, shard_total: 3 } - - { name: "SQLite", task: "sqlite", shard_index: 3, shard_total: 3 } - - # move that to cron-only workflow - # TODO https://github.com/FerretDB/FerretDB/issues/3126 - - { name: "SQLite", task: "sqlite", shard_index: 1, shard_total: 3, disable_filter_pushdown: true } - - { name: "SQLite", task: "sqlite", shard_index: 2, shard_total: 3, disable_filter_pushdown: true } - - { name: "SQLite", task: "sqlite", shard_index: 3, shard_total: 3, disable_filter_pushdown: true } + - { task: "mongodb", shard_index: 1, shard_total: 1 } - steps: - - name: Setup permissions monitoring - uses: GitHubSecurityLab/actions-permissions/monitor@v1 - if: false - - - name: Checkout code - uses: actions/checkout@v3 - with: - fetch-depth: 0 # for `git describe` to work - lfs: false # LFS is used only by website - - - name: Setup Go - uses: FerretDB/github-actions/setup-go@main - with: - cache-key: integration - - - name: Install Task - run: go generate -x - working-directory: tools - - - name: Start environment - run: bin/task env-up-detach - - - name: Run init - run: bin/task init - - - name: Wait for and setup environment - run: bin/task env-setup - - - name: > - Run ${{ matrix.name }} tests - (${{ matrix.shard_index }}/${{ matrix.shard_total }}, pushdown=${{ !(matrix.disable_filter_pushdown || false) }}) - run: > - bin/task test-integration-${{ matrix.task }} - SHARD_INDEX=${{ matrix.shard_index }} - SHARD_TOTAL=${{ matrix.shard_total }} - DISABLE_FILTER_PUSHDOWN=${{ matrix.disable_filter_pushdown || false }} - env: - GOFLAGS: ${{ runner.debug == '1' && '-v' || '' }} - - # The token is not required but should make uploads more stable. - # If secrets are unavailable (for example, for a pull request from a fork), it fallbacks to the tokenless uploads. - # - # Unfortunately, it seems that tokenless uploads fail too often. - # See https://github.com/codecov/codecov-action/issues/837. - # - # We also can't use ${{ vars.CODECOV_TOKEN }}: https://github.com/orgs/community/discussions/44322 - - name: Upload coverage information to codecov - if: always() - uses: codecov/codecov-action@v3 - with: - token: 22159d7c-856d-4fe9-8fdb-5d9ecff35514 - files: ./integration/integration-${{ matrix.task }}.txt - flags: integration,${{ matrix.task }}-${{ matrix.shard_index }},pushdown-${{ !(matrix.disable_filter_pushdown || false) }} - fail_ci_if_error: true - verbose: true - - - name: Upload coverage information to coveralls - uses: coverallsapp/github-action@v2 - with: - file: ./integration/integration-${{ matrix.task }}.txt - flag-name: integration-${{ matrix.task }}-${{ matrix.shard_index }}-pushdown-${{ !(matrix.disable_filter_pushdown || false) }} - parallel: true - - # we don't want them on CI - - name: Clean test and fuzz caches - if: always() - run: go clean -testcache -fuzzcache - - - name: Check dirty - run: | - git status - git diff --exit-code + uses: ./.github/workflows/call-test-integration.yml + with: + task: ${{ matrix.task }} + shard_index: ${{ matrix.shard_index }} + shard_total: ${{ matrix.shard_total }} # https://github.com/lemurheavy/coveralls-public/issues/1636 submit-coveralls: diff --git a/Taskfile.yml b/Taskfile.yml index 65072eafb337..feaa1e3b5661 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -9,6 +9,7 @@ vars: SHARD_INDEX: 1 SHARD_TOTAL: 1 DISABLE_FILTER_PUSHDOWN: false + ENABLE_SORT_PUSHDOWN: false TEST_RUN: "" TEST_TIMEOUT: 35m BENCH_TIME: 5s @@ -188,6 +189,7 @@ tasks: - test-integration-pg - test-integration-mongodb - test-integration-sqlite + # no test-integration-postgresql yet # no test-integration-hana test-integration-pg: @@ -201,6 +203,26 @@ tasks: -target-tls -postgresql-url='postgres://username@127.0.0.1:5432/ferretdb?pool_max_conns=50&search_path=' -compat-url='mongodb://username:password@127.0.0.1:47018/?tls=true&tlsCertificateKeyFile=../build/certs/client.pem&tlsCaFile=../build/certs/rootCA-cert.pem' + -disable-filter-pushdown={{.DISABLE_FILTER_PUSHDOWN}} + -enable-sort-pushdown={{.ENABLE_SORT_PUSHDOWN}} + vars: + SHARD_RUN: + sh: go run -C .. ./cmd/envtool tests shard --index={{.SHARD_INDEX}} --total={{.SHARD_TOTAL}} + + test-integration-postgresql: + desc: "Run integration tests for new `postgresql` backend" + dir: integration + cmds: + - > + go test -count=1 -run='{{or .TEST_RUN .SHARD_RUN}}' -timeout={{.TEST_TIMEOUT}} {{.RACE_FLAG}} -tags={{.BUILD_TAGS}} -shuffle=on -coverpkg=../... + -coverprofile=integration-postgresql.txt + -target-backend=ferretdb-postgresql + -target-tls + -postgresql-url='postgres://username@127.0.0.1:5432/ferretdb?pool_max_conns=50&search_path=' + -compat-url='mongodb://username:password@127.0.0.1:47018/?tls=true&tlsCertificateKeyFile=../build/certs/client.pem&tlsCaFile=../build/certs/rootCA-cert.pem' + -use-new-pg + -disable-filter-pushdown={{.DISABLE_FILTER_PUSHDOWN}} + -enable-sort-pushdown={{.ENABLE_SORT_PUSHDOWN}} vars: SHARD_RUN: sh: go run -C .. ./cmd/envtool tests shard --index={{.SHARD_INDEX}} --total={{.SHARD_TOTAL}} @@ -217,6 +239,7 @@ tasks: -target-tls -compat-url='mongodb://username:password@127.0.0.1:47018/?tls=true&tlsCertificateKeyFile=../build/certs/client.pem&tlsCaFile=../build/certs/rootCA-cert.pem' -disable-filter-pushdown={{.DISABLE_FILTER_PUSHDOWN}} + -enable-sort-pushdown={{.ENABLE_SORT_PUSHDOWN}} vars: SHARD_RUN: sh: go run -C .. ./cmd/envtool tests shard --index={{.SHARD_INDEX}} --total={{.SHARD_TOTAL}} diff --git a/internal/backends/backend.go b/internal/backends/backend.go index 315597dd2c89..0ec3646585e7 100644 --- a/internal/backends/backend.go +++ b/internal/backends/backend.go @@ -47,7 +47,6 @@ type Backend interface { prometheus.Collector // There is no interface method to create a database; see package documentation. - // TODO https://github.com/FerretDB/FerretDB/issues/3069 } // backendContract implements Backend interface. diff --git a/internal/backends/collection.go b/internal/backends/collection.go index d6d01b3084cc..c68bf883260b 100644 --- a/internal/backends/collection.go +++ b/internal/backends/collection.go @@ -107,7 +107,6 @@ type InsertAllResult struct{} // They will be frozen. // // Both database and collection may or may not exist; they should be created automatically if needed. -// TODO https://github.com/FerretDB/FerretDB/issues/3069 func (cc *collectionContract) InsertAll(ctx context.Context, params *InsertAllParams) (*InsertAllResult, error) { defer observability.FuncCall(ctx)() diff --git a/internal/backends/database.go b/internal/backends/database.go index 685743c42240..aca11bd7832f 100644 --- a/internal/backends/database.go +++ b/internal/backends/database.go @@ -106,7 +106,6 @@ type CreateCollectionParams struct { // CreateCollection creates a new collection with valid name in the database; it should not already exist. // // Database may or may not exist; it should be created automatically if needed. -// TODO https://github.com/FerretDB/FerretDB/issues/3069 func (dbc *databaseContract) CreateCollection(ctx context.Context, params *CreateCollectionParams) error { defer observability.FuncCall(ctx)() diff --git a/internal/backends/doc.go b/internal/backends/doc.go index 5c7ebfdcbd71..9832c151ae51 100644 --- a/internal/backends/doc.go +++ b/internal/backends/doc.go @@ -12,7 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package backends provides common interfaces and code for all backend implementations. +// Package backends provides common interfaces ([Backend], [Database], and [Collection]) +// and code for all backend implementations. // // # Design principles // @@ -35,7 +36,4 @@ // Contracts enforce error codes; they are not documented in the code comments // but are visible in the contract's code (to avoid duplication). // Methods should return different error codes only if the difference is important for the handler. -// -// Update, expand, etc. -// TODO https://github.com/FerretDB/FerretDB/issues/3069 package backends diff --git a/internal/handlers/sqlite/sqlite.go b/internal/handlers/sqlite/sqlite.go index 85356cec785a..74ec26faf905 100644 --- a/internal/handlers/sqlite/sqlite.go +++ b/internal/handlers/sqlite/sqlite.go @@ -67,6 +67,7 @@ func New(opts *NewOpts) (handlers.Interface, error) { b, err = postgresql.NewBackend(&postgresql.NewBackendParams{ URI: opts.URI, L: opts.L, + P: opts.StateProvider, }) case "sqlite": b, err = sqlite.NewBackend(&sqlite.NewBackendParams{ @@ -74,11 +75,6 @@ func New(opts *NewOpts) (handlers.Interface, error) { L: opts.L, P: opts.StateProvider, }) - - if opts.EnableOplog { - b = oplog.NewBackend(b, opts.L.Named("oplog")) - } - default: panic("unknown backend: " + opts.Backend) } @@ -87,6 +83,10 @@ func New(opts *NewOpts) (handlers.Interface, error) { return nil, err } + if opts.EnableOplog { + b = oplog.NewBackend(b, opts.L.Named("oplog")) + } + return &Handler{ b: b, NewOpts: opts,