From 0a2b1664a448d67eaa6d2dfe045bac6bc5b8db37 Mon Sep 17 00:00:00 2001 From: "Nong (Ron) Wang" Date: Mon, 20 Jul 2020 19:46:05 -0700 Subject: [PATCH] fix: do not throw error exit code when the enable-skill request skipped (#266) --- lib/commands/deploy/index.js | 4 +++ .../skill-metadata-controller/index.js | 7 +++-- test/unit/commands/deploy/index-test.js | 18 ++++++++++++ .../skill-metadata-controller-test.js | 28 ++++++++++--------- 4 files changed, 41 insertions(+), 16 deletions(-) diff --git a/lib/commands/deploy/index.js b/lib/commands/deploy/index.js index 467fdc60..0d036cc2 100644 --- a/lib/commands/deploy/index.js +++ b/lib/commands/deploy/index.js @@ -77,6 +77,10 @@ class DeployCommand extends AbstractCommand { // Post deploy logic // call smapiClient to enable skill helper.enableSkill(profile, cmd.debug, (enableError) => { + if (enableError instanceof CliWarn) { + Messenger.getInstance().warn(enableError); + return cb(); + } if (enableError) { Messenger.getInstance().error(enableError); return cb(enableError); diff --git a/lib/controllers/skill-metadata-controller/index.js b/lib/controllers/skill-metadata-controller/index.js index 8c2dc770..062e4a46 100644 --- a/lib/controllers/skill-metadata-controller/index.js +++ b/lib/controllers/skill-metadata-controller/index.js @@ -14,6 +14,7 @@ const zipUtils = require('@src/utils/zip-utils'); const hashUtils = require('@src/utils/hash-utils'); const CONSTANTS = require('@src/utils/constants'); const CLiError = require('@src/exceptions/cli-error'); +const CLiWarn = require('@src/exceptions/cli-warn'); module.exports = class SkillMetadataController { /** @@ -72,15 +73,15 @@ module.exports = class SkillMetadataController { validateDomain() { const domainInfo = Manifest.getInstance().getApis(); if (!domainInfo || R.isEmpty(domainInfo)) { - throw new CLiError('[Error]: Skill information is not valid. Please make sure "apis" field in the skill.json is not empty.'); + throw new CLiError('Skill information is not valid. Please make sure "apis" field in the skill.json is not empty.'); } const domainList = R.keys(domainInfo); if (domainList.length !== 1) { - throw new CLiError('[Warn]: Skill with multiple api domains cannot be enabled. Skip the enable process.'); + throw new CLiWarn('Skill with multiple api domains cannot be enabled. Skip the enable process.'); } if (CONSTANTS.SKILL.DOMAIN.CAN_ENABLE_DOMAIN_LIST.indexOf(domainList[0]) === -1) { - throw new CLiError(`[Warn]: Skill api domain "${domainList[0]}" cannot be enabled. Skip the enable process.`); + throw new CLiWarn(`Skill api domain "${domainList[0]}" cannot be enabled. Skip the enable process.`); } } diff --git a/test/unit/commands/deploy/index-test.js b/test/unit/commands/deploy/index-test.js index eaff6995..62fd783b 100644 --- a/test/unit/commands/deploy/index-test.js +++ b/test/unit/commands/deploy/index-test.js @@ -440,6 +440,22 @@ with build flow ${TEST_CODE_BUILD_RESULT[0].buildFlow}.`); sinon.stub(fse, 'existsSync').returns(true); }); + it('| can callbcak warn when enable fails with CliWarn class', (done) => { + // setup + const TEST_WARN = new CliWarn('warn'); + sinon.stub(helper, 'deploySkillMetadata').callsArgWith(1); + sinon.stub(helper, 'buildSkillCode').callsArgWith(2, null, TEST_CODE_BUILD_RESULT); + sinon.stub(helper, 'deploySkillInfrastructure').callsArgWith(3); + sinon.stub(helper, 'enableSkill').callsArgWith(2, TEST_WARN); + // call + instance.handle(TEST_CMD, (err) => { + // verify + expect(warnStub.args[0][0]).equal(TEST_WARN); + expect(err).equal(undefined); + done(); + }); + }); + it('| can callbcak error when enable fails', (done) => { // setup const TEST_ERROR = 'error'; @@ -447,7 +463,9 @@ with build flow ${TEST_CODE_BUILD_RESULT[0].buildFlow}.`); sinon.stub(helper, 'buildSkillCode').callsArgWith(2, null, TEST_CODE_BUILD_RESULT); sinon.stub(helper, 'deploySkillInfrastructure').callsArgWith(3); sinon.stub(helper, 'enableSkill').callsArgWith(2, 'error'); + // call instance.handle(TEST_CMD, (err) => { + // verify expect(errorStub.args[0][0]).equal(TEST_ERROR); expect(err).equal(TEST_ERROR); done(); diff --git a/test/unit/controller/skill-metadata-controller-test.js b/test/unit/controller/skill-metadata-controller-test.js index c9c2a7b5..e2a936ba 100644 --- a/test/unit/controller/skill-metadata-controller-test.js +++ b/test/unit/controller/skill-metadata-controller-test.js @@ -2,17 +2,19 @@ const { expect } = require('chai'); const sinon = require('sinon'); const path = require('path'); const fs = require('fs-extra'); -const jsonView = require('@src/view/json-view'); -const CLiError = require('@src/exceptions/cli-error'); -const ResourcesConfig = require('@src/model/resources-config'); + const httpClient = require('@src/clients/http-client'); -const SkillMetadataController = require('@src/controllers/skill-metadata-controller'); const AuthorizationController = require('@src/controllers/authorization-controller'); +const SkillMetadataController = require('@src/controllers/skill-metadata-controller'); +const CliError = require('@src/exceptions/cli-error'); +const CliWarn = require('@src/exceptions/cli-warn'); const Manifest = require('@src/model/manifest'); -const Messenger = require('@src/view/messenger'); -const zipUtils = require('@src/utils/zip-utils'); -const hashUtils = require('@src/utils/hash-utils'); +const ResourcesConfig = require('@src/model/resources-config'); const CONSTANTS = require('@src/utils/constants'); +const hashUtils = require('@src/utils/hash-utils'); +const zipUtils = require('@src/utils/zip-utils'); +const jsonView = require('@src/view/json-view'); +const Messenger = require('@src/view/messenger'); describe('Controller test - skill metadata controller test', () => { const FIXTURE_RESOURCES_CONFIG_FILE_PATH = path.join(process.cwd(), 'test', 'unit', 'fixture', 'model', 'regular-proj', 'ask-resources.json'); @@ -215,9 +217,9 @@ describe('Controller test - skill metadata controller test', () => { it('| return error when dominInfo is not provided', () => { // setup Manifest.getInstance().setApis({}); - const expectedErrMessage = '[Error]: Skill information is not valid. Please make sure "apis" field in the skill.json is not empty.'; + const expectedErrMessage = 'Skill information is not valid. Please make sure "apis" field in the skill.json is not empty.'; // call - expect(() => skillMetaController.validateDomain()).to.throw(CLiError, expectedErrMessage); + expect(() => skillMetaController.validateDomain()).to.throw(CliError, expectedErrMessage); }); it('| return error when dominInfo contains more than one domain', () => { @@ -226,9 +228,9 @@ describe('Controller test - skill metadata controller test', () => { custom: {}, smartHome: {} }); - const expectedErrMessage = '[Warn]: Skill with multiple api domains cannot be enabled. Skip the enable process.'; + const expectedErrMessage = 'Skill with multiple api domains cannot be enabled. Skip the enable process.'; // call - expect(() => skillMetaController.validateDomain()).to.throw(CLiError, expectedErrMessage); + expect(() => skillMetaController.validateDomain()).to.throw(CliWarn, expectedErrMessage); }); it('| return error when domain cannot be enabled', () => { @@ -236,9 +238,9 @@ describe('Controller test - skill metadata controller test', () => { Manifest.getInstance().setApis({ smartHome: {} }); - const expectedErrMessage = '[Warn]: Skill api domain "smartHome" cannot be enabled. Skip the enable process.'; + const expectedErrMessage = 'Skill api domain "smartHome" cannot be enabled. Skip the enable process.'; // call - expect(() => skillMetaController.validateDomain()).to.throw(CLiError, expectedErrMessage); + expect(() => skillMetaController.validateDomain()).to.throw(CliWarn, expectedErrMessage); }); it('| callback error when getSkillEnablement return error', (done) => {