From 933a0b5e2b15bbc00c57ab7cdc32561dae1e2a82 Mon Sep 17 00:00:00 2001 From: Stuart Adair <43574728+StuAA78@users.noreply.github.com> Date: Wed, 30 Nov 2022 10:36:39 +0000 Subject: [PATCH] Move test database URL to environment variable (#493) https://github.com/DEFRA/water-abstraction-team/issues/64 We currently hardcode the url of the test database in our `package.json` which isn't good practice, and also causes us problems when running within a Docker environment (as per the linked issue). We want to use `DATABASE_URL` instead as `db-migrate` will use this url if it exists. Unfortunately this causes an issue when we want to run migrations against different dbs in the same environment -- eg. normally we want migrations to run against our regular db but when running unit tests, we want migrations to run against the test db. `db-migrate` does allow for setting different environments in `database.json` (as in test, prod etc.) but this then moves configuration away from our `.env` file -- not what we want when our environments are more like "local", "local test", "CI test", etc. We therefore create a script `scripts/migrate.js`, which checks `NODE_ENV` to see if the environment is `test` or not and sets `DATABASE_URL` accordingly before running `db-migrate up`: - If `NODE_ENV` is `test` then it sets `DATABASE_URL` to the value of `TEST_DATABASE_URL` in the `.env` file; - Otherwise, it simply uses `DATABASE_URL` as-is. We update our `package.json` so that this is run whenever we would normally run `db-migrate up`. Since `lab` sets `NODE_ENV` to `test`, we don't need to make any changes to our `test` script. We also update `config.js` to also check `NODE_ENV` when setting the db connection string -- again, if `NODE_ENV` is `test` then we use `TEST_DATABASE_URL`, otherwise we use `DATABASE_URL`. This therefore means that things like the "create schema" script don't need to be changed as these use the db connection string in `config.js`. --- .env.example | 3 +++ .github/workflows/ci.yml | 4 ++-- config.js | 2 +- database.json | 11 ----------- package.json | 6 +++--- scripts/migrate.js | 26 ++++++++++++++++++++++++++ 6 files changed, 35 insertions(+), 17 deletions(-) delete mode 100644 database.json create mode 100644 scripts/migrate.js diff --git a/.env.example b/.env.example index 7007f6d6..051f9f46 100644 --- a/.env.example +++ b/.env.example @@ -7,6 +7,9 @@ RETURNS_URI= WATER_URI= CRM_URI= +DATABASE_URL= +TEST_DATABASE_URL= + S3_KEY= S3_SECRET= S3_BUCKET= diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3283aa4f..b78e43dc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,7 +6,7 @@ jobs: build: runs-on: ubuntu-latest env: - DATABASE_URL: postgres://water_user:password@localhost:5432/wabs_test + TEST_DATABASE_URL: postgres://water_user:password@localhost:5432/wabs_test JWT_SECRET: ItsNoSecretBecauseYouToldEverybody JWT_TOKEN: eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6MSwibmFtZSI6InRlc3QiLCJpYXQiOjE1MDMzMTg0NDV9.eWghqjYlPrb8ZjWacYzTCTh1PBtr2BeSv-_ZIwrtmwE # These need to be duplicated in services section for postgres. Unfortunately, there is not a way to reuse them @@ -86,7 +86,7 @@ jobs: - name: Database migrations run: | - npm run migrate + npm run migrate:test # This includes an extra run step. The sonarcloud analysis will be run in a docker container with the current # folder mounted as `/github/workspace`. The problem is when the lcov.info file is generated it will reference the diff --git a/config.js b/config.js index 974638ae..714691e8 100644 --- a/config.js +++ b/config.js @@ -26,7 +26,7 @@ module.exports = { }, pg: { - connectionString: process.env.DATABASE_URL, + connectionString: process.env.NODE_ENV !== 'test' ? process.env.DATABASE_URL : process.env.TEST_DATABASE_URL, max: 20 }, diff --git a/database.json b/database.json deleted file mode 100644 index a2eb3c31..00000000 --- a/database.json +++ /dev/null @@ -1,11 +0,0 @@ -{ - "dev": { - "driver": "pg", - "user": "postgres", - "password": "postgres", - "host": "127.0.0.1", - "port": "5432", - "database": "permits", - "schema": "water_import" - } -} diff --git a/package.json b/package.json index 1a807b7d..e32a6757 100644 --- a/package.json +++ b/package.json @@ -11,12 +11,12 @@ "author": "WRLS service team", "license": "OGL-UK-3.0", "scripts": { - "test": "DATABASE_URL=postgres://water_user:password@localhost:5432/wabs_test lab", + "test": "lab", "lint": "standard", - "migrate": "node scripts/create-schema && db-migrate up --verbose", + "migrate": "node scripts/create-schema && node scripts/migrate", "migrate:down": "db-migrate down --verbose", "migrate:create": "db-migrate create --sql-file --", - "migrate:test": "DATABASE_URL=postgres://water_user:password@localhost:5432/wabs_test node scripts/create-schema && DATABASE_URL=postgres://water_user:password@localhost:5432/wabs_test db-migrate up --verbose", + "migrate:test": "export NODE_ENV=test && node scripts/create-schema && node scripts/migrate", "version": "npx --yes auto-changelog -p --commit-limit false && git add CHANGELOG.md" }, "dependencies": { diff --git a/scripts/migrate.js b/scripts/migrate.js new file mode 100644 index 00000000..f1c88f81 --- /dev/null +++ b/scripts/migrate.js @@ -0,0 +1,26 @@ +'use strict' + +require('dotenv').config() + +// We use promisify to wrap exec in a promise. This allows us to await it without resorting to using callbacks. +const util = require('util') +const exec = util.promisify(require('child_process').exec) + +async function run () { + const databaseUrl = process.env.NODE_ENV !== 'test' ? process.env.DATABASE_URL : process.env.TEST_DATABASE_URL + + try { + const { stdout, stderr } = await exec(`export DATABASE_URL=${databaseUrl} && db-migrate up --verbose`) + + const output = stderr ? `ERROR: ${stderr}` : stdout.replace('\n', '') + console.log(output) + + process.exit(stderr ? 1 : 0) + } catch (error) { + console.log(`ERROR: ${error.message}`) + + process.exit(1) + } +} + +run()