Add compatibility with deprecated options to Settings

Author: javierbrea

jest.config.js

@@ -17,10 +17,10 @@ module.exports = {
   // An object that configures minimum threshold enforcement for coverage results
   coverageThreshold: {
     global: {
-      branches: 80,
-      functions: 78,
-      lines: 80,
-      statements: 80
+      branches: 85,
+      functions: 82,
+      lines: 85,
+      statements: 85
     }
   },
 
@@ -47,7 +47,7 @@ module.exports = {
     "**/test/unit/core/settings/Options.spec.js",
     "**/test/unit/core/settings/Settings.spec.js"
   ],
-  //testMatch: ["**/test/unit/core/settings/Settings.spec.js"],
+  //testMatch: ["**/test/unit/core/settings/Settings.spec.js", "**/test/unit/core/settings/Options.spec.js"],
 
   // The test environment that will be used for testing
   testEnvironment: "node"

lib/core/settings/Options.js

@@ -28,13 +28,19 @@ const DEFAULT_OPTIONS = {
   features: null
 };
 
+const DEPRECATED_OPTIONS = {
+  feature: "behavior",
+  features: "behaviors"
+};
+
 class Options {
   constructor(coreOptions = {}) {
     this._onlyProgrammaticOptions = coreOptions.onlyProgrammaticOptions;
     this._options = {};
     this._optionsNames = Object.keys(DEFAULT_OPTIONS);
     this._customDefaults = {};
     this._initialized = false;
+    this._removeDeprecatedOption = this._removeDeprecatedOption.bind(this);
     this._commandLineArguments = new CommandLineArguments(DEFAULT_OPTIONS);
   }
 
@@ -48,14 +54,14 @@ class Options {
       };
       if (!this._onlyProgrammaticOptions) {
         await this._commandLineArguments.init();
-        this._options = this._getDefinedOptions(
+        this._options = this._getValidOptions(
           this._removeDeprecatedOptions({
             ...baseOptions,
             ...this._commandLineArguments.options
           })
         );
       } else {
-        this._options = this._getDefinedOptions(this._removeDeprecatedOptions(baseOptions));
+        this._options = this._getValidOptions(this._removeDeprecatedOptions(baseOptions));
       }
     }
     return Promise.resolve();
@@ -101,7 +107,20 @@ class Options {
     this._commandLineArguments.addCustom(optionDetails);
   }
 
-  _getDefinedOptions(options) {
+  getValidOptionName(optionName) {
+    if (this._optionsNames.includes(optionName) && !DEPRECATED_OPTIONS[optionName]) {
+      return optionName;
+    }
+    if (DEPRECATED_OPTIONS[optionName]) {
+      tracer.warn(
+        `Deprecation warning: ${optionName} option will be deprecated. Use ${DEPRECATED_OPTIONS[optionName]} instead`
+      );
+      return DEPRECATED_OPTIONS[optionName];
+    }
+    throw new Error("Not valid option");
+  }
+
+  _getValidOptions(options) {
     return this._optionsNames.reduce((cleanObject, currentKey) => {
       if (!isUndefined(options[currentKey])) {
         cleanObject[currentKey] = options[currentKey];
@@ -110,30 +129,28 @@ class Options {
     }, {});
   }
 
-  // TODO, remove deprecated options
-  _removeDeprecatedOptions(options) {
-    if (options.feature !== DEFAULT_OPTIONS.feature) {
-      tracer.warn(
-        "Deprecation warning: --feature option will be deprecated. Use --behavior instead"
-      );
-      if (!options.behavior) {
-        options.behavior = options.feature;
+  _removeDeprecatedOption(options, optionName) {
+    if (options[optionName] !== DEFAULT_OPTIONS[optionName]) {
+      const newOption = DEPRECATED_OPTIONS[optionName];
+      if (!options[newOption]) {
+        options[newOption] = options[optionName];
       }
-    }
-
-    if (options.features !== DEFAULT_OPTIONS.features) {
       tracer.warn(
-        "Deprecation warning: --features option will be deprecated. Use --behaviors instead"
+        `Deprecation warning: --${optionName} option will be deprecated. Use --${DEPRECATED_OPTIONS[optionName]} instead`
       );
-      if (!options.behaviors) {
-        options.behaviors = options.features;
-      }
     }
-
-    delete options.feature;
-    delete options.features;
+    delete options[optionName];
     return options;
   }
+
+  _removeDeprecatedOptions(options) {
+    let newOptions = options;
+    Object.keys(DEPRECATED_OPTIONS).forEach(optionName => {
+      newOptions = this._removeDeprecatedOption(newOptions, optionName);
+    });
+
+    return newOptions;
+  }
 }
 
 module.exports = Options;

lib/core/settings/Settings.js

@@ -41,18 +41,19 @@ class Settings {
   }
 
   set(option, value) {
-    if (this._settings[option] !== value) {
-      this._settings[option] = value;
-      this._newSettings[option] = value;
+    const optionName = this._optionsHandler.getValidOptionName(option);
+    if (this._settings[optionName] !== value) {
+      this._settings[optionName] = value;
+      this._newSettings[optionName] = value;
       this._emitChange();
-      if (option === "log") {
+      if (optionName === "log") {
         this._setTracerLevel();
       }
     }
   }
 
   get(option) {
-    return this._settings[option];
+    return this._settings[this._optionsHandler.getValidOptionName(option)];
   }
 
   addCustom(option) {

test/unit/core/settings/Options.mocks.js

@@ -22,8 +22,10 @@ class Mock {
     this._stubs = {
       init: this._sandbox.stub(),
       addCustom: this._sandbox.stub(),
+      getValidOptionName: this._sandbox.stub(),
       options: {
-        log: "foo-log-level"
+        log: "foo-log-level",
+        behavior: "foo-behavior"
       }
     };
 

test/unit/core/settings/Options.spec.js

@@ -199,6 +199,27 @@ describe("options", () => {
       });
     });
 
+    it("should remove deprecated options", async () => {
+      commandLineArgumentsMocks.stubs.instance.options = {
+        behavior: "foo-behavior",
+        cli: true,
+        behaviors: "foo/behaviors/path",
+        foo: undefined,
+        foo2: "foooo",
+        recursive: false
+      };
+      await options.init();
+      expect(options.options).toEqual({
+        port: 3100,
+        host: "0.0.0.0",
+        log: "info",
+        delay: 0,
+        watch: true,
+        behavior: "foo-behavior",
+        behaviors: "foo/behaviors/path"
+      });
+    });
+
     it("should get values from keys defined in new options", async () => {
       commandLineArgumentsMocks.stubs.instance.options = {
         behavior: "foo-behavior",
@@ -312,6 +333,26 @@ describe("options", () => {
       });
     });
 
+    it("should remove deprecated options", async () => {
+      await options.init({
+        behavior: "foo-behavior",
+        cli: true,
+        behaviors: "foo/behaviors/path",
+        foo: undefined,
+        foo2: "foooo",
+        recursive: false
+      });
+      expect(options.options).toEqual({
+        port: 3100,
+        host: "0.0.0.0",
+        log: "info",
+        delay: 0,
+        watch: true,
+        behavior: "foo-behavior",
+        behaviors: "foo/behaviors/path"
+      });
+    });
+
     it("should get values from keys defined in new options", async () => {
       options.addCustom({
         name: "cli",
@@ -394,4 +435,42 @@ describe("options", () => {
       });
     });
   });
+
+  describe("getValidOptionName method", () => {
+    it("should throw an error if option is not valid", async () => {
+      expect.assertions(1);
+      await options.init();
+      try {
+        options.getValidOptionName("foo");
+      } catch (error) {
+        expect(error.message).toEqual(expect.stringContaining("Not valid option"));
+      }
+    });
+
+    it("should return option name if option is valid", async () => {
+      await options.init();
+      expect(options.getValidOptionName("behavior")).toEqual("behavior");
+    });
+
+    it("should return new option name if option is deprecated", async () => {
+      expect.assertions(2);
+      await options.init();
+      const option = options.getValidOptionName("feature");
+      expect(
+        tracer.warn.calledWith(
+          "Deprecation warning: feature option will be deprecated. Use behavior instead"
+        )
+      ).toEqual(true);
+      expect(option).toEqual("behavior");
+    });
+
+    it("should return true if option is custom option", async () => {
+      options.addCustom({
+        name: "foo",
+        type: "boolean"
+      });
+      await options.init();
+      expect(options.getValidOptionName("foo")).toEqual("foo");
+    });
+  });
 });

test/unit/core/settings/Settings.spec.js

@@ -29,6 +29,7 @@ describe("Settings", () => {
     sandbox = sinon.createSandbox();
     optionsMocks = new OptionsMocks();
     optionsInstance = optionsMocks.stubs.instance;
+    optionsInstance.getValidOptionName.callsFake(name => name);
     coreMocks = new CoreMocks();
     coreInstance = coreMocks.stubs.instance;
     sandbox.stub(tracer, "set");
@@ -58,6 +59,11 @@ describe("Settings", () => {
       expect(settings.get("log")).toEqual("foo-log-level");
     });
 
+    it("should return current deprecated option value", () => {
+      optionsInstance.getValidOptionName.returns("behavior");
+      expect(settings.get("log")).toEqual("foo-behavior");
+    });
+
     it("should return new value if set is called", () => {
       settings.set("log", "foo-new-value");
       expect(settings.get("log")).toEqual("foo-new-value");
@@ -70,6 +76,12 @@ describe("Settings", () => {
       expect(tracer.set.calledWith("console", "foo-new-value")).toEqual(true);
     });
 
+    it("should set new option if provided one is deprecated", () => {
+      optionsInstance.getValidOptionName.returns("behavior");
+      settings.set("log", "foo");
+      expect(settings.get("behavior")).toEqual("foo");
+    });
+
     it("should emit change if setting has changed value", () => {
       settings.set("foo", "foo-new-value");
       expect(