Skip to content
Browse files

clean up #load_yaml_file error handling

  • Loading branch information...
1 parent 37efb91 commit 5ed9261069895c5e9570a628355f8d1972efa23f David Sabeti and Dmitriy Kalinin committed Mar 29, 2013
View
20 bosh_cli/lib/cli/core_ext.rb
@@ -67,17 +67,25 @@ def format_time(time)
def load_yaml_file(path, expected_type = Hash)
err("Cannot find file `#{path}'".red) unless File.exist?(path)
- yaml = Psych::load(ERB.new(File.read(path)).result)
- if expected_type && !yaml.is_a?(expected_type)
- err("Incorrect file format in `#{path}', #{expected_type} expected")
+ begin
+ yaml_str = ERB.new(File.read(path)).result
+ rescue SystemCallError => e
+ err("Cannot load YAML file at `#{path}': #{e}".red)
+ end
+
+ begin
+ Bosh::Cli::YamlHelper.check_duplicate_keys(yaml_str)
+ rescue => e
+ err("Incorrect YAML structure in `#{path}': #{e}".red)
end
- Bosh::Cli::YamlHelper.check_duplicate_keys(path)
+ yaml = Psych::load(yaml_str)
+ if expected_type && !yaml.is_a?(expected_type)
+ err("Incorrect YAML structure in `#{path}': expected #{expected_type} at the root".red)
+ end
yaml
- rescue SystemCallError => e
- err("Cannot load YAML file at `#{path}': #{e}".red)
end
def write_yaml(manifest, path)
View
21 bosh_cli/lib/cli/yaml_helper.rb
@@ -29,7 +29,7 @@ def process_mapping(m)
m.children.each_with_index do |key_or_value, index|
next if index.odd? # skip the values
key = key_or_value.value # Sorry this is confusing, Psych mappings don't behave nicely like maps
- raise "Found duplicate key #{key}" if s.include?(key)
+ raise "Found duplicate key '#{key}'" if s.include?(key)
s.add(key)
end
@@ -40,20 +40,11 @@ def process_mapping(m)
end
public
- def check_duplicate_keys(path)
- begin
- string_with_erb = File.read(path)
- yaml = ERB.new(string_with_erb).result
- document = Psych.parse(yaml)
- process_mapping(document.root) if document
- rescue => e
- if e.message.include? "Found duplicate key"
- raise "Bad YAML file when parsing file: #{path}\n#{e.message}"
- else
- raise e
- end
- end
+
+ def check_duplicate_keys(yaml_str)
+ document = Psych.parse(yaml_str)
+ process_mapping(document.root) if document
end
end
end
-end
+end
View
3 bosh_cli/spec/unit/blob_manager_spec.rb
@@ -49,9 +49,10 @@ def make_manager(release)
File.open(File.join(@config_dir, "blobs.yml"), "w") do |f|
Psych.dump("string", f)
end
+
expect {
make_manager(@release)
- }.to raise_error(/Incorrect file format/)
+ }.to raise_error(/Incorrect YAML structure/)
end
it "migrates legacy index file" do
View
14 bosh_cli/spec/unit/core_ext_spec.rb
@@ -84,21 +84,27 @@
o.should_not be_blank
end
- describe "loading YAML files" do
+ describe "#load_yaml_file" do
it "can load YAML files with ERB" do
load_yaml_file(spec_asset("dummy.yml.erb")).should == {"four" => 4}
end
it "gives a nice error when the file cannot be found" do
- expect { load_yaml_file("non-existent.yml") }.to raise_error(Bosh::Cli::CliError, "Cannot find file `non-existent.yml'")
+ expect {
+ load_yaml_file("non-existent.yml")
+ }.to raise_error(Bosh::Cli::CliError, "Cannot find file `non-existent.yml'")
end
it "gives a nice error when the parsed YAML is not a Hash" do
- expect { load_yaml_file(spec_asset("not_a_hash.yml")) }.to raise_error(Bosh::Cli::CliError, /Incorrect file format in/)
+ expect {
+ load_yaml_file(spec_asset("not_a_hash.yml"))
+ }.to raise_error(Bosh::Cli::CliError, /Incorrect YAML structure .* expected Hash/)
end
it "gives a nice error when the parsed YAML produces a hash with repeated keys" do
- expect { load_yaml_file(spec_asset("duplicate_keys.yml")) }.to raise_error(RuntimeError, /Bad YAML file/)
+ expect {
+ load_yaml_file(spec_asset("duplicate_keys.yml"))
+ }.to raise_error(Bosh::Cli::CliError, /Incorrect YAML structure .* duplicate key 'unique_key'/)
end
end
end

0 comments on commit 5ed9261

Please sign in to comment.
Something went wrong with that request. Please try again.