Skip to content

Commit

Permalink
ci: Run tests using PostgreSQL database and mock (#49)
Browse files Browse the repository at this point in the history
* ci: Run tests using PostgreSQL database and mock

This allows us to use the mock database for quick iterative testing,
and have confidence from CI using a real PostgreSQL database.

PostgreSQL tests are only ran on Linux. They are *really* slow on MacOS
and Windows runners, and don't provide much additional confidence.

* Only run PostgreSQL tests once for speed

* Fix race condition of log after close

Not all resources were cleaned up immediately after a peer connection was
closed. DataChannels could have a goroutine exit after Close() prior to this.

* Fix comment
  • Loading branch information
kylecarbs committed Jan 22, 2022
1 parent dfddaf1 commit 50d8151
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 8 deletions.
10 changes: 9 additions & 1 deletion .github/workflows/coder.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -149,11 +149,19 @@ jobs:
- run: go install gotest.tools/gotestsum@latest

- run:
- name: Test with Mock Database
run:
gotestsum --jsonfile="gotests.json" --packages="./..." --
-covermode=atomic -coverprofile="gotests.coverage" -timeout=3m
-count=3 -race -parallel=2

- name: Test with PostgreSQL Database
if: runner.os == 'Linux'
run:
DB=true gotestsum --jsonfile="gotests.json" --packages="./..." --
-covermode=atomic -coverprofile="gotests.coverage" -timeout=3m
-count=1 -race -parallel=2

- uses: codecov/codecov-action@v2
with:
token: ${{ secrets.CODECOV_TOKEN }}
Expand Down
18 changes: 18 additions & 0 deletions coderd/coderdtest/coderdtest.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,20 @@ package coderdtest

import (
"context"
"database/sql"
"net/http/httptest"
"net/url"
"os"
"testing"

"github.com/stretchr/testify/require"

"cdr.dev/slog/sloggers/slogtest"
"github.com/coder/coder/coderd"
"github.com/coder/coder/codersdk"
"github.com/coder/coder/database"
"github.com/coder/coder/database/databasefake"
"github.com/coder/coder/database/postgres"
)

// Server represents a test instance of coderd.
Expand All @@ -27,6 +31,20 @@ type Server struct {
func New(t *testing.T) Server {
// This can be hotswapped for a live database instance.
db := databasefake.New()
if os.Getenv("DB") != "" {
connectionURL, close, err := postgres.Open()
require.NoError(t, err)
t.Cleanup(close)
sqlDB, err := sql.Open("postgres", connectionURL)
require.NoError(t, err)
t.Cleanup(func() {
_ = sqlDB.Close()
})
err = database.Migrate(sqlDB)
require.NoError(t, err)
db = database.New(sqlDB)
}

handler := coderd.New(&coderd.Options{
Logger: slogtest.Make(t, nil),
Database: db,
Expand Down
3 changes: 2 additions & 1 deletion database/query.sql
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,14 @@ INSERT INTO
email,
name,
login_type,
revoked,
hashed_password,
created_at,
updated_at,
username
)
VALUES
($1, $2, $3, $4, $5, $6, $7, $8) RETURNING *;
($1, $2, $3, $4, false, $5, $6, $7, $8) RETURNING *;

-- name: UpdateAPIKeyByID :exec
UPDATE
Expand Down
3 changes: 2 additions & 1 deletion database/query.sql.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions peer/channel.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ func (c *Channel) init() {

c.conn.dcDisconnectListeners.Add(1)
c.conn.dcFailedListeners.Add(1)
c.conn.dcClosedWaitGroup.Add(1)
go func() {
var err error
// A DataChannel can disconnect multiple times, so this needs to loop.
Expand Down Expand Up @@ -274,6 +275,7 @@ func (c *Channel) closeWithError(err error) error {
close(c.sendMore)
c.conn.dcDisconnectListeners.Sub(1)
c.conn.dcFailedListeners.Sub(1)
c.conn.dcClosedWaitGroup.Done()

if c.rwc != nil {
_ = c.rwc.Close()
Expand Down
15 changes: 10 additions & 5 deletions peer/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ type Conn struct {
dcDisconnectListeners atomic.Uint32
dcFailedChannel chan struct{}
dcFailedListeners atomic.Uint32
dcClosedWaitGroup sync.WaitGroup

localCandidateChannel chan webrtc.ICECandidateInit
localSessionDescriptionChannel chan webrtc.SessionDescription
Expand All @@ -125,11 +126,10 @@ type Conn struct {
pingEchoChan *Channel
pingEchoOnce sync.Once
pingEchoError error

pingMutex sync.Mutex
pingOnce sync.Once
pingChan *Channel
pingError error
pingMutex sync.Mutex
pingOnce sync.Once
pingChan *Channel
pingError error
}

func (c *Conn) init() error {
Expand Down Expand Up @@ -502,5 +502,10 @@ func (c *Conn) CloseWithError(err error) error {
// this call will return an error that isn't typed. We don't check the error because
// closing an already closed connection isn't an issue for us.
_ = c.rtc.Close()

// Waits for all DataChannels to exit before officially labeling as closed.
// All logging, goroutines, and async functionality is cleaned up after this.
c.dcClosedWaitGroup.Wait()

return err
}

0 comments on commit 50d8151

Please sign in to comment.