Improvements suggested in the PR review

- Changed the error message for invalid flag names and values to be more specific.
- Changed to do stronger test assertions for error message validations.

Author: hsinn0

app/models/runtime/feature_flag.rb

@@ -87,8 +87,11 @@ def self.admin_read_only?
     end
 
     def self.override_default_flags(feature_flag_overrides)
-      raise "Invalid feature flag name(s): #{feature_flag_overrides}" unless feature_flag_overrides.keys.to_set <= DEFAULT_FLAGS.keys.to_set
-      raise "Invalid feature flag value(s): #{feature_flag_overrides}" unless feature_flag_overrides.values.all? { |item| item.is_a?(TrueClass) || item.is_a?(FalseClass) }
+      invalid_keys = feature_flag_overrides.keys.to_set - FeatureFlag::DEFAULT_FLAGS.keys.to_set
+      raise "Invalid feature flag name(s): #{invalid_keys.to_a}" if invalid_keys.any?
+
+      invalid_values = feature_flag_overrides.select { |k,v| !v.in? [true,false] }
+      raise "Invalid feature flag value(s): #{invalid_values}" if invalid_values.any?
 
       feature_flag_overrides.each do |flag_name, flag_value|
         feature_flag = FeatureFlag.find(name: flag_name.to_s)

spec/unit/models/runtime/feature_flag_spec.rb

@@ -291,27 +291,31 @@ module VCAP::CloudController
     describe '.override_default_flags' do
       context 'with invalid flags' do
         it 'raises an error for the one and only invalid name' do
+          feature_flag_overrides = { an_invalid_name: true }
           expect do
-            FeatureFlag.override_default_flags({ an_invalid_name: true })
-          end.to raise_error(/Invalid feature flag name\(s\)/)
+            FeatureFlag.override_default_flags(feature_flag_overrides)
+          end.to raise_error("Invalid feature flag name(s): [:an_invalid_name]")
         end
 
         it 'raises an error for a mix of valid and invalid names' do
+          feature_flag_overrides = { diego_docker: true, an_invalid_name: true }
           expect do
-            FeatureFlag.override_default_flags({ diego_docker: true, an_invalid_name: true })
-          end.to raise_error(/Invalid feature flag name\(s\)/)
+            FeatureFlag.override_default_flags(feature_flag_overrides)
+          end.to raise_error("Invalid feature flag name(s): [:an_invalid_name]")
         end
 
         it 'raises an error for all invalid names' do
+          feature_flag_overrides = { invalid_name1: true, invalid_name2: false }
           expect do
-            FeatureFlag.override_default_flags({ invalid_name1: true, invalid_name2: false })
-          end.to raise_error(/Invalid feature flag name\(s\)/)
+            FeatureFlag.override_default_flags(feature_flag_overrides)
+          end.to raise_error("Invalid feature flag name(s): [:invalid_name1, :invalid_name2]")
         end
 
         it 'raises an error for invalid values' do
+          feature_flag_overrides = { diego_docker: 'an invalid value', user_org_creation: false }
           expect do
-            FeatureFlag.override_default_flags({ diego_docker: 'an invalid value', user_org_creation: false })
-          end.to raise_error(/Invalid feature flag value\(s\)/)
+            FeatureFlag.override_default_flags(feature_flag_overrides)
+          end.to raise_error("Invalid feature flag value(s): {:diego_docker=>\"an invalid value\"}")
         end
       end