add higher-level db migration tests

.github/workflows/db-migrations.yml

@@ -0,0 +1,51 @@
+name: Database Migrations
+
+on:
+  pull_request:
+    paths:
+    - .github/workflows/db-migrations.yml
+    - lib/bin/create-docker-databases.js
+    - lib/model/migrations/**
+    - test/db-migrations/**
+    - package.json
+    - package-lock.json
+    - Makefile
+  push:
+    paths:
+    - .github/workflows/db-migrations.yml
+    - lib/bin/create-docker-databases.js
+    - lib/model/migrations/**
+    - test/db-migrations/**
+    - package.json
+    - package-lock.json
+    - Makefile
+
+jobs:
+  db-migration-tests:
+    timeout-minutes: 2
+    # TODO should we use the same container as circle & central?
+    runs-on: ubuntu-latest
+    services:
+      # see: https://docs.github.com/en/[email protected]/actions/using-containerized-services/creating-postgresql-service-containers
+      postgres:
+        image: postgres:14.10
+        env:
+          POSTGRES_PASSWORD: odktest
+        ports:
+          - 5432:5432
+        # Set health checks to wait until postgres has started
+        options: >-
+          --health-cmd pg_isready
+          --health-interval 10s
+          --health-timeout 5s
+          --health-retries 5
+    steps:
+    - uses: actions/checkout@v4
+    - name: Set node version
+      uses: actions/setup-node@v4
+      with:
+        node-version: 22.12.0
+        cache: 'npm'
+    - run: npm ci
+    - run: node lib/bin/create-docker-databases.js
+    - run: make test-db-migrations

.mocharc.js

@@ -9,6 +9,7 @@
 
 module.exports = {
   ignore: [
+    'test/db-migrations/**',
     'test/e2e/**',
   ],
 };

Makefile

@@ -107,6 +107,12 @@ test: lint
 test-ci: lint
 	BCRYPT=insecure npx mocha --recursive --reporter test/ci-mocha-reporter.js
 
+.PHONY: test-db-migrations
+test-db-migrations:
+	NODE_CONFIG_ENV=db-migration-test npx mocha --bail --sort --timeout=20000 \
+	    --require test/db-migrations/mocha-setup.js \
+	    ./test/db-migrations/**/*.spec.js
+
 .PHONY: test-fast
 test-fast: node_version
 	BCRYPT=insecure npx mocha --recursive --fgrep @slow --invert

config/db-migration-test.json

@@ -0,0 +1,10 @@
+{
+  "default": {
+    "database": {
+      "host": "localhost",
+      "user": "jubilant",
+      "password": "jubilant",
+      "database": "jubilant_test"
+    }
+  }
+}

test/db-migrations/.eslintrc.js

@@ -0,0 +1,8 @@
+module.exports = {
+  extends: '../.eslintrc.js',
+  globals: {
+    db: false,
+    log: false,
+    sql: false,
+  },
+};

test/db-migrations/.gitignore

@@ -0,0 +1 @@
+/.holding-pen/

test/db-migrations/20241008-01-add-user_preferences.spec.js

@@ -0,0 +1,46 @@
+const { // eslint-disable-line object-curly-newline
+  assertIndexExists,
+  assertTableDoesNotExist,
+  assertTableSchema,
+  describeMigration,
+} = require('./utils'); // eslint-disable-line object-curly-newline
+
+describeMigration('20241008-01-add-user_preferences', ({ runMigrationBeingTested }) => {
+  before(async () => {
+    await assertTableDoesNotExist('user_site_preferences');
+    await assertTableDoesNotExist('user_project_preferences');
+
+    await runMigrationBeingTested();
+  });
+
+  it('should create user_site_preferences table', async () => {
+    await assertTableSchema('user_site_preferences',
+      { column_name: 'userId',        is_nullable: 'NO', data_type: 'integer' }, // eslint-disable-line no-multi-spaces
+      { column_name: 'propertyName',  is_nullable: 'NO', data_type: 'text' },    // eslint-disable-line no-multi-spaces
+      { column_name: 'propertyValue', is_nullable: 'NO', data_type: 'jsonb' },
+    );
+  });
+
+  it('should create user_site_preferences userId index', async () => {
+    await assertIndexExists(
+      'user_site_preferences',
+      'CREATE INDEX "user_site_preferences_userId_idx" ON public.user_site_preferences USING btree ("userId")',
+    );
+  });
+
+  it('should create user_project_preferences table', async () => {
+    await assertTableSchema('user_project_preferences',
+      { column_name: 'userId',        is_nullable: 'NO', data_type: 'integer' }, // eslint-disable-line no-multi-spaces
+      { column_name: 'projectId',     is_nullable: 'NO', data_type: 'integer' }, // eslint-disable-line no-multi-spaces
+      { column_name: 'propertyName',  is_nullable: 'NO', data_type: 'text' },    // eslint-disable-line no-multi-spaces
+      { column_name: 'propertyValue', is_nullable: 'NO', data_type: 'jsonb' },
+    );
+  });
+
+  it('should create user_project_preferences userId index', async () => {
+    await assertIndexExists(
+      'user_project_preferences',
+      'CREATE INDEX "user_project_preferences_userId_idx" ON public.user_project_preferences USING btree ("userId")',
+    );
+  });
+});

test/db-migrations/migrator.js

@@ -0,0 +1,108 @@
+// This functions by moving migration files in and out of the migrations target
+// directory.
+//
+// Aims:
+//
+// * ensure that the real production migration command is run in tests
+// * avoid isolation differences between running the tests & migrations in the
+//   same node instance.  These differences might include shared globals,
+//   database connection pool state, database transaction states etc.
+// * ensure that these tests do not depend directly on knex.  Removing knex
+//   dependency is a long-term project goal.
+// * allow replacement of the production migration implementation without
+//   changing tests
+
+const fs = require('node:fs');
+const { execSync } = require('node:child_process');
+
+const migrationsDir = './lib/model/migrations';
+const holdingPen = './test/db-migrations/.holding-pen';
+
+fs.mkdirSync(holdingPen, { recursive: true });
+
+restoreMigrations(); // eslint-disable-line no-use-before-define
+const allMigrations = loadMigrationsList(); // eslint-disable-line no-use-before-define
+moveMigrationsToHoldingPen(); // eslint-disable-line no-use-before-define
+
+let lastRunIdx = -1;
+
+function runBefore(migrationName) {
+  const idx = getIndex(migrationName); // eslint-disable-line no-use-before-define
+  if (idx === 0) return;
+
+  const previousMigration = allMigrations[idx - 1];
+
+  return runIncluding(previousMigration); // eslint-disable-line no-use-before-define
+}
+
+function runIncluding(lastMigrationToRun) {
+  const finalIdx = getIndex(lastMigrationToRun); // eslint-disable-line no-use-before-define
+
+  for (let restoreIdx=lastRunIdx+1; restoreIdx<=finalIdx; ++restoreIdx) { // eslint-disable-line no-plusplus
+    const f = allMigrations[restoreIdx] + '.js';
+    fs.renameSync(`${holdingPen}/${f}`, `${migrationsDir}/${f}`);
+  }
+
+  log('Running migrations until:', lastMigrationToRun, '...');
+  const res = execSync(`node ./lib/bin/run-migrations.js`, { encoding: 'utf8' });
+
+  lastRunIdx = finalIdx;
+
+  log(`Ran migrations up-to-and-including ${lastMigrationToRun}:\n`, res);
+}
+
+function getIndex(migrationName) {
+  const idx = allMigrations.indexOf(migrationName);
+  log('getIndex()', migrationName, 'found at', idx);
+  if (idx === -1) throw new Error(`Unknown migration: ${migrationName}`);
+  return idx;
+}
+
+function restoreMigrations() {
+  moveAll(holdingPen, migrationsDir); // eslint-disable-line no-use-before-define
+}
+
+function moveMigrationsToHoldingPen() {
+  moveAll(migrationsDir, holdingPen); // eslint-disable-line no-use-before-define
+}
+
+function moveAll(src, tgt) {
+  fs.readdirSync(src)
+    .forEach(f => fs.renameSync(`${src}/${f}`, `${tgt}/${f}`));
+}
+
+function loadMigrationsList() {
+  const migrations = fs.readdirSync(migrationsDir)
+    .filter(f => f.endsWith('.js'))
+    .map(f => f.replace(/\.js$/, ''))
+    .sort(); // TODO check that this is how knex sorts migration files
+  log();
+  log('All migrations:');
+  log();
+  migrations.forEach(m => log('*', m));
+  log();
+  log('Total:', migrations.length);
+  log();
+  return migrations;
+}
+
+function exists(migrationName) {
+  try {
+    getIndex(migrationName);
+    return true;
+  } catch (err) {
+    return false;
+  }
+}
+
+function hasRun(migrationName) {
+  return lastRunIdx >= getIndex(migrationName);
+}
+
+module.exports = {
+  exists,
+  hasRun,
+  runBefore,
+  runIncluding,
+  restoreMigrations,
+};

test/db-migrations/mocha-setup.js

@@ -0,0 +1,36 @@
+const _log = level => (...args) => console.log(level, ...args); // eslint-disable-line no-console
+global.log = _log('[INFO]');
+
+const fs = require('node:fs');
+const slonik = require('slonik');
+const migrator = require('./migrator');
+
+async function mochaGlobalSetup() {
+  log('mochaGlobalSetup() :: ENTRY');
+
+  global.sql = slonik.sql;
+
+  const { user, password, host, database } = jsonFile('./config/db-migration-test.json').default.database; // eslint-disable-line no-use-before-define
+  const dbUrl = `postgres://${user}:${password}@${host}/${database}`;
+  log('dbUrl:', dbUrl);
+  global.db = slonik.createPool(dbUrl);
+
+  // Try to clean up the test database.  This should work unless you've used
+  // different users to create/configure the DB.
+  await db.query(sql`DROP OWNED BY CURRENT_USER`);
+
+  log('mochaGlobalSetup() :: EXIT');
+}
+
+function mochaGlobalTeardown() {
+  log('mochaGlobalTeardown() :: ENTRY');
+  db?.end();
+  migrator.restoreMigrations();
+  log('mochaGlobalTeardown() :: EXIT');
+}
+
+module.exports = { mochaGlobalSetup, mochaGlobalTeardown };
+
+function jsonFile(path) {
+  return JSON.parse(fs.readFileSync(path, { encoding: 'utf8' }));
+}