Refactoring test code to use same function to create connections

gionatamettifogo · gionatamettifogo · commit 05fb42b693df · 2023-11-17T18:38:43.000+01:00
diff --git a/jest.config.js b/jest.config.js
@@ -2,6 +2,7 @@ module.exports = {
   preset: 'ts-jest',
   testEnvironment: 'node',
   testMatch: ['**/test/**/*.test.ts'],
+  // longer timeout when using a debugger:
   testTimeout: 30 * 1000,
   maxWorkers: 8,
   collectCoverageFrom: ['<rootDir>/src/**/*.ts', '!<rootDir>/src/index.ts', '!<rootDir>/src/types/**/*.ts'],
diff --git a/src/connection.ts b/src/connection.ts
@@ -162,37 +162,45 @@ export class SQLiteCloudConnection {
     }
 
     this.operations.enqueue(done => {
+      // clear all listeners and call done in the operations queue
+      const finish: ResultsCallback = (error, result) => {
+        if (this.socket) {
+          this.socket.removeAllListeners('data')
+          this.socket.removeAllListeners('error')
+          this.socket.removeAllListeners('close')
+        }
+        if (callback) {
+          callback?.call(this, error)
+          callback = undefined
+        }
+        if (error) {
+          this.close()
+        }
+        // process next operation in the queue
+        done?.call(this, error ? error : undefined)
+      }
+
       // connect to tls socket, initialize connection, setup event handlers
       const client: tls.TLSSocket = tls.connect(this.config.port as number, this.config.host, this.config.tlsOptions, () => {
         if (!client.authorized) {
           const anonimizedError = anonimizeError(client.authorizationError)
           this.log('Connection was not authorized', anonimizedError)
           this.close()
-          const error = new SQLiteCloudError('Connection was not authorized', { cause: anonimizedError })
-          callback?.call(this, error)
-          done?.call(this, error)
+          finish(new SQLiteCloudError('Connection was not authorized', { cause: anonimizedError }))
         } else {
           this.socket = client
-          callback?.call(this, null)
-          done?.call(this)
+          finish(null)
         }
       })
 
       client.on('close', () => {
-        if (this.socket) {
-          // no loggin if already disposed
-          this.log('Connection closed')
-          this.socket.destroy()
-          this.socket = undefined
-        }
+        this.log('Connection closed')
+        finish(new SQLiteCloudError('Connection was closed'))
       })
 
       client.once('error', (error: any) => {
-        error = new SQLiteCloudError('Connection error', { cause: error })
         this.log('Connection error', error)
-        this.close()
-        callback?.call(this, error)
-        done?.call(this, error)
+        finish(new SQLiteCloudError('Connection error', { cause: error }))
       })
     })
 
@@ -225,13 +233,19 @@ export class SQLiteCloudConnection {
       let socketTimeout: number
 
       // clear all listeners and call done in the operations queue
-      const finish = (error?: Error) => {
+      const finish: ResultsCallback = (error, result) => {
         clearTimeout(socketTimeout)
         if (this.socket) {
           this.socket.removeAllListeners('data')
           this.socket.removeAllListeners('error')
+          this.socket.removeAllListeners('close')
         }
-        done?.call(this, error)
+        if (callback) {
+          callback?.call(this, error, result)
+          callback = undefined
+        }
+        // process next operation in the queue
+        done?.call(this, error ? error : undefined)
       }
 
       // define the Promise that waits for the server response
@@ -263,15 +277,13 @@ export class SQLiteCloudConnection {
               if (dataType !== CMD_ROWSET_CHUNK && compressedDataType !== CMD_ROWSET_CHUNK) {
                 this.socket?.off('data', readData)
                 const { data } = popData(buffer)
-                callback?.call(this, null, data)
-                finish?.call(this)
+                finish(null, data)
               } else {
                 // @ts-expect-error
                 // check if rowset received the ending chunk
                 if (data.subarray(data.indexOf(' ') + 1, data.length).toString() === '0 0 0 ') {
                   const parsedData = parseRowsetChunks(rowsetChunks)
-                  callback?.call(this, null, parsedData)
-                  finish?.call(this)
+                  finish?.call(this, null, parsedData)
                 } else {
                   // no ending string? ask server for another chunk
                   rowsetChunks.push(buffer)
@@ -285,28 +297,32 @@ export class SQLiteCloudConnection {
             const lastChar = buffer.subarray(buffer.length - 1, buffer.length).toString('utf8')
             if (lastChar == ' ') {
               const { data } = popData(buffer)
-              callback?.call(this, null, data)
-              finish?.call(this)
+              finish(null, data)
             }
           }
         } catch (error) {
-          this.close()
-          callback?.call(this, error as Error)
-          finish?.call(this, error as Error)
+          console.assert(error instanceof Error)
+          if (error instanceof Error) {
+            finish(error)
+          }
         }
       }
 
+      this.socket?.once('close', () => {
+        finish(new SQLiteCloudError('Connection was closed', { cause: anonimizeCommand(commands) }))
+      })
+
       this.socket?.write(commands, 'utf8', () => {
         socketTimeout = setTimeout(() => {
-          finish?.call(this, new SQLiteCloudError('Request timed out', { cause: anonimizeCommand(commands) }))
+          finish(new SQLiteCloudError('Request timed out', { cause: anonimizeCommand(commands) }))
         }, this.config.timeout)
         this.socket?.on('data', readData)
       })
 
       this.socket?.once('error', (error: any) => {
         this.log('Socket error', error)
         this.close()
-        finish?.call(this, new SQLiteCloudError('Socket error', { cause: anonimizeError(error) }))
+        finish(new SQLiteCloudError('Socket error', { cause: anonimizeError(error) }))
       })
     })
 
diff --git a/src/database.ts b/src/database.ts
@@ -18,6 +18,9 @@ import { Statement } from './statement'
 import { ErrorCallback, ResultsCallback, RowCallback, RowsCallback } from './types'
 import EventEmitter from 'eventemitter3'
 
+// Uses eventemitter3 instead of node events for browser compatibility
+// https://github.com/primus/eventemitter3
+
 /**
  * Creating a Database object automatically opens a connection to the SQLite database.
  * When the connection is established the Database object emits an open event and calls
@@ -110,9 +113,6 @@ export class Database extends EventEmitter {
   /** Emits given event with optional arguments on the next tick so callbacks can complete first */
   private emitEvent(event: string, ...args: any[]): void {
     process.nextTick(() => {
-      if (this.config.verbose) {
-        console.log(`Database.emitEvent - emitted '${event}'`, ...args)
-      }
       this.emit(event, ...args)
     })
   }
diff --git a/test/compare.test.ts b/test/compare.test.ts
@@ -2,7 +2,6 @@
  * compare.test.ts - test driver api against sqlite3 equivalents
  */
 
-import { Database } from '../src/index'
 import { CHINOOK_DATABASE_FILE, CHINOOK_DATABASE_URL, CHINOOK_FIRST_TRACK, LONG_TIMEOUT, TESTING_SQL } from './shared'
 import { getChinookDatabase, getTestingDatabase } from './shared'
 
@@ -44,7 +43,7 @@ describe('Database.on', () => {
   })
 
   it('sqlitecloud: should emit open event', done => {
-    const chinook = new Database(CHINOOK_DATABASE_URL, error => {
+    const chinook = getChinookDatabase(error => {
       expect(chinook).toBeDefined()
       expect(error).toBeNull()
       chinook.once('open', () => {
diff --git a/test/connection.test.ts b/test/connection.test.ts
@@ -5,28 +5,17 @@
 import { SQLiteCloudError } from '../src/index'
 import { SQLiteCloudConnection, anonimizeCommand } from '../src/connection'
 import { parseConnectionString } from '../src/utilities'
-import { CHINOOK_DATABASE_URL, TESTING_DATABASE_URL, LONG_TIMEOUT, getTestingConfig, getChinoookConfig } from './shared'
+import { CHINOOK_DATABASE_URL, TESTING_DATABASE_URL, LONG_TIMEOUT, getTestingConfig, getChinookConfig, getChinookConnection } from './shared'
 
 describe('connection', () => {
   let chinook: SQLiteCloudConnection
 
-  beforeAll(() => {
-    expect(CHINOOK_DATABASE_URL).toBeDefined()
-    expect(TESTING_DATABASE_URL).toBeDefined()
-  })
-
-  beforeEach(done => {
-    chinook = new SQLiteCloudConnection(CHINOOK_DATABASE_URL + '?nonlinearizable=1', error => {
-      expect(chinook).toBeDefined()
-      done()
-    })
-    // connection.verbose()
+  beforeEach(() => {
+    chinook = getChinookConnection()
   })
 
   afterEach(() => {
-    if (chinook) {
-      chinook.close()
-    }
+    chinook?.close()
     // @ts-ignore
     chinook = undefined
   })
@@ -37,11 +26,11 @@ describe('connection', () => {
     })
 
     it('should add self signed certificate for localhost connections', () => {
-      const localConfig = getChinoookConfig('sqlitecloud://admin:xxx@localhost:8850/chinook.db')
+      const localConfig = getChinookConfig('sqlitecloud://admin:xxx@localhost:8850/chinook.db')
       expect(localConfig.host).toBe('localhost')
       expect(localConfig.tlsOptions?.ca).toBeTruthy()
 
-      const remoteConfig = getChinoookConfig('sqlitecloud://admin:xxx@sqlitecloud.io:8850/chinook.db')
+      const remoteConfig = getChinookConfig('sqlitecloud://admin:xxx@sqlitecloud.io:8850/chinook.db')
       expect(remoteConfig.host).toBe('sqlitecloud.io')
       expect(remoteConfig.tlsOptions).toBeFalsy()
     })
diff --git a/test/database.test.ts b/test/database.test.ts
@@ -3,41 +3,43 @@
  */
 
 import { SQLiteCloudRowset, SQLiteCloudRow, SQLiteCloudError } from '../src/index'
-import { Database } from '../src/database'
-import { getTestingDatabase, getTestingDatabaseAsync, getChinookDatabase, removeDatabase, removeDatabaseAsync } from './shared'
-import { CHINOOK_DATABASE_URL, LONG_TIMEOUT } from './shared'
+import { getTestingDatabase, getTestingDatabaseAsync, getChinookDatabase, removeDatabase, removeDatabaseAsync, LONG_TIMEOUT } from './shared'
 import { RowCountCallback } from '../src/types'
 
 //
 // utility methods to setup and destroy temporary test databases
 //
 
 describe('Database.run', () => {
-  it('simple update', done => {
-    const updateSql = "UPDATE people SET name = 'Charlie Brown' WHERE id = 3; UPDATE people SET name = 'David Bowie' WHERE id = 4; "
+  it(
+    'simple update',
+    done => {
+      const updateSql = "UPDATE people SET name = 'Charlie Brown' WHERE id = 3; UPDATE people SET name = 'David Bowie' WHERE id = 4; "
 
-    // lambda callback would "hide" this
-    function plainCallbackNotALambda(err: Error, results: any) {
-      expect(err).toBeNull()
-      expect(results).toBeUndefined()
+      // lambda callback would "hide" this
+      function plainCallbackNotALambda(err: Error, results: any) {
+        expect(err).toBeNull()
+        expect(results).toBeUndefined()
 
-      // Database.run should return number of rows modified and lastID
-      // @ts-expect-error
-      const context = this as any
-      expect(context.lastID).toBe(20)
-      expect(context.changes).toBe(1) // should this be 2?
-      expect(context.totalChanges).toBe(22)
-      expect(context.finalized).toBe(1)
+        // Database.run should return number of rows modified and lastID
+        // @ts-expect-error
+        const context = this as any
+        expect(context.lastID).toBe(20)
+        expect(context.changes).toBe(1) // should this be 2?
+        expect(context.totalChanges).toBe(22)
+        expect(context.finalized).toBe(1)
 
-      removeDatabase(database, error => {
-        expect(error).toBeNull()
-        done()
-      })
-    }
+        removeDatabase(database, error => {
+          expect(error).toBeNull()
+          done()
+        })
+      }
 
-    const database = getTestingDatabase()
-    database.run(updateSql, plainCallbackNotALambda)
-  })
+      const database = getTestingDatabase()
+      database.run(updateSql, plainCallbackNotALambda)
+    },
+    LONG_TIMEOUT
+  )
 
   it(
     'insert with parameter value',
@@ -90,8 +92,8 @@ describe('Database.all', () => {
   it(
     'simple select',
     done => {
-      const db = new Database(CHINOOK_DATABASE_URL)
-      db.all('SELECT * FROM tracks', (err: Error, rows: SQLiteCloudRowset) => {
+      const chinook = getChinookDatabase()
+      chinook.all('SELECT * FROM tracks', (err: Error, rows: SQLiteCloudRowset) => {
         expect(err).toBeNull()
         expect(rows).toBeDefined()
         expect(rows).toHaveLength(3503)
@@ -107,7 +109,7 @@ describe('Database.all', () => {
           UnitPrice: 0.99
         })
 
-        db.close(error => {
+        chinook.close(error => {
           expect(error).toBeNull()
           done()
         })
@@ -205,27 +207,32 @@ describe('Database.exec', () => {
     })
   })
 
-  it('execute statement with errors', done => {
-    // sqlitecloud-js / fix problem with jest tests of sendCommands error conditions #24
-    const chinook = getChinookDatabase()
-    try {
-      chinook.exec('SET BOGUS STATEMENT TO 1;', error => {
-        expect(error).toBeInstanceOf(SQLiteCloudError)
-        expect(error).toMatchObject({
-          errorCode: '10002',
-          externalErrorCode: '0',
-          name: 'SQLiteCloudError',
-          offsetCode: -1,
-          message: 'Unable to find command SET BOGUS STATEMENT TO 1;.'
+  it(
+    'execute statement with errors',
+    done => {
+      // sqlitecloud-js / fix problem with jest tests of sendCommands error conditions #24
+      const chinook = getChinookDatabase()
+      try {
+        chinook.exec('SET BOGUS STATEMENT TO 1;', error => {
+          expect(error).toBeDefined()
+          expect(error).toBeInstanceOf(SQLiteCloudError)
+
+          const sqliteCloudError = error as SQLiteCloudError
+          expect(sqliteCloudError.errorCode).toBe('10002')
+          expect(sqliteCloudError.externalErrorCode).toBe('0')
+          expect(sqliteCloudError.name).toBe('SQLiteCloudError')
+          expect(sqliteCloudError.offsetCode).toBe(-1)
+          expect(sqliteCloudError.message.includes('Unable to find command SET BOGUS STATEMENT TO 1;')).toBe(true)
+
+          chinook.close()
+          done()
         })
-
-        chinook.close()
-        done()
-      })
-    } catch (error) {
-      done(error)
-    }
-  })
+      } catch (error) {
+        done(error)
+      }
+    },
+    LONG_TIMEOUT
+  )
 })
 
 describe('Database.sql (async)', () => {
diff --git a/test/rowset.test.ts b/test/rowset.test.ts
diff --git a/test/shared.ts b/test/shared.ts
diff --git a/test/statement.test.ts b/test/statement.test.ts
