test: run database tests in CI (#174)

* use transactions for db tests

* add mssql actions

* convert offer-adapter tests to use parameter db

* dotenv with path in config

* set env ci db pwd

* remove redundant jest config config

* use db transaction context for delete-listing and get-listings-with-applicants

* convert all tests to use transaction context

* allow read commited snapshot to allow parallelised jest tests that uses transactions

* use transaction context

* run without parallelisation for now

* move test config stuff out of application code and remove unused exports

* clean up

.env.ci

@@ -0,0 +1,5 @@
+LEASING_DATABASE__PASSWORD=Passw0rd!
+LEASING_DATABASE__HOST=localhost
+LEASING_DATABASE__USER=sa
+LEASING_DATABASE__PORT=1433
+LEASING_DATABASE__DATABASE=tenants-leases-test

.github/workflows/lint-and-test.yml

@@ -7,6 +7,14 @@ on:
   workflow_call:
 jobs:
   test:
+    services:
+      sql:
+        image: mcr.microsoft.com/mssql/server:2019-latest
+        env:
+          SA_PASSWORD: Passw0rd!
+          ACCEPT_EULA: Y
+        ports:
+          - "1433:1433"
     runs-on: ubuntu-latest
     steps:
       - name: Checkout repository
@@ -15,6 +23,9 @@ jobs:
         uses: actions/setup-node@v4
         with:
           node-version: '20'
+      - name: Create db
+        run: |
+          /opt/mssql-tools/bin/sqlcmd -S localhost -U sa -P Passw0rd! -Q "CREATE DATABASE [tenants-leases-test];"
       - name: Install dependencies
         run: npm ci
       - name: Run tests

.gitignore

@@ -10,4 +10,4 @@ config.json
 # Jetbrains IDE
 .idea/
 
-.vscode/
+.vscode/

.jest/migrate.ts

@@ -0,0 +1,27 @@
+import path from 'path'
+import knex from 'knex'
+
+import Config from '../src/common/config'
+
+export default async function migrate() {
+  const db = knex({
+    client: 'mssql',
+    connection: Config.leasingDatabase,
+    useNullAsDefault: true,
+    migrations: {
+      tableName: 'migrations',
+      directory: path.join(__dirname, '../migrations'),
+    },
+  })
+
+  await db.migrate
+    .latest()
+    .then(() => {
+      console.log('Migrations applied')
+    })
+    .catch((error) => {
+      console.error('Error applying migrations', error)
+    })
+
+  await db.destroy()
+}

.jest/teardown.ts

@@ -0,0 +1,26 @@
+import path from 'path'
+import knex from 'knex'
+
+import Config from '../src/common/config'
+
+export default async function teardown() {
+  const db = knex({
+    client: 'mssql',
+    connection: Config.leasingDatabase,
+    useNullAsDefault: true,
+    migrations: {
+      tableName: 'migrations',
+      directory: path.join(__dirname, '../migrations'),
+    },
+  })
+
+  try {
+    await db.migrate.rollback().then(() => {
+      console.log('Migrations rolled back')
+    })
+  } catch (error) {
+    console.error('Error rolling back migrations:', error)
+  }
+
+  await db.destroy()
+}

jest.config.js

@@ -2,24 +2,11 @@
 module.exports = {
   preset: 'ts-jest',
   testEnvironment: 'node',
-  modulePathIgnorePatterns: [
-    '<rootDir>/build/',
-    //todo: excludes the db tests so that GH-action does not remove data by accident
-    //todo: remove below line when test db connection exists for GH-action
-  ].concat(
-    process.env.NODE_ENV === 'test-ci'
-      ? [
-          '<rootDir>/src/services/lease-service/tests/adapters/',
-          '<rootDir>/src/services/lease-service/tests/sync-internal-parking-space-listings-from-xpand.test.ts',
-          '<rootDir>/src/services/lease-service/tests/offer-service.test.ts',
-          '<rootDir>/src/services/lease-service/tests/update-or-create-application-profile.test.ts',
-        ]
-      : []
-  ),
-  //todo: maxWorkers: 1 runs all tests in sequence so that we don't get deadlocks for db tests
-  //todo: implement a more elegant solution (run db tests in sequence, all other tests in parallel)
-  maxWorkers: 1,
+  modulePathIgnorePatterns: ['<rootDir>/build/'],
   transformIgnorePatterns: ['node_modules/(?!(onecore-types)/)'],
   extensionsToTreatAsEsm: ['.d.ts, .ts'],
   setupFiles: ['<rootDir>/.jest/common.ts'],
+  maxWorkers: 1,
+  globalSetup: '<rootDir>/.jest/migrate.ts',
+  globalTeardown: '<rootDir>/.jest/teardown.ts',
 }

knexfile.js

@@ -1,4 +1,5 @@
 // Update with your config settings.
+
 require('dotenv').config()
 
 /**

package.json

@@ -7,19 +7,19 @@
     "node": "20.*"
   },
   "scripts": {
-    "test": "jest",
-    "test:watch": "jest --watch",
+    "test": "DOTENV_CONFIG_PATH=.env.test node -r dotenv/config node_modules/jest/bin/jest --config jest.config.js",
+    "test:watch": "DOTENV_CONFIG_PATH=.env.test jest --watch",
     "build": "tsc",
-    "start": "npm run migrate:up && node index",
-    "dev": "npm run migrate:up && nodemon -e ts,js --exec ts-node src/index ",
+    "start": "npm run migrate:up && node -r dotenv/config index",
+    "dev": "DOTENV_CONFIG_PATH=.env npm run migrate:up && nodemon -e ts,js --exec ts-node -r dotenv/config src/index ",
     "lint": "eslint src/",
     "migrate:make": "knex migrate:make",
     "migrate:up": "knex migrate:latest --env dev",
     "migrate:down": "knex migrate:down --env dev",
     "seed": "knex seed:run --env dev",
     "script:expire-listings": "node scripts/expire-listings.js",
     "ts:watch": "tsc --watch --noEmit",
-    "test:ci": "NODE_ENV=test-ci jest --silent",
+    "test:ci": "DOTENV_CONFIG_PATH=.env.ci node -r dotenv/config node_modules/jest/bin/jest --config jest.config.js",
     "ts:ci": "tsc --noEmit"
   },
   "author": "",

src/common/config.ts

@@ -1,12 +1,5 @@
 import configPackage from '@iteam/config'
-import dotenv from 'dotenv'
-import path from 'path'
-
-if (process.env.NODE_ENV == 'test') {
-  dotenv.config({ path: path.join(__dirname, '../../.env.test') })
-} else {
-  dotenv.config()
-}
+import 'dotenv/config'
 
 export interface Config {
   port: number

src/services/lease-service/adapters/db.ts

@@ -1,55 +1,11 @@
 import knex from 'knex'
-import Config from '../../../common/config'
-import * as path from 'path'
 
-const getStandardConfig = () => ({
-  client: 'mssql',
-  connection: Config.leasingDatabase,
-})
+import Config from '../../../common/config'
 
-const getTestConfig = () => {
-  return {
+export const createDbClient = () =>
+  knex({
     client: 'mssql',
     connection: Config.leasingDatabase,
-    useNullAsDefault: true,
-    migrations: {
-      tableName: 'migrations',
-      directory: path.join(__dirname, '../../../../migrations'),
-    },
-  }
-}
-
-const getConfigBasedOnEnvironment = () => {
-  const environment = process.env.NODE_ENV || 'dev'
-  return environment === 'test' ? getTestConfig() : getStandardConfig()
-}
-
-export const db = knex(getConfigBasedOnEnvironment())
-
-const migrate = async () => {
-  await db.migrate
-    .latest()
-    .then(() => {
-      console.log('Migrations applied')
-    })
-    .catch((error) => {
-      console.error('Error applying migrations', error)
-    })
-}
-
-const teardown = async () => {
-  console.log('Rolling back migrations')
-  try {
-    await db.migrate.rollback().then(() => {
-      console.log('Migrations rolled back')
-    })
-  } catch (error) {
-    console.error('Error rolling back migrations:', error)
-  }
-
-  await db.destroy().then(() => {
-    console.log('Database destroyed')
   })
-}
 
-export { migrate, teardown }
+export const db = createDbClient()

src/services/lease-service/adapters/listing-adapter.ts

@@ -51,10 +51,11 @@ function transformDbApplicant(row: DbApplicant): Applicant {
 }
 
 const createListing = async (
-  listingData: Omit<Listing, 'id'>
+  listingData: Omit<Listing, 'id'>,
+  dbConnection = db
 ): Promise<AdapterResult<Listing, 'conflict-active-listing' | 'unknown'>> => {
   try {
-    const insertedRow = await db<DbListing>('Listing')
+    const insertedRow = await dbConnection<DbListing>('Listing')
       .insert({
         RentalObjectCode: listingData.rentalObjectCode,
         Address: listingData.address,
@@ -109,9 +110,10 @@ const createListing = async (
  * @returns {Promise<Listing>} - Promise that resolves to the existing listing if it exists.
  */
 const getActiveListingByRentalObjectCode = async (
-  rentalObjectCode: string
+  rentalObjectCode: string,
+  dbConnection = db
 ): Promise<Listing | undefined> => {
-  const listing = await db<DbListing>('Listing')
+  const listing = await dbConnection<DbListing>('Listing')
     .where({
       RentalObjectCode: rentalObjectCode,
       Status: ListingStatus.Active,
@@ -125,10 +127,11 @@ const getActiveListingByRentalObjectCode = async (
 }
 
 const getListingById = async (
-  listingId: number
+  listingId: number,
+  dbConnection = db
 ): Promise<Listing | undefined> => {
   logger.info({ listingId }, `Getting listing ${listingId} from leasing DB`)
-  const result = await db
+  const result = await dbConnection
     .from('listing AS l')
     .select<DbListing & { applicants: string | null }>(
       'l.*',
@@ -184,10 +187,11 @@ const getListingById = async (
  * @returns {Promise<Applicant | undefined>} - Returns the applicant.
  */
 const getApplicantById = async (
-  applicantId: number
+  applicantId: number,
+  dbConnection = db
 ): Promise<Applicant | undefined> => {
   logger.info({ applicantId }, 'Getting applicant from leasing DB')
-  const applicant = await db<DbApplicant>('Applicant')
+  const applicant = await dbConnection<DbApplicant>('Applicant')
     .where({
       Id: applicantId,
     })
@@ -206,13 +210,16 @@ const getApplicantById = async (
   return transformDbApplicant(applicant)
 }
 
-const createApplication = async (applicationData: Omit<Applicant, 'id'>) => {
+const createApplication = async (
+  applicationData: Omit<Applicant, 'id'>,
+  dbConnection = db
+) => {
   logger.info(
     { contactCode: applicationData.contactCode },
     'Creating application in listing DB'
   )
 
-  const insertedRow = await db('applicant')
+  const insertedRow = await dbConnection('applicant')
     .insert({
       Name: applicationData.name,
       NationalRegistrationNumber: applicationData.nationalRegistrationNumber,
@@ -253,6 +260,7 @@ const updateApplicantStatus = async (
 }
 
 const getListingsWithApplicants = async (
+  db: Knex,
   opts?: GetListingsWithApplicantsFilterParams
 ): Promise<AdapterResult<Array<Listing>, 'unknown'>> => {
   try {
@@ -274,7 +282,7 @@ const getListingsWithApplicants = async (
       )
       .with({ type: 'ready-for-offer' }, () =>
         db.raw(
-          `WHERE l.Status = ? 
+          `WHERE l.Status = ?
           AND EXISTS (
             SELECT 1
             FROM applicant a
@@ -291,7 +299,7 @@ const getListingsWithApplicants = async (
       )
       .with({ type: 'offered' }, () =>
         db.raw(
-          `WHERE l.Status = ? 
+          `WHERE l.Status = ?
           AND EXISTS (
             SELECT 1
             FROM offer o
@@ -362,8 +370,11 @@ const getListingsWithApplicants = async (
  * @returns {Promise<Applicant | undefined>} - Returns the applicants.
  */
 
-const getApplicantsByContactCode = async (contactCode: string) => {
-  const result = await db('Applicant')
+const getApplicantsByContactCode = async (
+  contactCode: string,
+  dbConnection = db
+) => {
+  const result = await dbConnection('Applicant')
     .where({ ContactCode: contactCode })
     .select<Array<DbApplicant>>('*')
 
@@ -379,9 +390,10 @@ const getApplicantsByContactCode = async (contactCode: string) => {
  */
 const getApplicantByContactCodeAndListingId = async (
   contactCode: string,
-  listingId: number
+  listingId: number,
+  dbConnection = db
 ) => {
-  const result = await db<DbApplicant>('Applicant')
+  const result = await dbConnection<DbApplicant>('Applicant')
     .where({
       ContactCode: contactCode,
       ListingId: listingId,
@@ -400,8 +412,12 @@ const getApplicantByContactCodeAndListingId = async (
  * @param {number} listingId - The ID of the listing the applicant belongs to.
  * @returns {Promise<boolean>} - Returns true if applicant belongs to listing, false if not.
  */
-const applicationExists = async (contactCode: string, listingId: number) => {
-  const result = await db<DbApplicant>('applicant')
+const applicationExists = async (
+  contactCode: string,
+  listingId: number,
+  dbConnection = db
+) => {
+  const result = await dbConnection<DbApplicant>('applicant')
     .where({
       ContactCode: contactCode,
       ListingId: listingId,
@@ -415,9 +431,9 @@ const applicationExists = async (contactCode: string, listingId: number) => {
   return true
 }
 
-const getExpiredListings = async () => {
+const getExpiredListings = async (dbConnection = db) => {
   const currentDate = new Date()
-  const listings = await db('listing')
+  const listings = await dbConnection('listing')
     .where('PublishedTo', '<', currentDate)
     .andWhere('Status', '=', ListingStatus.Active)
   return listings
@@ -464,10 +480,11 @@ const updateListingStatuses = async (
 }
 
 const deleteListing = async (
-  listingId: number
+  listingId: number,
+  dbConnection = db
 ): Promise<AdapterResult<null, 'unknown' | 'conflict'>> => {
   try {
-    await db('listing').delete().where('Id', listingId)
+    await dbConnection('listing').delete().where('Id', listingId)
     return { ok: true, data: null }
   } catch (err) {
     logger.error(err, 'listingAdapter.deleteListing')