Improving tests

Author: imcaizheng

bin/topcoder-cli.js

@@ -187,11 +187,11 @@ program
   .description('Let copilot/managers process private task payments')
   .option('-o --copilot <payment>', 'copilot payment.')
   .option('--dev', 'Points to Topcoder development environment')
-  .action(args => {
+  .action(async args => {
     if (args.dev) {
       process.env.NODE_ENV = 'dev'
     }
-    payHandler.handleCommand(program.args)
+    await payHandler.handleCommand(program.args)
   })
 
 // error on unknown commands

docs/Development.md

@@ -19,14 +19,6 @@ The following parameters can be set in config files or in env variables:
 | TC_CLIENT_V2CONNECTION | CLIENT_V2CONNECTION     | TC-User-Database                           | TC client connection protocol          |
 | AUTH0_AUDIENCE         | AUTH0_AUDIENCE          | https://m2m.topcoder.com/                  | AUTH0 Audience (For M2M)               |
 
-# Configuration for Test
-Configuration for test is at `test/common/testConfig.js`.
-The following parameters can be set in config files or in env variables:
-
-| Property  | Environment varible | Default value | Description                                                                   |
-| ---       | ---                 | ---           | ---                                                                           |
-| WAIT_TIME | WAIT_TIME           | 500           | Waiting time for the CLI tool to process subcommands. Increase the value if needed |
-
 # Publish the package to npm
 - Create a npm account on https://www.npmjs.com/signup if you don't have one.
 - Use the account to sign in via cli: `npm login`

package.json

@@ -43,7 +43,6 @@
   "homepage": "https://topcoder-platform.github.io/topcoder-cli/",
   "devDependencies": {
     "chai": "^4.2.0",
-    "delay": "^4.3.0",
     "mocha": "^6.1.4",
     "mocha-prepare": "^0.1.0",
     "mock-require": "^3.0.3",

src/commands/pay.js

@@ -5,7 +5,7 @@ const logger = require('../common/logger')
  * Handles the "pay" command
  * @param {Array} args Arguments
  */
-function handleCommand (args) {
+async function handleCommand (args) {
   const options = args[0].opts()
   const challengeDetails = [
     {
@@ -45,7 +45,7 @@ function handleCommand (args) {
     logger.info(response)
     // => response => { username, age, about }
   }
-  promptQuestions()
+  await promptQuestions()
 }
 
 module.exports = {

test/cli.test.js

@@ -1,13 +1,11 @@
 /*
  * Test CLI functionalities.
  */
-const delay = require('delay')
 const chai = require('chai')
 const _ = require('lodash')
 
 const { program, docs } = require('../bin/topcoder-cli')
 const testHelper = require('./common/testHelper')
-const testConfig = require('./common/testConfig')
 
 const localTestData = {
   argsWithHelp: testHelper.buildArgs(undefined, { help: true }),
@@ -57,31 +55,26 @@ describe('CLI Test', async function () {
 
   it('It should show general help info', async function () {
     program.parse(localTestData.argsWithHelp)
-    await delay(testConfig.WAIT_TIME)
     chai.expect(_.last(messages)).to.include('Topcoder CLI to interact with Topcoder systems')
   })
 
   it('It should show help documentation for command submit', async function () {
     program.parse(localTestData.argsForCommandSubmit)
-    await delay(testConfig.WAIT_TIME)
     chai.expect(_.last(messages)).to.equal(docs.submit)
   })
 
   it('It should show help documentation for command fetch-submissions', async function () {
     program.parse(localTestData.argsForCommandFetchSubmissions)
-    await delay(testConfig.WAIT_TIME)
     chai.expect(_.last(messages)).to.equal(docs['fetch-submissions'])
   })
 
   it('It should show help documentation for command fetch-artifacts', async function () {
     program.parse(localTestData.argsForCommandFetchArtifacts)
-    await delay(testConfig.WAIT_TIME)
     chai.expect(_.last(messages)).to.equal(docs['fetch-artifacts'])
   })
 
   it('failure - It should handle unknown commands', async function () {
     program.parse(localTestData.argsWithUnknownCommand)
-    await delay(testConfig.WAIT_TIME)
     chai.expect(_.last(errorMessages)).to.include('Invalid command:')
   })
 })

test/common/testConfig.js

@@ -1,16 +1,14 @@
 /*
  * Helper functions for test.
  */
-const AdmZip = require('adm-zip')
 const _ = require('lodash')
 const fs = require('fs-extra')
 const ini = require('ini')
 const { ObjectWritableMock } = require('stream-mock')
+const EventEmitter = require('events')
+const commander = require('commander')
 
-function listZipEntries (buffer) {
-  const zip = new AdmZip(buffer)
-  return zip.getEntries()
-}
+const commandEmitter = new EventEmitter()
 
 /**
  * Build command line arguments.
@@ -144,12 +142,44 @@ function parseMultipart (text) {
   return data
 }
 
+/**
+ * Inject an event emitter into every actions,
+ * so that we can inpect an action completed by listening to the `action:finish` event.
+ *
+ * @returns {undefined}
+ */
+function spyActions () {
+  const originalAction = commander.Command.prototype.action
+  commander.Command.prototype.action = function (action) {
+    originalAction.call(this, async (...args) => {
+      try {
+        await action(...args)
+      } catch (err) {
+        // ignore
+      }
+      commandEmitter.emit('action:finish')
+    })
+  }
+}
+
+/**
+ * Wait for an action finished by listening to the `action:finish` event.
+ *
+ * @returns {Promise}
+ */
+async function waitForCommandExit () {
+  return new Promise((resolve) => {
+    commandEmitter.on('action:finish', () => { resolve() })
+  })
+}
+
 module.exports = {
-  listZipEntries,
   buildArgs,
   mockFunction,
   mockRCConfig,
   mockGlobalConfig,
   mockDownload,
-  parseMultipart
+  parseMultipart,
+  spyActions,
+  waitForCommandExit
 }

test/common/testHelper.js

@@ -2,14 +2,12 @@
  * Test for the Config command.
  */
 const chai = require('chai')
-const delay = require('delay')
 const fs = require('fs-extra')
 const _ = require('lodash')
 const ini = require('ini')
 
 const testHelper = require('./common/testHelper')
 const testData = require('./common/testData')
-const testConfig = require('./common/testConfig')
 const logger = require('../src/common/logger')
 let { program } = require('../bin/topcoder-cli')
 
@@ -67,46 +65,46 @@ describe('Config Command Test', async function () {
       }
     )
     program.parse(localTestData.argsWithList)
-    await delay(testConfig.WAIT_TIME)
+    await testHelper.waitForCommandExit()
     chai.expect(_.last(messages)).to.equal(localTestData.globalConfigWithUserCredentials)
   })
 
   it('success - add global config', async function () {
     program.parse(localTestData.argsWithAdd)
-    await delay(testConfig.WAIT_TIME)
+    await testHelper.waitForCommandExit()
     chai.expect(content[0]).to.include('username=aaron2017')
   })
 
   it('success - add global config when config file not found', async function () {
     mocks.readFile.restore()
     mocks.readFile = testHelper.mockFunction(fs, 'readFile', () => { throw new Error() })
     program.parse(localTestData.argsWithAdd)
-    await delay(testConfig.WAIT_TIME)
+    await testHelper.waitForCommandExit()
     chai.expect(content[0]).to.include('username=aaron2017')
   })
 
   it('success - unset global config', async function () {
     program.parse(localTestData.argsWithUnset)
-    await delay(testConfig.WAIT_TIME)
+    await testHelper.waitForCommandExit()
     chai.expect(content[0]).to.include('password=xxx123')
     chai.expect(content[0]).to.not.include('username=TonyJ')
   })
 
   it('failure - add global config with invalid key', async function () {
     program.parse(localTestData.argsWithAddInvalidKey)
-    await delay(testConfig.WAIT_TIME)
+    await testHelper.waitForCommandExit()
     chai.expect(_.last(errorMessages)).to.include('Invalid key value.')
   })
 
   it('failure - add global config with extra values', async function () {
     program.parse(localTestData.argsWithExtraValues)
-    await delay(testConfig.WAIT_TIME)
+    await testHelper.waitForCommandExit()
     chai.expect(_.last(errorMessages)).to.include('Invalid number of values passed.')
   })
 
   it('failure - unset global config with non-existent key', async function () {
     program.parse(localTestData.argsWithUnsetNotExistentKey)
-    await delay(testConfig.WAIT_TIME)
+    await testHelper.waitForCommandExit()
     chai.expect(_.last(errorMessages)).to.include(`${invalidConfig[0]} is not found in the config file`)
   })
 })

test/config.command.test.js

@@ -2,14 +2,12 @@
  * Test for the FetchArtifacts command.
  */
 const chai = require('chai')
-const delay = require('delay')
 const path = require('path')
 const _ = require('lodash')
 
 const logger = require('../src/common/logger')
 const testHelper = require('./common/testHelper')
 const testData = require('./common/testData')
-const testConfig = require('./common/testConfig')
 let { program } = require('../bin/topcoder-cli')
 
 const submissionId = '4d38573f-404c-4c2f-90b1-a05ccd3fe860'
@@ -83,7 +81,7 @@ describe('FetchArtifacts Command Test', async function () {
 
   it('success - fetch artifacts to local', async function () {
     program.parse(localTestData.argsWithSubmissionId)
-    await delay(testConfig.WAIT_TIME)
+    await testHelper.waitForCommandExit()
     chai.expect(testData._variables.artifactDownloadInfo.length).to.equal(
       testData.responses.submissionAPI.searchArtifacts.artifacts.length
     )
@@ -98,7 +96,7 @@ describe('FetchArtifacts Command Test', async function () {
       mocks.returnEmptyRC.restore()
       mocks.mockRCConfig = testHelper.mockRCConfig({ submissionId, ...credentials })
       program.parse(localTestData.argsBasic)
-      await delay(testConfig.WAIT_TIME)
+      await testHelper.waitForCommandExit()
       chai.expect(testData._variables.artifactDownloadInfo.length).to.equal(
         testData.responses.submissionAPI.searchArtifacts.artifacts.length
       )
@@ -111,7 +109,7 @@ describe('FetchArtifacts Command Test', async function () {
       mocks.mockRCConfig = testHelper.mockRCConfig({ submissionId })
       mocks.mockGlobalConfig = testHelper.mockGlobalConfig(credentials)
       program.parse(localTestData.argsBasic)
-      await delay(testConfig.WAIT_TIME)
+      await testHelper.waitForCommandExit()
       chai.expect(testData._variables.artifactDownloadInfo.length).to.equal(
         testData.responses.submissionAPI.searchArtifacts.artifacts.length
       )
@@ -121,7 +119,7 @@ describe('FetchArtifacts Command Test', async function () {
 
   it('success - fetch artifacts to local with argument dev', async function () {
     program.parse(localTestData.argsWithDev)
-    await delay(testConfig.WAIT_TIME)
+    await testHelper.waitForCommandExit()
     chai.expect(process.env.NODE_ENV).to.equal('dev')
     chai.expect(testData._variables.artifactDownloadInfo.length).to.equal(
       testData.responses.submissionAPI.searchArtifacts.artifacts.length
@@ -131,7 +129,7 @@ describe('FetchArtifacts Command Test', async function () {
 
   it('success - fetch artifacts to local with argument legacySubmissionId', async function () {
     program.parse(localTestData.argsWithLegacySubmissionId)
-    await delay(testConfig.WAIT_TIME)
+    await testHelper.waitForCommandExit()
     chai.expect(testData._variables.artifactDownloadInfo.length).to.equal(
       testData.responses.submissionAPI.searchArtifacts.artifacts.length
     )
@@ -140,14 +138,14 @@ describe('FetchArtifacts Command Test', async function () {
 
   it('success - show info if no artifacts found', async function () {
     program.parse(localTestData.argsWithSubmissionIdContainingZeroArtifacts)
-    await delay(testConfig.WAIT_TIME)
+    await testHelper.waitForCommandExit()
     chai.expect(_.nth(messages, -2)).to.equal(`No artifact exists for submission with ID: ${testData.submissionIdWithZeroArtifacts}.`)
     chai.expect(_.nth(messages, -1)).to.equal('All Done!')
   })
 
   it('failure - fetch artifacts missing password', async function () {
     program.parse(localTestData.argsWithoutPassword)
-    await delay(testConfig.WAIT_TIME)
+    await testHelper.waitForCommandExit()
     chai.expect(_.last(errorMessages)).to.include('"username" missing required peer "password"')
   })
 
@@ -157,7 +155,7 @@ describe('FetchArtifacts Command Test', async function () {
     mocks.mockRCConfig = testHelper.mockRCConfig({ submissionId })
     mocks.mockGlobalConfig = testHelper.mockGlobalConfig(_.pick(testData.m2mConfig, ['m2m.client_secret']))
     program.parse(localTestData.argsBasic)
-    await delay(testConfig.WAIT_TIME)
+    await testHelper.waitForCommandExit()
     chai.expect(_.last(errorMessages)).to.include('m2m.client_id" is required')
   })
 
@@ -167,34 +165,34 @@ describe('FetchArtifacts Command Test', async function () {
     mocks.mockRCConfig = testHelper.mockRCConfig({ submissionId })
     mocks.mockGlobalConfig = testHelper.mockGlobalConfig(_.pick(testData.m2mConfig, ['m2m.client_id']))
     program.parse(localTestData.argsBasic)
-    await delay(testConfig.WAIT_TIME)
+    await testHelper.waitForCommandExit()
     chai.expect(_.last(errorMessages)).to.include('m2m.client_secret" is required')
   })
 
   it('failure - fetch artifacts without submissionId or legacySubmissionId', async function () {
     program.parse(localTestData.argsWithoutId)
-    await delay(testConfig.WAIT_TIME)
+    await testHelper.waitForCommandExit()
     chai.expect(_.last(errorMessages)).to.include('"value" must contain at least one of [submissionId, legacySubmissionId]')
   })
 
   it('failure - fetch artifacts provided both submissionId and legacySubmissionId', async function () {
     program.parse(localTestData.argsWithBothIds)
-    await delay(testConfig.WAIT_TIME)
+    await testHelper.waitForCommandExit()
     chai.expect(_.last(errorMessages)).to.include('contains a conflict between exclusive peers [submissionId, legacySubmissionId]')
   })
 
   it('failure - it should handle possible request errors', async function () {
     testData._variables.needErrorResponse = true // instruct nock server to return 500
     program.parse(localTestData.argsWithSubmissionId)
-    await delay(testConfig.WAIT_TIME)
+    await testHelper.waitForCommandExit()
     chai.expect(_.nth(errorMessages, 0)).to.include('Couldn\'t download artifact')
   })
 
   it('failure - fetch artifacts provided both m2m and userCredentials', async function () {
     mocks.returnEmptyRC.restore()
     mocks.mockRCConfig = testHelper.mockRCConfig({ submissionId, ...testData.m2mConfig })
     program.parse(localTestData.argsWithSubmissionId)
-    await delay(testConfig.WAIT_TIME)
+    await testHelper.waitForCommandExit()
     chai.expect(_.last(errorMessages)).to.include('contains a conflict between exclusive peers [username, m2m]')
   })
 })