Skip to content

chore: run macOS, windows, and race tests with Postgres in CI #15520

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 23 commits into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .github/actions/setup-imdisk/action.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
name: "Setup ImDisk"
if: runner.os == 'Windows'
description: |
Sets up the ImDisk toolkit for Windows and creates a RAM disk on drive R:.
runs:
using: "composite"
steps:
- name: Download ImDisk
if: runner.os == 'Windows'
shell: bash
run: |
mkdir imdisk
cd imdisk
curl -L -o files.cab https://github.com/coder/imdisk-artifacts/raw/92a17839ebc0ee3e69be019f66b3e9b5d2de4482/files.cab
curl -L -o install.bat https://github.com/coder/imdisk-artifacts/raw/92a17839ebc0ee3e69be019f66b3e9b5d2de4482/install.bat
Comment on lines +14 to +15
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sucks that people still use sourceforge lol

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's Windows for you

cd ..

- name: Install ImDisk
shell: cmd
run: |
cd imdisk
install.bat /silent

- name: Create RAM Disk
shell: cmd
run: |
imdisk -a -s 4096M -m R: -p "/fs:ntfs /q /y"
90 changes: 86 additions & 4 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -370,15 +370,20 @@ jobs:
api-key: ${{ secrets.DATADOG_API_KEY }}

test-go-pg:
runs-on: ${{ github.repository_owner == 'coder' && 'depot-ubuntu-22.04-8' || 'ubuntu-latest' }}
needs:
- changes
runs-on: ${{ matrix.os == 'ubuntu-latest' && github.repository_owner == 'coder' && 'depot-ubuntu-22.04-4' || matrix.os == 'macos-latest' && github.repository_owner == 'coder' && 'macos-latest-xlarge' || matrix.os == 'windows-2022' && github.repository_owner == 'coder' && 'windows-latest-16-cores' || matrix.os }}
needs: changes
if: needs.changes.outputs.go == 'true' || needs.changes.outputs.ci == 'true' || github.ref == 'refs/heads/main'
# This timeout must be greater than the timeout set by `go test` in
# `make test-postgres` to ensure we receive a trace of running
# goroutines. Setting this to the timeout +5m should work quite well
# even if some of the preceding steps are slow.
timeout-minutes: 25
strategy:
matrix:
os:
- ubuntu-latest
- macos-latest
- windows-2022
steps:
- name: Harden Runner
uses: step-security/harden-runner@0080882f6c36860b6ba35c610c98ce87d4e2f26f # v2.10.2
Expand All @@ -396,12 +401,46 @@ jobs:
- name: Setup Terraform
uses: ./.github/actions/setup-tf

# Sets up the ImDisk toolkit for Windows and creates a RAM disk on drive R:.
- name: Setup ImDisk
if: runner.os == 'Windows'
uses: ./.github/actions/setup-imdisk

- name: Test with PostgreSQL Database
env:
POSTGRES_VERSION: "13"
TS_DEBUG_DISCO: "true"
shell: bash
run: |
make test-postgres
# if macOS, install google-chrome for scaletests
# As another concern, should we really have this kind of external dependency
# requirement on standard CI?
if [ "${{ matrix.os }}" == "macos-latest" ]; then
brew install google-chrome
fi

# By default Go will use the number of logical CPUs, which
# is a fine default.
PARALLEL_FLAG=""

# macOS will output "The default interactive shell is now zsh"
# intermittently in CI...
if [ "${{ matrix.os }}" == "macos-latest" ]; then
touch ~/.bash_profile && echo "export BASH_SILENCE_DEPRECATION_WARNING=1" >> ~/.bash_profile
fi

if [ "${{ runner.os }}" == "Linux" ]; then
make test-postgres
elif [ "${{ runner.os }}" == "Windows" ]; then
# Create a temp dir on the R: ramdisk drive for Windows. The default
# C: drive is extremely slow: https://github.com/actions/runner-images/issues/8755
mkdir -p "R:/temp/embedded-pg"
go run scripts/embedded-pg/main.go -path "R:/temp/embedded-pg"
DB=ci gotestsum --format standard-quiet -- -v -short -count=1 ./...
else
go run scripts/embedded-pg/main.go
DB=ci gotestsum --format standard-quiet -- -v -short -count=1 ./...
fi

- name: Upload test stats to Datadog
timeout-minutes: 1
Expand Down Expand Up @@ -494,6 +533,47 @@ jobs:
with:
api-key: ${{ secrets.DATADOG_API_KEY }}

test-go-race-pg:
runs-on: ${{ github.repository_owner == 'coder' && 'depot-ubuntu-22.04-16' || 'ubuntu-latest' }}
needs: changes
if: needs.changes.outputs.go == 'true' || needs.changes.outputs.ci == 'true' || github.ref == 'refs/heads/main'
timeout-minutes: 25
steps:
- name: Harden Runner
uses: step-security/harden-runner@91182cccc01eb5e619899d80e4e971d6181294a7 # v2.10.1
with:
egress-policy: audit

- name: Checkout
uses: actions/checkout@eef61447b9ff4aafe5dcd4e0bbf5d482be7e7871 # v4.2.1
with:
fetch-depth: 1

- name: Setup Go
uses: ./.github/actions/setup-go

- name: Setup Terraform
uses: ./.github/actions/setup-tf

# We run race tests with reduced parallelism because they use more CPU and we were finding
# instances where tests appear to hang for multiple seconds, resulting in flaky tests when
# short timeouts are used.
# c.f. discussion on https://github.com/coder/coder/pull/15106
- name: Run Tests
env:
POSTGRES_VERSION: "16"
run: |
make test-postgres-docker
DB=ci gotestsum --junitfile="gotests.xml" -- -race -parallel 4 -p 4 ./...

- name: Upload test stats to Datadog
timeout-minutes: 1
continue-on-error: true
uses: ./.github/actions/upload-datadog
if: always()
with:
api-key: ${{ secrets.DATADOG_API_KEY }}

# Tailnet integration tests only run when the `tailnet` directory or `go.sum`
# and `go.mod` are changed. These tests are to ensure we don't add regressions
# to tailnet, either due to our code or due to updating dependencies.
Expand Down Expand Up @@ -771,6 +851,7 @@ jobs:
- test-go
- test-go-pg
- test-go-race
- test-go-race-pg
- test-js
- test-e2e
- offlinedocs
Expand All @@ -793,6 +874,7 @@ jobs:
echo "- test-go: ${{ needs.test-go.result }}"
echo "- test-go-pg: ${{ needs.test-go-pg.result }}"
echo "- test-go-race: ${{ needs.test-go-race.result }}"
echo "- test-go-race-pg: ${{ needs.test-go-race-pg.result }}"
echo "- test-js: ${{ needs.test-js.result }}"
echo "- test-e2e: ${{ needs.test-e2e.result }}"
echo "- offlinedocs: ${{ needs.offlinedocs.result }}"
Expand Down
76 changes: 76 additions & 0 deletions scripts/embedded-pg/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
// Start an embedded postgres database on port 5432. Used in CI on macOS and Windows.
package main

import (
"database/sql"
"flag"
"os"
"path/filepath"

embeddedpostgres "github.com/fergusstrange/embedded-postgres"
)

func main() {
var customPath string
flag.StringVar(&customPath, "path", "", "Optional custom path for postgres data directory")
flag.Parse()

postgresPath := filepath.Join(os.TempDir(), "coder-test-postgres")
if customPath != "" {
postgresPath = customPath
}

ep := embeddedpostgres.NewDatabase(
embeddedpostgres.DefaultConfig().
Version(embeddedpostgres.V16).
BinariesPath(filepath.Join(postgresPath, "bin")).
DataPath(filepath.Join(postgresPath, "data")).
RuntimePath(filepath.Join(postgresPath, "runtime")).
CachePath(filepath.Join(postgresPath, "cache")).
Username("postgres").
Password("postgres").
Database("postgres").
Encoding("UTF8").
Port(uint32(5432)).
Logger(os.Stdout),
)
err := ep.Start()
if err != nil {
panic(err)
}
// We execute these queries instead of using the embeddedpostgres
// StartParams because it doesn't work on Windows. The library
// seems to have a bug where it sends malformed parameters to
// pg_ctl. It encloses each parameter in single quotes, which
// Windows can't handle.
Comment on lines +41 to +45
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we open a PR upstream to fix this bug?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. I opened fergusstrange/embedded-postgres#145 and I could submit a PR later.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add the issue tracking link to this comment?

// Related issue:
// https://github.com/fergusstrange/embedded-postgres/issues/145
paramQueries := []string{
`ALTER SYSTEM SET effective_cache_size = '1GB';`,
`ALTER SYSTEM SET fsync = 'off';`,
`ALTER SYSTEM SET full_page_writes = 'off';`,
`ALTER SYSTEM SET max_connections = '1000';`,
`ALTER SYSTEM SET shared_buffers = '1GB';`,
`ALTER SYSTEM SET synchronous_commit = 'off';`,
`ALTER SYSTEM SET client_encoding = 'UTF8';`,
}
db, err := sql.Open("postgres", "postgres://postgres:postgres@127.0.0.1:5432/postgres?sslmode=disable")
if err != nil {
panic(err)
}
for _, query := range paramQueries {
if _, err := db.Exec(query); err != nil {
panic(err)
}
}
if err := db.Close(); err != nil {
panic(err)
}
// We restart the database to apply all the parameters.
if err := ep.Stop(); err != nil {
panic(err)
}
if err := ep.Start(); err != nil {
panic(err)
}
}
Loading