fergusstrange / embedded-postgres

Run a real Postgres database locally on Linux, OSX or Windows as part of another Go application or test
MIT License
876 stars 89 forks source link

Process already running in port when test panics using embedded postgres #115

Closed manuelarte closed 1 year ago

manuelarte commented 1 year ago

We are using embedded postgres for testing, and I created some signals to stop embedded postgres process when either the test suite finishes or any of the following signals happens: os.Interrupt, syscall.SIGINT, syscall.SIGTERM.

But, if a panic occurrs, then we don't stop the embedded postgres, and if we try to run the test again we get the following:

process already listening on port 4876

So we need to kill it manually to test again.

I think of several solutions to this:

Right now I am using the 3rd approach (recover from panic), but I am wondering if this is a known issue or if people came up with better alternatives.

Regards.

peterldowns commented 1 year ago

This is something I ran into, too, while trying embedded-postgres out for testing purposes. Here's the main_test.go I was iterating on. It will start the postgres server if it isn't already running, but it will not stop the server at the end of the tests (impossible to guarantee that this happens reliably.)

package main_test

import (
    "database/sql"
    "fmt"
    "strings"
    "sync"
    "testing"

    embeddedpostgres "github.com/fergusstrange/embedded-postgres"
    _ "github.com/lib/pq"
    "github.com/peterldowns/pgtestdb"
    "github.com/peterldowns/pgtestdb/migrators/goosemigrator"
    "github.com/peterldowns/testy/assert"
    "github.com/peterldowns/testy/check"
)

// serverOnce will make this example succeed, but really the synchronization
// should happen in the APIs that embeddedpostgres provides for
// starting/stopping the database.
var serverOnce = &sync.Once{}

// ensureServer is a test helper that will start the embeddedPostgres server if
// it isn't running. It does nothing if the server is already running. If it
// fails to start the server for any other reason, it fails the test
// immediately.
func ensureServer(t *testing.T) {
    t.Helper()
    serverOnce.Do(func() {
        postgres := embeddedpostgres.NewDatabase()
        err := postgres.Start()
        if err != nil {
            if strings.Contains(err.Error(), "process already listening on port 5432") {
                t.Log("embedded-postgres already running")
            } else {
                t.Fatalf("failed to start postgres: %s", err)
            }
        }
    })
}

// newDB connects to the embedded postgres server and creates a new database
// based on the current migrations. If there is any problem creating that
// database, it fails the test immediately.
func newDB(t *testing.T) *sql.DB {
    t.Helper()
    ensureServer(t)
    config := pgtestdb.Config{
        DriverName: "postgres",
        User:       "postgres",
        Password:   "postgres",
        Host:       "localhost",
        Port:       "5432",
        Database:   "postgres",
        Options:    "sslmode=disable",
    }
    migrator := goosemigrator.New("migrations")
    return pgtestdb.New(t, config, migrator)
}

func TestManyParallelTestsAllUsingTheDatabase(t *testing.T) {
    t.Parallel()
    // Creates 10 subtests, each running in parallel, each of which runs a
    // datbase query.
    for i := 0; i < 10; i++ {
        t.Run(fmt.Sprintf("subtest_%d", i), func(t *testing.T) {
            t.Parallel()

            // - starts the embedded-postgres server if it isn't running
            // - ensures a template database based on the current migrations
            // - creates a new database from that template
            db := newDB(t)

            // silly example, just showing how the database is usable.
            var result string
            err := db.QueryRow("SELECT 'hello world'").Scan(&result)
            assert.Nil(t, err)
            check.Equal(t, "hello world", result)
        })
    }

}

Without the mutex, if I make sure that there is no ~/.embedded-postgres-go directory and I killall postgres to make sure that I'm starting from a clean state, I'll see initdb failures like this as multiple calls to postgres.Start() collide.

    --- FAIL: TestManyParallelTestsAllUsingTheDatabase/subtest_6 (6.27s)
        main_test.go:71: failed to start postgres: unable to init database using '/Users/pd/.embedded-postgres-go/extracted/bin/initdb -A password -U postgres -D /Users/pd/.embedded-postgres-go/extracted/data --pwfile=/Users/pd/.embedded-postgres-go/extracted/pwfile': exit status 1
            The files belonging to this database system will be owned by user "pd".
            This user must also own the server process.

            The database cluster will be initialized with locale "en_US.UTF-8".
            The default database encoding has accordingly been set to "UTF8".
            The default text search configuration will be set to "english".

            Data page checksums are disabled.

            initdb: error: directory "/Users/pd/.embedded-postgres-go/extracted/data" exists but is not empty
            initdb: hint: If you want to create a new database system, either remove or empty the directory "/Users/pd/.embedded-postgres-go/extracted/data" or run initdb with an argument other than "/Users/pd/.embedded-postgres-go/extracted/data".
    --- FAIL: TestManyParallelTestsAllUsingTheDatabase/subtest_5 (7.09s)
        main_test.go:71: could not create pgtestdb user: sessionlock(testdb-user) failed to open conn: dial tcp [::1]:5432: connect: connection refused
FAIL
FAIL    github.com/peterldowns/example-embedded-postgres-testdb 7.223s
FAIL

I recommend looking into an API similar to what ory/dockertest provides (i think), where you can ask that a container be running,a nd it will make sure that is the case regardless of whether or not it was previously started.

@manuelarte if you're looking for a way to quickly and easily use unique postgres databases for each of your tests, check out my project https://github.com/peterldowns/pgtestdb. I currently recommend starting the postgres server yourself using docker-compose or something similar, but I'd love to be able to endorse embedded-postgres in the future (or provide some kind of easy integration!)

peterldowns commented 1 year ago

to run my example you'll need to put this in a directory that contains a migrations directory (it can be empty).

❯ tree
.
├── go.mod
├── go.sum
├── main_test.go
└── migrations

1 directory, 3 files
fergusstrange commented 1 year ago

@manuelarte is spot on with the assessment.

If we wanted to control how people run their tests we'd need to provide some sort of test wrapper that caught panics within these tests and then did some controlled shutdown behaviour. Whilst it's been considered it feels just outside of the scope of this project which offers as close to vanilla Postgres as possible, leaving users to control how they run tests. Testify has some pretty advanced features for allowing predefined test set ups with some error handling that I'd recommend using.

@peterldowns your issue seems slightly different in that you will always have port clash in the example - I think this was your intention? We intentionally want to throw away any instances that have been started up previously as this is intended as a testing tool, designed to allow us to have consistent and reproducible tests that run independently of each other.

Cheers all, let me know how you get on.

byrnedo commented 1 year ago

@peterldowns The issue is since embedded postgres uses pg_ctrl to launch postgres thus giving up ownership of the process. I made an mr here so that on Linux, once the tests exit, the process would be killed. Mr Strange did not approve 😢

fergusstrange commented 1 year ago

I'm not sure it passed all the tests @byrnedo?

byrnedo commented 1 year ago

This one hopefully passes @fergusstrange: https://github.com/fergusstrange/embedded-postgres/pull/127