Move test database URL to environment variable (#493)

DEFRA/water-abstraction-team#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

@@ -7,6 +7,9 @@ RETURNS_URI=
 WATER_URI=
 CRM_URI=
 
+DATABASE_URL=
+TEST_DATABASE_URL=
+
 S3_KEY=
 S3_SECRET=
 S3_BUCKET=

.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

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
   },
 

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": {

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()