test: improve test coverage

Signed-off-by: Lenin Mehedy <[email protected]>

src/commands/base.mjs

@@ -18,20 +18,7 @@
 import { FullstackTestingError, MissingArgumentError } from '../core/errors.mjs'
 import { ConfigManager } from '../core/index.mjs'
 import { ShellRunner } from '../core/shell_runner.mjs'
-
-/**
- * This is a default command error handler used by handleCommand
- * @param err error
- * @param logger logger
- */
-const defaultErrorHandler = (err, logger) => {
-  logger.showUserError(err)
-
-  // do not exit immediately so that logger can flush properly
-  setTimeout(() => {
-    process.exit(1)
-  }, 1)
-}
+import * as helpers from '../core/helpers.mjs'
 
 export class BaseCommand extends ShellRunner {
   async prepareChartPath (chartDir, chartRepo, chartName) {
@@ -74,7 +61,7 @@ export class BaseCommand extends ShellRunner {
    * @param errHandler error handler
    * @return {Promise<void>}
    */
-  async handleCommand (argv, handleFunc, errHandler = defaultErrorHandler) {
+  async handleCommand (argv, handleFunc, errHandler = helpers.defaultErrorHandler) {
     if (!argv) throw new MissingArgumentError('argv is required')
     if (!handleFunc) throw new MissingArgumentError('handleFunc is required')
 

src/commands/init.mjs

@@ -165,8 +165,7 @@ export class InitCommand extends BaseCommand {
       },
       handler: argv => initCmd.handleCommand(
         argv,
-        async (args) => await initCmd.init(args),
-        initCmd.logger
+        async (args) => await initCmd.init(args)
       )
     }
   }

src/core/config_manager.mjs

@@ -237,10 +237,9 @@ export class ConfigManager {
    *
    * If no lock file exists, it will create a lock file and write the pid in the file
    * @param logger
-   * @param lockExistHandler error handler when lock exists
-   * @return {void}
+   * @return {Promise<boolean>}
    */
-  static acquireProcessLock (logger, lockExistHandler = (existingPid) => { process.exit(1) }) {
+  static async acquireProcessLock (logger) {
     const pid = process.pid.toString()
     const pidFile = path.normalize(constants.SOLO_PID_FILE)
     if (!fs.existsSync(pidFile)) {
@@ -249,15 +248,13 @@ export class ConfigManager {
         pidFile,
         pid
       })
-      return
+      return true
     }
 
     // pid lock exists
     const existingPid = fs.readFileSync(pidFile).toString()
-    logger.showUserError(new FullstackTestingError(`Process lock exists: ${constants.SOLO_PID_FILE}` +
-      `\nEnsure process '${existingPid}' is not running [ ps -p ${existingPid} ]`))
-
-    return lockExistHandler(existingPid)
+    throw new FullstackTestingError(`Process lock exists: ${constants.SOLO_PID_FILE}` +
+      `\nEnsure process '${existingPid}' is not running [ ps -p ${existingPid} ]`)
   }
 
   /**
@@ -269,7 +266,7 @@ export class ConfigManager {
    * @param logger
    * @return {Promise<void>}
    */
-  static releaseProcessLock (logger) {
+  static async releaseProcessLock (logger) {
     const pidFile = path.normalize(constants.SOLO_PID_FILE)
     if (fs.existsSync(pidFile)) {
       const existingPid = fs.readFileSync(pidFile).toString()

src/core/helpers.mjs

@@ -138,3 +138,13 @@ export function getRootImageRepository (releaseTag) {
 export function getTmpDir () {
   return fs.mkdtempSync(path.join(constants.SOLO_TMP_DIR, 'solo-'))
 }
+
+/**
+ * This is a default command error handler used by handleCommand
+ * @param err error
+ * @param logger logger
+ */
+export function defaultErrorHandler (err, logger) {
+  // TODO add user friendly message for the error
+  logger.showUserError(err)
+}

src/core/logging.mjs

@@ -125,6 +125,8 @@ export const Logger = class {
     }
 
     console.log(chalk.red('*********************************** ERROR ****************************************'))
+    console.log(chalk.yellow(`Error Reference: ${this.traceId}`))
+    // console.log(chalk.red('----------------------------------------------------------------------------------'))
     if (this.devMode) {
       let prefix = ''
       let indent = ''

test/unit/commands/base.test.mjs

@@ -14,8 +14,12 @@
  * limitations under the License.
  *
  */
-import { expect, it, describe } from '@jest/globals'
+import { expect, it, describe, jest } from '@jest/globals'
+import fs from 'fs'
+import { FullstackTestingError } from '../../../src/core/errors.mjs'
+import * as helpers from '../../../src/core/helpers.mjs'
 import {
+  constants,
   DependencyManager,
   ChartManager,
   ConfigManager,
@@ -24,6 +28,7 @@ import {
 } from '../../../src/core/index.mjs'
 import { BaseCommand } from '../../../src/commands/base.mjs'
 import { K8 } from '../../../src/core/k8.mjs'
+import { Logger } from '../../../src/core/logging.mjs'
 
 const testLogger = logging.NewLogger('debug')
 
@@ -51,4 +56,38 @@ describe('BaseCommand', () => {
       await expect(baseCmd.run('date')).resolves.not.toBeNull()
     })
   })
+
+  describe('handle command', () => {
+    it('should succeed in running a valid command handler', async () => {
+      expect.assertions(2)
+      expect(fs.existsSync(constants.SOLO_PID_FILE)).toBeFalsy()
+
+      const argv = {}
+      argv._ = ['test']
+
+      let error = null
+      await baseCmd.handleCommand(argv,
+        async () => {
+        },
+        (err, logger) => {
+          error = err
+        }
+      )
+      expect(error).toBeNull()
+    })
+
+    it('should throw error if it fails to do process lock', async () => {
+      expect.assertions(2)
+      expect(fs.existsSync(constants.SOLO_PID_FILE)).toBeFalsy()
+      await ConfigManager.acquireProcessLock(testLogger)
+
+      const argv = {}
+      argv._ = ['test']
+      await baseCmd.handleCommand(argv, () => {}, (err, logger) => {
+        expect(err.message.includes('Process lock exists')).toBeTruthy()
+      })
+
+      await ConfigManager.releaseProcessLock(testLogger)
+    })
+  })
 })

test/unit/core/config_manager.test.mjs

@@ -313,15 +313,20 @@ describe('ConfigManager', () => {
   })
 
   describe('process lock', () => {
-    it('should be able to acquire process lock', () => {
+    it('should be able to acquire process lock', async () => {
+      expect.assertions(6)
       expect(fs.existsSync(constants.SOLO_PID_FILE)).toBeFalsy()
-      ConfigManager.acquireProcessLock(testLogger)
+      expect(await ConfigManager.acquireProcessLock(testLogger)).toBeTruthy()
       expect(fs.existsSync(constants.SOLO_PID_FILE)).toBeTruthy()
       expect(fs.readFileSync(constants.SOLO_PID_FILE).toString()).toStrictEqual(process.pid.toString())
-      ConfigManager.acquireProcessLock(testLogger, (existingPid) => {
-        expect(existingPid).toStrictEqual(process.pid.toString())
-      })
-      ConfigManager.releaseProcessLock(testLogger)
+
+      // re-attempt should fail
+      try {
+        await ConfigManager.acquireProcessLock(testLogger)
+      } catch (e) {
+        expect(e.message.startsWith('Process lock exists')).toBeTruthy()
+      }
+      await ConfigManager.releaseProcessLock(testLogger)
       expect(fs.existsSync(constants.SOLO_PID_FILE)).toBeFalsy()
     })
   })

test/unit/core/helpers.test.mjs

@@ -14,9 +14,11 @@
  * limitations under the License.
  *
  */
-import { describe, expect, it } from '@jest/globals'
+import { describe, expect, it, jest } from '@jest/globals'
 import { FullstackTestingError } from '../../../src/core/errors.mjs'
+import { Logger } from '../../../src/core/logging.mjs'
 import * as helpers from '../../../src/core/helpers.mjs'
+import { testLogger } from '../../test_util.js'
 
 describe('Helpers', () => {
   it.each([
@@ -70,4 +72,14 @@ describe('Helpers', () => {
       expect(helpers.compareVersion('v3.12.3', 'v3.14.0')).toBe(1)
     })
   })
+
+  describe('default error handler', () => {
+    it('should exit with error code on error', () => {
+      expect.assertions(1)
+      const spy = jest.spyOn(Logger.prototype, 'showUserError').mockImplementation()
+      const err = new FullstackTestingError('test')
+      helpers.defaultErrorHandler(err, testLogger)
+      expect(spy).toHaveBeenCalledWith(err)
+    })
+  })
 })