fix: do not throw error exit code when the enable-skill request skipp…

…ed (#266)
alexa · Jul 21, 2020 · 0a2b166 · 0a2b166
1 parent 0e50cfe
commit 0a2b166
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 16 deletions.
diff --git 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
@@ -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
@@ -440,14 +440,32 @@ 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';
                 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, '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
@@ -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,19 +228,19 @@ 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', () => {
             // setup
             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) => {