From 09ab0468c07d537a6e560af6dfdc05554e01237e Mon Sep 17 00:00:00 2001
From: Andrey Tserkus <zerkyn@gmail.com>
Date: Thu, 16 Feb 2017 23:18:56 -0800
Subject: [PATCH] Improved tests for `logBreakingChange()` function: - fixed
 the tests when `SLS_IGNORE_WARNING` is set in the shell environment -
 refactored the tests to clearly expose the tested input combinations (+ added
 the missing combination - to have a formally complete verification) - added
 the `SLS_IGNORE_WARNING=*` setting to Travis builds, so that their output
 doesn't contain unnecessary noise about upcoming changes

---
 .travis.yml             |   6 +++
 lib/classes/CLI.test.js | 106 +++++++++++++++++++++++-----------------
 2 files changed, 68 insertions(+), 44 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 4f75d48e7..7806cdef9 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -3,8 +3,14 @@ language: node_js
 matrix:
   include:
     - node_js: '4.4'
+      env:
+        SLS_IGNORE_WARNING=*
     - node_js: '5.11'
+      env:
+        SLS_IGNORE_WARNING=*
     - node_js: '6.2'
+      env:
+        SLS_IGNORE_WARNING=*
     - node_js: '6.2'
       env:
         - INTEGRATION_TEST=true
diff --git a/lib/classes/CLI.test.js b/lib/classes/CLI.test.js
index 05bc82a7b..f27aab9ad 100644
--- a/lib/classes/CLI.test.js
+++ b/lib/classes/CLI.test.js
@@ -307,67 +307,85 @@ describe('CLI', () => {
 
   describe('#logBreakingChanges()', () => {
     let consoleLogStub;
+    let origIgnoreWarning;
 
     beforeEach(() => {
       cli = new CLI(serverless);
       consoleLogStub = sinon.stub(cli, 'consoleLog').returns();
+      origIgnoreWarning = process.env.SLS_IGNORE_WARNING;
     });
 
     afterEach(() => {
       cli.consoleLog.restore();
-      delete process.env.SLS_IGNORE_WARNING;
+      if (origIgnoreWarning === undefined) {
+        delete process.env.SLS_IGNORE_WARNING;
+      } else {
+        process.env.SLS_IGNORE_WARNING = origIgnoreWarning;
+      }
     });
 
-    it('should log breaking changes when they are provided', () => {
-      const nextVersion = 'Next';
+    // Test all possible combinations of provided changes and "ignored" setting
+    [
+      {
+        name: 'should NOT LOG breaking changes when changes are NOT PROVIDED and NOT IGNORED',
+        isProvided: false,
+        isIgnored: false,
+        isLogExpected: false,
+      },
+      {
+        name: 'should NOT LOG breaking changes when changes are NOT PROVIDED and IGNORED',
+        isProvided: false,
+        isIgnored: true,
+        isLogExpected: false,
+      },
+      {
+        name: 'should LOG breaking changes when changes are PROVIDED and NOT IGNORED',
+        isProvided: true,
+        isIgnored: false,
+        isLogExpected: true,
+      },
+      {
+        name: 'should NOT LOG breaking changes when changes are PROVIDED and IGNORED',
+        isProvided: true,
+        isIgnored: true,
+        isLogExpected: false,
+      },
+    ].forEach((testCase) => {
+      it(testCase.name, () => {
+        // Stubs
+        const nextVersion = 'Next';
 
-      cli.breakingChanges = [
-        'x is broken',
-        'y will be updated',
-      ];
+        let outMessage = '\n';
+        outMessage += chalk.yellow(`  WARNING: You are running v${serverlessVersion}. v${nextVersion} will include the following breaking changes:\n`); //eslint-disable-line
+        outMessage += chalk.yellow('    - x is broken\n');
+        outMessage += chalk.yellow('    - y will be updated\n');
+        outMessage += '\n';
+        outMessage += chalk.yellow('  You can opt-out from these warnings by setting the "SLS_IGNORE_WARNING=*" environment variable.\n'); //eslint-disable-line
 
-      let expectedMessage = '\n';
-      expectedMessage += chalk.yellow(`  WARNING: You are running v${serverlessVersion}. v${nextVersion} will include the following breaking changes:\n`); //eslint-disable-line
-      expectedMessage += chalk.yellow('    - x is broken\n');
-      expectedMessage += chalk.yellow('    - y will be updated\n');
-      expectedMessage += '\n';
-      expectedMessage += chalk.yellow('  You can opt-out from these warnings by setting the "SLS_IGNORE_WARNING=*" environment variable.\n'); //eslint-disable-line
+        const breakingChanges = [
+          'x is broken',
+          'y will be updated',
+        ];
 
-      const message = cli.logBreakingChanges(nextVersion);
+        // Prepare the test environment
+        cli.breakingChanges = testCase.isProvided ? breakingChanges : [];
 
-      expect(consoleLogStub.calledOnce).to.equal(true);
-      expect(message).to.equal(expectedMessage);
-    });
+        if (testCase.isIgnored) {
+          process.env.SLS_IGNORE_WARNING = '*';
+        } else {
+          delete process.env.SLS_IGNORE_WARNING;
+        }
 
-    it('should not log breaking changes when they are not provided', () => {
-      cli.breakingChanges = [];
+        // Test expectations
+        const expectedResult = testCase.isLogExpected ? outMessage : '';
+        const expectedLogCalled = testCase.isLogExpected;
 
-      const expectedMessage = '';
+        // Carry the test
+        const actualResult = cli.logBreakingChanges(nextVersion);
 
-      const message = cli.logBreakingChanges();
-
-      expect(consoleLogStub.calledOnce).to.equal(false);
-      expect(message).to.equal(expectedMessage);
-    });
-
-    it('should not log breaking changes when the "disable environment variable" is set', () => {
-      // we have some breaking changes
-      cli.breakingChanges = [
-        'x is broken',
-        'y will be updated',
-      ];
-
-      // this should prevent the breaking changes from being logged
-      process.env.SLS_IGNORE_WARNING = '*';
-
-      cli.breakingChanges = [];
-
-      const expectedMessage = '';
-
-      const message = cli.logBreakingChanges();
-
-      expect(consoleLogStub.calledOnce).to.equal(false);
-      expect(message).to.equal(expectedMessage);
+        expect(actualResult).to.equal(expectedResult);
+        expect(consoleLogStub.calledOnce).to.equal(expectedLogCalled);
+      });
     });
   });