From d50fb7c92205cfc18f3dd912e9c47db9be31d0e2 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Wed, 16 Mar 2022 13:18:59 +0000 Subject: [PATCH] Added global setup to reset DB before test run - We've been seeing weird errors with tables not existing when running tests locally - This appears to happen after an error causes the tests to abort - This change includes two fixes: 1. we triggers a full DB reset just before the entire test suite runs - this is done by wrapping the override file for tests that use a db, and supplying a mochaGlobalSetup hook 2. catch errors when attempting to do a fast truncation-based reset & init, and do a full reset & init instead - These two changes should ensure the DB is always in the state we expect when running a new test suite --- package.json | 10 +++++----- test/utils/db-overrides.js | 17 +++++++++++++++++ test/utils/db-utils.js | 24 ++++++++++++++++++++---- test/utils/overrides.js | 7 ++++++- 4 files changed, 48 insertions(+), 10 deletions(-) create mode 100644 test/utils/db-overrides.js diff --git a/package.json b/package.json index c9af536e6a..1c50eb1aad 100644 --- a/package.json +++ b/package.json @@ -26,20 +26,20 @@ "setup": "yarn install && knex-migrator init && grunt symlink && grunt init || (exit 0)", "main": "grunt shell:main && grunt subgrunt:init", "build": "grunt build", - "test": "mocha --require=./test/utils/overrides.js --exit --trace-warnings --recursive --extension=test.js --timeout=60000", + "test": "mocha --require=./test/utils/db-overrides.js --exit --trace-warnings --recursive --extension=test.js --timeout=60000", "test:all": "yarn test:unit && yarn test:integration && yarn test:e2e && yarn lint", "test:debug": "DEBUG=ghost:test* yarn test", "test:unit": "c8 yarn test:unit:base", "test:unit:base": "mocha --require=./test/utils/overrides.js --exit --trace-warnings --recursive --extension=test.js './test/unit' --timeout=2000", - "test:integration": "mocha --require=./test/utils/overrides.js --exit --trace-warnings --recursive --extension=test.js './test/integration' --timeout=5000", - "test:e2e": "mocha --require=./test/utils/overrides.js --exit --trace-warnings --recursive --extension=test.js './test/e2e-api' './test/e2e-frontend' './test/e2e-server' --timeout=10000", - "test:regression": "mocha --require=./test/utils/overrides.js --exit --trace-warnings --recursive --extension=test.js './test/regression' --timeout=60000", + "test:integration": "mocha --require=./test/utils/db-overrides.js --exit --trace-warnings --recursive --extension=test.js './test/integration' --timeout=5000", + "test:e2e": "mocha --require=./test/utils/db-overrides.js --exit --trace-warnings --recursive --extension=test.js './test/e2e-api' './test/e2e-frontend' './test/e2e-server' --timeout=10000", + "test:regression": "mocha --require=./test/utils/db-overrides.js --exit --trace-warnings --recursive --extension=test.js './test/regression' --timeout=60000", "test:browser": "playwright test --browser=all test/e2e-browser", "test:ci": "yarn test:e2e -b && yarn test:integration -b && yarn test:regression -b", "test:unit:slow": "yarn test:unit --reporter=mocha-slow-test-reporter", "test:int:slow": "yarn test:integration --reporter=mocha-slow-test-reporter", "test:e2e:slow": "yarn test:e2e --reporter=mocha-slow-test-reporter", - "test:reg:slow": "mocha --require=./test/utils/overrides.js --exit --trace-warnings --recursive --extension=test.js './test/regression' --timeout=60000 --reporter=mocha-slow-test-reporter", + "test:reg:slow": "yarn test:regression --reporter=mocha-slow-test-reporter", "lint:server": "eslint --ignore-path .eslintignore 'core/server/**/*.js' 'core/*.js' '*.js'", "lint:shared": "eslint --ignore-path .eslintignore 'core/shared/**/*.js'", "lint:frontend": "eslint --ignore-path .eslintignore 'core/frontend/**/*.js'", diff --git a/test/utils/db-overrides.js b/test/utils/db-overrides.js new file mode 100644 index 0000000000..aa7bd8eb13 --- /dev/null +++ b/test/utils/db-overrides.js @@ -0,0 +1,17 @@ +/** + * This overrides file should be used for all tests that use the DB + */ + +module.exports = require('./overrides'); + +/** + * Global Setup + * Guaranteed to only ever run once regardless of whether + * mocha is run in serial or parallel + */ +module.exports.mochaGlobalSetup = async function () { + // Actually delete the database so we start afresh + // This protects against DB errors after an aborted test run + // This technically only needs to happen before DB-related tests + await require('./db-utils').destroy(); +}; diff --git a/test/utils/db-utils.js b/test/utils/db-utils.js index 1c5cee7934..70c4880f04 100644 --- a/test/utils/db-utils.js +++ b/test/utils/db-utils.js @@ -17,6 +17,10 @@ const urlServiceUtils = require('./url-service-utils'); const dbHash = Date.now(); +module.exports.destroy = async () => { + await knexMigrator.reset({force: true}); +}; + module.exports.reset = async ({truncate} = {truncate: false}) => { // Only run this copy in CI until it gets fleshed out if (process.env.CI && config.get('database:client') === 'sqlite3') { @@ -37,12 +41,21 @@ module.exports.reset = async ({truncate} = {truncate: false}) => { await fs.copyFile(filename, filenameOrig); } } else { + debug('not CI'); if (truncate) { - // Perform a fast reset by tearing down all the tables and - // inserting the fixtures - await module.exports.teardown(); - await knexMigrator.init({only: 2}); + debug('truncate mode'); + // Perform a fast reset by tearing down all the tables and inserting the fixtures + try { + await module.exports.teardown(); + await knexMigrator.init({only: 2}); + } catch (err) { + // If it fails, try a normal reset and init + debug('teardown failed, reset and init'); + await knexMigrator.reset({force: true}); + await knexMigrator.init(); + } } else { + debug('reset and init'); // Do a full database reset + initialisation await knexMigrator.reset({force: true}); await knexMigrator.init(); @@ -137,6 +150,9 @@ module.exports.teardown = () => { } throw err; + }) + .finally(() => { + debug('Database teardown end'); }); }); }; diff --git a/test/utils/overrides.js b/test/utils/overrides.js index 3a1cd66880..6bf7add595 100644 --- a/test/utils/overrides.js +++ b/test/utils/overrides.js @@ -1,7 +1,12 @@ process.env.NODE_ENV = process.env.NODE_ENV || 'testing'; + +/** + * This overrides file should be used for all tests that DO NOT use the DB - e.g. unit tests + */ + process.env.WEBHOOK_SECRET = process.env.WEBHOOK_SECRET || 'TEST_STRIPE_WEBHOOK_SECRET'; require('../../core/server/overrides'); const {mochaHooks} = require('@tryghost/express-test').snapshot; -exports.mochaHooks = mochaHooks; +module.exports.mochaHooks = mochaHooks;