From 212f6ce8bb093fe6aa05e927a8c3e297685040ef Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 5 Dec 2018 16:28:10 -0800 Subject: [PATCH 01/10] Add experimental flag to guard development features This commit introduces a special flag for enabling features that are not ready for release. It can either be enabled by setting the `VAGRANT_EXPERIMENTAL` flag to "1", or by setting it to a string of one or more comma seperated values for specific features. It also adds a couple of Vagrant developer focused methods for making it easier to determine if the flag has been enabled, and if so, what features. --- bin/vagrant | 25 ++++++++ lib/vagrant/util/experimental.rb | 48 +++++++++++++++ templates/locales/en.yml | 12 ++++ test/unit/vagrant/util/experimental_test.rb | 67 +++++++++++++++++++++ 4 files changed, 152 insertions(+) create mode 100644 lib/vagrant/util/experimental.rb create mode 100644 test/unit/vagrant/util/experimental_test.rb diff --git a/bin/vagrant b/bin/vagrant index 7372a726d..4ab3f013d 100755 --- a/bin/vagrant +++ b/bin/vagrant @@ -159,6 +159,31 @@ begin env.ui.warn(I18n.t("vagrant.general.not_in_installer") + "\n", prefix: false) end + # Acceptable experimental flag values include: + # + # Unset - Disables experimental features + # 0 - Disables experimental features + # 1 - Enables all features + # String - Enables one or more features, separated by commas + if ENV["VAGRANT_EXPERIMENTAL"] + experimental = ENV["VAGRANT_EXPERIMENTAL"].to_s + if experimental == "0" + logger.debug("Experimental flag is set but disabled") + else + ui = Vagrant::UI::Prefixed.new(env.ui, "vagrant") + if experimental != "1" + logger.debug("Experimental flag is enabled") + features = experimental.split(',') + ui.warn(I18n.t("vagrant.general.experimental.features", features: features.join(", ")), bold: true, prefix: true, channel: :error) + elsif experimental == "1" + logger.debug("Experimental flag is enabled") + ui.warn(I18n.t("vagrant.general.experimental.all"), bold: true, prefix: true, channel: :error) + else + logger.warn("Experimental flag is set to an unsupported value") + end + end + end + begin # Execute the CLI interface, and exit with the proper error code exit_status = env.cli(argv) diff --git a/lib/vagrant/util/experimental.rb b/lib/vagrant/util/experimental.rb new file mode 100644 index 000000000..732a0643c --- /dev/null +++ b/lib/vagrant/util/experimental.rb @@ -0,0 +1,48 @@ +module Vagrant + module Util + class Experimental + VALID_FEATURES = [].freeze + class << self + + # A method for determining if the experimental flag has been enabled + # + # @return [Boolean] + def enabled? + if !defined?(@_experimental) + experimental = ENV["VAGRANT_EXPERIMENTAL"].to_s + if experimental != "0" && !experimental.empty? + @_experimental = true + else + @_experimental = false + end + end + @_experimental + end + + # A method for Vagrant internals to determine if a given feature + # has been abled and can be used. + # + # @param [String] - An array of strings of features to check against + # @return [Boolean] - A hash containing the original array and if it is valid + def feature_enabled?(feature) + experimental = ENV["VAGRANT_EXPERIMENTAL"].to_s.downcase + if experimental == "1" + return true + elsif VALID_FEATURES.include?(feature) && + experimental.split(',').include?(feature) + return true + else + return false + end + end + + # @private + # Reset the cached values for platform. This is not considered a public + # API and should only be used for testing. + def reset! + instance_variables.each(&method(:remove_instance_variable)) + end + end + end + end +end diff --git a/templates/locales/en.yml b/templates/locales/en.yml index e6967d2a0..3890182b6 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -393,6 +393,18 @@ en: shown below. %{output} + experimental: + all: |- + You have enabled the experimental flag with all features enabled. + Please use with caution, as some of the features may not be fully + functional yet. + features: |- + You have requested to enabled the experimental flag with the following features: + + Features: %{features} + + Please use with caution, as some of the features may not be fully + functional yet. not_in_installer: |- You appear to be running Vagrant outside of the official installers. Note that the installers are what ensure that Vagrant has all required diff --git a/test/unit/vagrant/util/experimental_test.rb b/test/unit/vagrant/util/experimental_test.rb new file mode 100644 index 000000000..222e19723 --- /dev/null +++ b/test/unit/vagrant/util/experimental_test.rb @@ -0,0 +1,67 @@ +require File.expand_path("../../../base", __FILE__) + +require "vagrant/util/experimental" + +describe Vagrant::Util::Experimental do + include_context "unit" + before(:each) { described_class.reset! } + subject { described_class } + + describe "#enabled?" do + it "returns true if enabled with '1'" do + allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("1") + expect(subject.enabled?).to eq(true) + end + + it "returns true if enabled with a list of features" do + allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("list,of,features") + expect(subject.enabled?).to eq(true) + end + + it "returns false if disabled" do + allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("0") + expect(subject.enabled?).to eq(false) + end + + it "returns false if not set" do + allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return(nil) + expect(subject.enabled?).to eq(false) + end + end + + describe "#feature_enabled?" do + before(:each) do + stub_const("Vagrant::Util::Experimental::VALID_FEATURES", ["secret_feature"]) + end + + it "returns true if flag set to 1" do + allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("1") + expect(subject.feature_enabled?("anything")).to eq(true) + end + + it "returns true if flag contains feature requested" do + allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("secret_feature") + expect(subject.feature_enabled?("secret_feature")).to eq(true) + end + + it "returns true if flag contains feature requested with other features 'enabled'" do + allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("secret_feature,other_secret") + expect(subject.feature_enabled?("secret_feature")).to eq(true) + end + + it "returns false if flag does not contain feature requested" do + allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("secret_feature") + expect(subject.feature_enabled?("anything")).to eq(false) + end + + it "returns false if flag set to 0" do + allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("0") + expect(subject.feature_enabled?("anything")).to eq(false) + end + + it "returns false if flag is not set" do + allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return(nil) + expect(subject.feature_enabled?("anything")).to eq(false) + end + end +end From 44fa134c480d693eab325dead0ffd9830b7f474b Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 7 Dec 2018 10:27:21 -0800 Subject: [PATCH 02/10] Unfreeze valid features constant --- lib/vagrant/util/experimental.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/vagrant/util/experimental.rb b/lib/vagrant/util/experimental.rb index 732a0643c..1b9f6d206 100644 --- a/lib/vagrant/util/experimental.rb +++ b/lib/vagrant/util/experimental.rb @@ -1,7 +1,7 @@ module Vagrant module Util class Experimental - VALID_FEATURES = [].freeze + VALID_FEATURES = [] class << self # A method for determining if the experimental flag has been enabled From 611e3dce968658e26cd7a46f5ce4ed384dbade7b Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 7 Dec 2018 10:27:47 -0800 Subject: [PATCH 03/10] Use util methods in vagrant bin for experimental flag --- bin/vagrant | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/bin/vagrant b/bin/vagrant index 4ab3f013d..58568ba49 100755 --- a/bin/vagrant +++ b/bin/vagrant @@ -89,6 +89,7 @@ begin require 'vagrant/bundler' require 'vagrant/cli' require 'vagrant/util/platform' + require 'vagrant/util/experimental' # Schedule the cleanup of things at_exit(&Vagrant::Bundler.instance.method(:deinit)) @@ -165,22 +166,14 @@ begin # 0 - Disables experimental features # 1 - Enables all features # String - Enables one or more features, separated by commas - if ENV["VAGRANT_EXPERIMENTAL"] - experimental = ENV["VAGRANT_EXPERIMENTAL"].to_s - if experimental == "0" - logger.debug("Experimental flag is set but disabled") + if Vagrant::Util::Experimental.enabled? + experimental = Vagrant::Util::Experimental.features_requested + ui = Vagrant::UI::Prefixed.new(env.ui, "vagrant") + logger.debug("Experimental flag is enabled") + if experimental.size >= 1 && experimental.first != "1" + ui.warn(I18n.t("vagrant.general.experimental.features", features: experimental.join(", ")), bold: true, prefix: true, channel: :error) else - ui = Vagrant::UI::Prefixed.new(env.ui, "vagrant") - if experimental != "1" - logger.debug("Experimental flag is enabled") - features = experimental.split(',') - ui.warn(I18n.t("vagrant.general.experimental.features", features: features.join(", ")), bold: true, prefix: true, channel: :error) - elsif experimental == "1" - logger.debug("Experimental flag is enabled") - ui.warn(I18n.t("vagrant.general.experimental.all"), bold: true, prefix: true, channel: :error) - else - logger.warn("Experimental flag is set to an unsupported value") - end + ui.warn(I18n.t("vagrant.general.experimental.all"), bold: true, prefix: true, channel: :error) end end From 1a32930017a3f361ccea43e2fdc8bc396bf28239 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 7 Dec 2018 10:28:21 -0800 Subject: [PATCH 04/10] Add guard_with method for protecting ruby blocks --- lib/vagrant/util/experimental.rb | 26 +++++++++++++++++---- test/unit/vagrant/util/experimental_test.rb | 23 ++++++++++++++++++ 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/lib/vagrant/util/experimental.rb b/lib/vagrant/util/experimental.rb index 1b9f6d206..cd05cf85a 100644 --- a/lib/vagrant/util/experimental.rb +++ b/lib/vagrant/util/experimental.rb @@ -22,20 +22,38 @@ module Vagrant # A method for Vagrant internals to determine if a given feature # has been abled and can be used. # - # @param [String] - An array of strings of features to check against + # @param [String] feature # @return [Boolean] - A hash containing the original array and if it is valid def feature_enabled?(feature) - experimental = ENV["VAGRANT_EXPERIMENTAL"].to_s.downcase - if experimental == "1" + experimental = features_requested + if experimental.size == 1 && experimental.first == "1" return true elsif VALID_FEATURES.include?(feature) && - experimental.split(',').include?(feature) + experimental.include?(feature) return true else return false end end + # Returns the features requested for the experimental flag + # + # @return [Array] - Returns an array of requested experimental features + def features_requested + if !defined?(@_requested_features) + @_requested_features = ENV["VAGRANT_EXPERIMENTAL"].to_s.downcase.split(',') + end + @_requested_features + end + + # A function to guard experimental blocks of code from being executed + # + # @param [Array] features - Array of features to guard a method with + # @param [Block] block - Block of ruby code to be guarded against + def guard_with(*features, &block) + yield if block_given? && features.any? {|f| feature_enabled?(f)} + end + # @private # Reset the cached values for platform. This is not considered a public # API and should only be used for testing. diff --git a/test/unit/vagrant/util/experimental_test.rb b/test/unit/vagrant/util/experimental_test.rb index 222e19723..7ebe53234 100644 --- a/test/unit/vagrant/util/experimental_test.rb +++ b/test/unit/vagrant/util/experimental_test.rb @@ -64,4 +64,27 @@ describe Vagrant::Util::Experimental do expect(subject.feature_enabled?("anything")).to eq(false) end end + + describe "#features_requested" do + it "returns an array of requested features" do + allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("secret_feature,other_secret") + expect(subject.features_requested).to eq(["secret_feature","other_secret"]) + end + end + + describe "#guard_with" do + before(:each) do + stub_const("Vagrant::Util::Experimental::VALID_FEATURES", ["secret_feature"]) + end + + it "does not execute the block if the feature is not requested" do + allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return(nil) + expect{|b| subject.guard_with("secret_feature", &b) }.not_to yield_control + end + + it "executes the block if the feature is valid and requested" do + allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("secret_feature,other_secret") + expect{|b| subject.guard_with("secret_feature", &b) }.to yield_control + end + end end From fc4ba7f420280e2105877e4bd39eba14afbe0b49 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 7 Dec 2018 10:50:20 -0800 Subject: [PATCH 05/10] Update to global_enabled? --- bin/vagrant | 2 +- lib/vagrant/util/experimental.rb | 2 +- test/unit/vagrant/util/experimental_test.rb | 10 +++++----- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/bin/vagrant b/bin/vagrant index 58568ba49..9acbcb024 100755 --- a/bin/vagrant +++ b/bin/vagrant @@ -166,7 +166,7 @@ begin # 0 - Disables experimental features # 1 - Enables all features # String - Enables one or more features, separated by commas - if Vagrant::Util::Experimental.enabled? + if Vagrant::Util::Experimental.global_enabled? experimental = Vagrant::Util::Experimental.features_requested ui = Vagrant::UI::Prefixed.new(env.ui, "vagrant") logger.debug("Experimental flag is enabled") diff --git a/lib/vagrant/util/experimental.rb b/lib/vagrant/util/experimental.rb index cd05cf85a..667e3f8a7 100644 --- a/lib/vagrant/util/experimental.rb +++ b/lib/vagrant/util/experimental.rb @@ -7,7 +7,7 @@ module Vagrant # A method for determining if the experimental flag has been enabled # # @return [Boolean] - def enabled? + def global_enabled? if !defined?(@_experimental) experimental = ENV["VAGRANT_EXPERIMENTAL"].to_s if experimental != "0" && !experimental.empty? diff --git a/test/unit/vagrant/util/experimental_test.rb b/test/unit/vagrant/util/experimental_test.rb index 7ebe53234..6d1f351b0 100644 --- a/test/unit/vagrant/util/experimental_test.rb +++ b/test/unit/vagrant/util/experimental_test.rb @@ -7,25 +7,25 @@ describe Vagrant::Util::Experimental do before(:each) { described_class.reset! } subject { described_class } - describe "#enabled?" do + describe "#global_enabled?" do it "returns true if enabled with '1'" do allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("1") - expect(subject.enabled?).to eq(true) + expect(subject.global_enabled?).to eq(true) end it "returns true if enabled with a list of features" do allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("list,of,features") - expect(subject.enabled?).to eq(true) + expect(subject.global_enabled?).to eq(true) end it "returns false if disabled" do allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("0") - expect(subject.enabled?).to eq(false) + expect(subject.global_enabled?).to eq(false) end it "returns false if not set" do allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return(nil) - expect(subject.enabled?).to eq(false) + expect(subject.global_enabled?).to eq(false) end end From d551738bc763c8cb7a3bfe9a5013a6ea1e4c856b Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 7 Dec 2018 10:50:34 -0800 Subject: [PATCH 06/10] Allow feature_enabled? to accept symbols --- lib/vagrant/util/experimental.rb | 2 ++ test/unit/vagrant/util/experimental_test.rb | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/lib/vagrant/util/experimental.rb b/lib/vagrant/util/experimental.rb index 667e3f8a7..46c3b9c86 100644 --- a/lib/vagrant/util/experimental.rb +++ b/lib/vagrant/util/experimental.rb @@ -26,6 +26,8 @@ module Vagrant # @return [Boolean] - A hash containing the original array and if it is valid def feature_enabled?(feature) experimental = features_requested + feature = feature.to_s + if experimental.size == 1 && experimental.first == "1" return true elsif VALID_FEATURES.include?(feature) && diff --git a/test/unit/vagrant/util/experimental_test.rb b/test/unit/vagrant/util/experimental_test.rb index 6d1f351b0..5a0f17ad4 100644 --- a/test/unit/vagrant/util/experimental_test.rb +++ b/test/unit/vagrant/util/experimental_test.rb @@ -44,6 +44,11 @@ describe Vagrant::Util::Experimental do expect(subject.feature_enabled?("secret_feature")).to eq(true) end + it "returns true if flag contains feature requested and the request is a symbol" do + allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("secret_feature") + expect(subject.feature_enabled?(:secret_feature)).to eq(true) + end + it "returns true if flag contains feature requested with other features 'enabled'" do allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("secret_feature,other_secret") expect(subject.feature_enabled?("secret_feature")).to eq(true) From c07f99fe7d440fb1a1c428c6dd303403347640a3 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 7 Dec 2018 10:59:21 -0800 Subject: [PATCH 07/10] Move feature flag checking into a single function --- lib/vagrant/util/experimental.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/vagrant/util/experimental.rb b/lib/vagrant/util/experimental.rb index 46c3b9c86..e1200678e 100644 --- a/lib/vagrant/util/experimental.rb +++ b/lib/vagrant/util/experimental.rb @@ -9,8 +9,8 @@ module Vagrant # @return [Boolean] def global_enabled? if !defined?(@_experimental) - experimental = ENV["VAGRANT_EXPERIMENTAL"].to_s - if experimental != "0" && !experimental.empty? + experimental = features_requested + if experimental.size >= 1 && experimental.first != "0" @_experimental = true else @_experimental = false From 01ec72cac2c51e0f52dd33aab45506086e2de226 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 7 Dec 2018 13:30:50 -0800 Subject: [PATCH 08/10] Introduce a local and global check for enabled experimental features --- bin/vagrant | 6 ++-- lib/vagrant/util/experimental.rb | 23 +++++++++++-- test/unit/vagrant/util/experimental_test.rb | 36 ++++++++++++++++++--- 3 files changed, 54 insertions(+), 11 deletions(-) diff --git a/bin/vagrant b/bin/vagrant index 9acbcb024..efe5eab63 100755 --- a/bin/vagrant +++ b/bin/vagrant @@ -166,13 +166,11 @@ begin # 0 - Disables experimental features # 1 - Enables all features # String - Enables one or more features, separated by commas - if Vagrant::Util::Experimental.global_enabled? + if Vagrant::Util::Experimental.enabled? experimental = Vagrant::Util::Experimental.features_requested ui = Vagrant::UI::Prefixed.new(env.ui, "vagrant") logger.debug("Experimental flag is enabled") - if experimental.size >= 1 && experimental.first != "1" - ui.warn(I18n.t("vagrant.general.experimental.features", features: experimental.join(", ")), bold: true, prefix: true, channel: :error) - else + if Vagrant::Util::Experimental.global_enabled? ui.warn(I18n.t("vagrant.general.experimental.all"), bold: true, prefix: true, channel: :error) end end diff --git a/lib/vagrant/util/experimental.rb b/lib/vagrant/util/experimental.rb index e1200678e..9a2c6fb48 100644 --- a/lib/vagrant/util/experimental.rb +++ b/lib/vagrant/util/experimental.rb @@ -4,10 +4,11 @@ module Vagrant VALID_FEATURES = [] class << self - # A method for determining if the experimental flag has been enabled + # A method for determining if the experimental flag has been enabled with + # any features # # @return [Boolean] - def global_enabled? + def enabled? if !defined?(@_experimental) experimental = features_requested if experimental.size >= 1 && experimental.first != "0" @@ -19,8 +20,24 @@ module Vagrant @_experimental end + # A method for determining if all experimental features have been enabled + # by either a global enabled value "1" or all features explicitly enabled. + # + # @return [Boolean] + def global_enabled? + if !defined?(@_global_enabled) + experimental = features_requested + if experimental.size == 1 && experimental.first == "1" || (experimental.size > 0 && experimental.sort == VALID_FEATURES.sort) + @_global_enabled = true + else + @_global_enabled = false + end + end + @_global_enabled + end + # A method for Vagrant internals to determine if a given feature - # has been abled and can be used. + # has been abled by the user, is a valid feature flag and can be used. # # @param [String] feature # @return [Boolean] - A hash containing the original array and if it is valid diff --git a/test/unit/vagrant/util/experimental_test.rb b/test/unit/vagrant/util/experimental_test.rb index 5a0f17ad4..20b197b48 100644 --- a/test/unit/vagrant/util/experimental_test.rb +++ b/test/unit/vagrant/util/experimental_test.rb @@ -7,13 +7,41 @@ describe Vagrant::Util::Experimental do before(:each) { described_class.reset! } subject { described_class } + describe "#enabled?" do + it "returns true if enabled with '1'" do + allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("1") + expect(subject.enabled?).to eq(true) + end + + it "returns true if enabled with a list of features" do + allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("list,of,features") + expect(subject.enabled?).to eq(true) + end + + it "returns false if disabled" do + allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("0") + expect(subject.enabled?).to eq(false) + end + + it "returns false if not set" do + allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return(nil) + expect(subject.enabled?).to eq(false) + end + end + describe "#global_enabled?" do it "returns true if enabled with '1'" do allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("1") expect(subject.global_enabled?).to eq(true) end - it "returns true if enabled with a list of features" do + it "returns false if enabled with a partial list of features" do + allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("list,of,features") + expect(subject.global_enabled?).to eq(false) + end + + it "returns true if enabled with a complete list of features" do + stub_const("Vagrant::Util::Experimental::VALID_FEATURES", ["list","of","features"]) allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("list,of,features") expect(subject.global_enabled?).to eq(true) end @@ -54,9 +82,9 @@ describe Vagrant::Util::Experimental do expect(subject.feature_enabled?("secret_feature")).to eq(true) end - it "returns false if flag does not contain feature requested" do - allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("secret_feature") - expect(subject.feature_enabled?("anything")).to eq(false) + it "returns false if flag is set but does not contain feature requested" do + allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("fake_feature") + expect(subject.feature_enabled?("secret_feature")).to eq(false) end it "returns false if flag set to 0" do From accabdd7cae30fcf18ce5e06b23b3ea2ebe80f64 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 7 Dec 2018 13:31:18 -0800 Subject: [PATCH 09/10] Warn users about unknown requested experimental features --- bin/vagrant | 7 +++++++ lib/vagrant/util/experimental.rb | 19 ++++++++++++------- templates/locales/en.yml | 4 ++++ test/unit/vagrant/util/experimental_test.rb | 16 ++++++++++++++++ 4 files changed, 39 insertions(+), 7 deletions(-) diff --git a/bin/vagrant b/bin/vagrant index efe5eab63..5436ad40b 100755 --- a/bin/vagrant +++ b/bin/vagrant @@ -172,6 +172,13 @@ begin logger.debug("Experimental flag is enabled") if Vagrant::Util::Experimental.global_enabled? ui.warn(I18n.t("vagrant.general.experimental.all"), bold: true, prefix: true, channel: :error) + else + ui.warn(I18n.t("vagrant.general.experimental.features", features: experimental.join(", ")), bold: true, prefix: true, channel: :error) + + diff = Vagrant::Util::Experimental.features_valid? + if !diff.empty? + ui.error(I18n.t("vagrant.general.experimental.unknown_features", unknown: diff.join(", ")), bold: true, prefix: true, channel: :error) + end end end diff --git a/lib/vagrant/util/experimental.rb b/lib/vagrant/util/experimental.rb index 9a2c6fb48..49490ec41 100644 --- a/lib/vagrant/util/experimental.rb +++ b/lib/vagrant/util/experimental.rb @@ -45,14 +45,19 @@ module Vagrant experimental = features_requested feature = feature.to_s - if experimental.size == 1 && experimental.first == "1" - return true - elsif VALID_FEATURES.include?(feature) && - experimental.include?(feature) - return true - else - return false + return global_enabled? || (VALID_FEATURES.include?(feature) && experimental.include?(feature)) + end + + # Determines if there are any unrecognized requested features + # + # @return [Array] + def features_valid? + if !defined?(@_features_diff) + @_features_diff = [] + features = features_requested + features.each { |f| @_features_diff << f if !VALID_FEATURES.include?(f) } end + @_features_diff end # Returns the features requested for the experimental flag diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 3890182b6..9b2e8502d 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -405,6 +405,10 @@ en: Please use with caution, as some of the features may not be fully functional yet. + unknown_features: |- + Requested unknown feature flags will be ignored: + + Unknown: %{unknown} not_in_installer: |- You appear to be running Vagrant outside of the official installers. Note that the installers are what ensure that Vagrant has all required diff --git a/test/unit/vagrant/util/experimental_test.rb b/test/unit/vagrant/util/experimental_test.rb index 20b197b48..aa5e11978 100644 --- a/test/unit/vagrant/util/experimental_test.rb +++ b/test/unit/vagrant/util/experimental_test.rb @@ -98,6 +98,22 @@ describe Vagrant::Util::Experimental do end end + describe "#features_valid" do + before(:each) do + stub_const("Vagrant::Util::Experimental::VALID_FEATURES", ["cool_feature", "other_feature", "secret_feature"]) + end + + it "returns an empty array if no diffs found" do + allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("cool_feature,other_feature") + expect(subject.features_valid?).to eq([]) + end + + it "returns the invalid flag if found" do + allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("fake_feature") + expect(subject.features_valid?).to eq(["fake_feature"]) + end + end + describe "#features_requested" do it "returns an array of requested features" do allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("secret_feature,other_secret") From 2783b121f963eedf4ba5c96642ef3a6bbbb08125 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 7 Dec 2018 13:52:02 -0800 Subject: [PATCH 10/10] Remove VALID_FEATURES constant --- bin/vagrant | 5 ---- lib/vagrant/util/experimental.rb | 18 ++----------- templates/locales/en.yml | 4 --- test/unit/vagrant/util/experimental_test.rb | 30 --------------------- 4 files changed, 2 insertions(+), 55 deletions(-) diff --git a/bin/vagrant b/bin/vagrant index 5436ad40b..0e6abdcef 100755 --- a/bin/vagrant +++ b/bin/vagrant @@ -174,11 +174,6 @@ begin ui.warn(I18n.t("vagrant.general.experimental.all"), bold: true, prefix: true, channel: :error) else ui.warn(I18n.t("vagrant.general.experimental.features", features: experimental.join(", ")), bold: true, prefix: true, channel: :error) - - diff = Vagrant::Util::Experimental.features_valid? - if !diff.empty? - ui.error(I18n.t("vagrant.general.experimental.unknown_features", unknown: diff.join(", ")), bold: true, prefix: true, channel: :error) - end end end diff --git a/lib/vagrant/util/experimental.rb b/lib/vagrant/util/experimental.rb index 49490ec41..8888a7224 100644 --- a/lib/vagrant/util/experimental.rb +++ b/lib/vagrant/util/experimental.rb @@ -1,9 +1,7 @@ module Vagrant module Util class Experimental - VALID_FEATURES = [] class << self - # A method for determining if the experimental flag has been enabled with # any features # @@ -27,7 +25,7 @@ module Vagrant def global_enabled? if !defined?(@_global_enabled) experimental = features_requested - if experimental.size == 1 && experimental.first == "1" || (experimental.size > 0 && experimental.sort == VALID_FEATURES.sort) + if experimental.size == 1 && experimental.first == "1" @_global_enabled = true else @_global_enabled = false @@ -45,19 +43,7 @@ module Vagrant experimental = features_requested feature = feature.to_s - return global_enabled? || (VALID_FEATURES.include?(feature) && experimental.include?(feature)) - end - - # Determines if there are any unrecognized requested features - # - # @return [Array] - def features_valid? - if !defined?(@_features_diff) - @_features_diff = [] - features = features_requested - features.each { |f| @_features_diff << f if !VALID_FEATURES.include?(f) } - end - @_features_diff + return global_enabled? || experimental.include?(feature) end # Returns the features requested for the experimental flag diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 9b2e8502d..3890182b6 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -405,10 +405,6 @@ en: Please use with caution, as some of the features may not be fully functional yet. - unknown_features: |- - Requested unknown feature flags will be ignored: - - Unknown: %{unknown} not_in_installer: |- You appear to be running Vagrant outside of the official installers. Note that the installers are what ensure that Vagrant has all required diff --git a/test/unit/vagrant/util/experimental_test.rb b/test/unit/vagrant/util/experimental_test.rb index aa5e11978..d4a03c4ad 100644 --- a/test/unit/vagrant/util/experimental_test.rb +++ b/test/unit/vagrant/util/experimental_test.rb @@ -40,12 +40,6 @@ describe Vagrant::Util::Experimental do expect(subject.global_enabled?).to eq(false) end - it "returns true if enabled with a complete list of features" do - stub_const("Vagrant::Util::Experimental::VALID_FEATURES", ["list","of","features"]) - allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("list,of,features") - expect(subject.global_enabled?).to eq(true) - end - it "returns false if disabled" do allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("0") expect(subject.global_enabled?).to eq(false) @@ -58,10 +52,6 @@ describe Vagrant::Util::Experimental do end describe "#feature_enabled?" do - before(:each) do - stub_const("Vagrant::Util::Experimental::VALID_FEATURES", ["secret_feature"]) - end - it "returns true if flag set to 1" do allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("1") expect(subject.feature_enabled?("anything")).to eq(true) @@ -98,22 +88,6 @@ describe Vagrant::Util::Experimental do end end - describe "#features_valid" do - before(:each) do - stub_const("Vagrant::Util::Experimental::VALID_FEATURES", ["cool_feature", "other_feature", "secret_feature"]) - end - - it "returns an empty array if no diffs found" do - allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("cool_feature,other_feature") - expect(subject.features_valid?).to eq([]) - end - - it "returns the invalid flag if found" do - allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("fake_feature") - expect(subject.features_valid?).to eq(["fake_feature"]) - end - end - describe "#features_requested" do it "returns an array of requested features" do allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return("secret_feature,other_secret") @@ -122,10 +96,6 @@ describe Vagrant::Util::Experimental do end describe "#guard_with" do - before(:each) do - stub_const("Vagrant::Util::Experimental::VALID_FEATURES", ["secret_feature"]) - end - it "does not execute the block if the feature is not requested" do allow(ENV).to receive(:[]).with("VAGRANT_EXPERIMENTAL").and_return(nil) expect{|b| subject.guard_with("secret_feature", &b) }.not_to yield_control