From 01a943d0d6274a8666eae4272f23923de60868f1 Mon Sep 17 00:00:00 2001 From: Stuart Frost Date: Mon, 24 Jul 2023 15:30:38 +0100 Subject: [PATCH 1/5] Raise error when passed invalid config file path --- spec/ameba/cli/cmd_spec.cr | 10 ++++++++-- src/ameba/cli/cmd.cr | 2 ++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/spec/ameba/cli/cmd_spec.cr b/spec/ameba/cli/cmd_spec.cr index a412c1941..624d1ad9a 100644 --- a/spec/ameba/cli/cmd_spec.cr +++ b/spec/ameba/cli/cmd_spec.cr @@ -20,8 +20,14 @@ module Ameba::Cli %w(-c --config).each do |f| it "accepts #{f} flag" do - c = Cli.parse_args [f, "config.yml"] - c.config.should eq Path["config.yml"] + c = Cli.parse_args [f, "shard.yml"] + c.config.should eq Path["shard.yml"] + end + + it "raises when config file doesn't exist" do + expect_raises(ArgumentError, "Unable to find config file foo.yml") do + Cli.parse_args [f, "foo.yml"] + end end end diff --git a/src/ameba/cli/cmd.cr b/src/ameba/cli/cmd.cr index d8ce4a7e8..3b06b9059 100644 --- a/src/ameba/cli/cmd.cr +++ b/src/ameba/cli/cmd.cr @@ -77,6 +77,8 @@ module Ameba::Cli parser.on("-c", "--config PATH", "Specify a configuration file") do |path| + raise ArgumentError.new("Unable to find config file #{path}") if !File.exists?(path) && !opts.skip_reading_config? + opts.config = Path[path] unless path.empty? end From 5f878fb40fed4c8e0b7284c640b0bf1003f0805a Mon Sep 17 00:00:00 2001 From: Stuart Frost Date: Mon, 24 Jul 2023 19:06:33 +0100 Subject: [PATCH 2/5] Move missing config file check into Ameba::Config --- spec/ameba/cli/cmd_spec.cr | 10 ++-------- spec/ameba/config_spec.cr | 8 +++++++- spec/ameba_fixture.yml | 2 ++ src/ameba/cli/cmd.cr | 2 -- src/ameba/config.cr | 2 +- 5 files changed, 12 insertions(+), 12 deletions(-) create mode 100644 spec/ameba_fixture.yml diff --git a/spec/ameba/cli/cmd_spec.cr b/spec/ameba/cli/cmd_spec.cr index 624d1ad9a..a412c1941 100644 --- a/spec/ameba/cli/cmd_spec.cr +++ b/spec/ameba/cli/cmd_spec.cr @@ -20,14 +20,8 @@ module Ameba::Cli %w(-c --config).each do |f| it "accepts #{f} flag" do - c = Cli.parse_args [f, "shard.yml"] - c.config.should eq Path["shard.yml"] - end - - it "raises when config file doesn't exist" do - expect_raises(ArgumentError, "Unable to find config file foo.yml") do - Cli.parse_args [f, "foo.yml"] - end + c = Cli.parse_args [f, "config.yml"] + c.config.should eq Path["config.yml"] end end diff --git a/spec/ameba/config_spec.cr b/spec/ameba/config_spec.cr index d738310f5..6287de05b 100644 --- a/spec/ameba/config_spec.cr +++ b/spec/ameba/config_spec.cr @@ -2,7 +2,7 @@ require "../spec_helper" module Ameba describe Config do - config_sample = "config/ameba.yml" + config_sample = "spec/ameba_fixture.yml" it "should have a list of available formatters" do Config::AVAILABLE_FORMATTERS.should_not be_nil @@ -84,6 +84,12 @@ module Ameba config.formatter.should_not be_nil end + it "raises when custom config file doesn't exist" do + expect_raises(Exception, "Config file is invalid: Unable to find config file foo.yml") do + Config.load "foo.yml" + end + end + it "loads default config" do config = Config.load config.should_not be_nil diff --git a/spec/ameba_fixture.yml b/spec/ameba_fixture.yml new file mode 100644 index 000000000..c5394b90f --- /dev/null +++ b/spec/ameba_fixture.yml @@ -0,0 +1,2 @@ +Lint/ComparisonToBoolean: + Enabled: true diff --git a/src/ameba/cli/cmd.cr b/src/ameba/cli/cmd.cr index 3b06b9059..d8ce4a7e8 100644 --- a/src/ameba/cli/cmd.cr +++ b/src/ameba/cli/cmd.cr @@ -77,8 +77,6 @@ module Ameba::Cli parser.on("-c", "--config PATH", "Specify a configuration file") do |path| - raise ArgumentError.new("Unable to find config file #{path}") if !File.exists?(path) && !opts.skip_reading_config? - opts.config = Path[path] unless path.empty? end diff --git a/src/ameba/config.cr b/src/ameba/config.cr index 3d3a3c71a..ad4f5dc60 100644 --- a/src/ameba/config.cr +++ b/src/ameba/config.cr @@ -120,7 +120,7 @@ class Ameba::Config protected def self.read_config(path = nil) if path - return File.exists?(path) ? File.read(path) : nil + return File.exists?(path) ? File.read(path) : raise("Unable to find config file #{path}") end each_config_path do |config_path| return File.read(config_path) if File.exists?(config_path) From d9b2d690557b00a311c6dea8f3ec757f1cebaff5 Mon Sep 17 00:00:00 2001 From: Stuart Frost Date: Tue, 25 Jul 2023 08:43:08 +0100 Subject: [PATCH 3/5] Reword error when file doesn't exist Applied suggestion from PR Co-authored-by: Vitalii Elenhaupt <3624712+veelenga@users.noreply.github.com> --- spec/ameba/config_spec.cr | 2 +- src/ameba/config.cr | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/spec/ameba/config_spec.cr b/spec/ameba/config_spec.cr index 6287de05b..7b08f64cc 100644 --- a/spec/ameba/config_spec.cr +++ b/spec/ameba/config_spec.cr @@ -85,7 +85,7 @@ module Ameba end it "raises when custom config file doesn't exist" do - expect_raises(Exception, "Config file is invalid: Unable to find config file foo.yml") do + expect_raises(Exception, "Config file is invalid: Config file does not exist foo.yml") do Config.load "foo.yml" end end diff --git a/src/ameba/config.cr b/src/ameba/config.cr index ad4f5dc60..2e5019de7 100644 --- a/src/ameba/config.cr +++ b/src/ameba/config.cr @@ -120,7 +120,8 @@ class Ameba::Config protected def self.read_config(path = nil) if path - return File.exists?(path) ? File.read(path) : raise("Unable to find config file #{path}") + raise ArgumentError.new("Config file does not exist #{path}") unless File.exists?(path) + return File.read(path) end each_config_path do |config_path| return File.read(config_path) if File.exists?(config_path) From 4741c9f4c4fef879e875f3415f64e0e66103cef8 Mon Sep 17 00:00:00 2001 From: Stuart Frost Date: Tue, 25 Jul 2023 08:46:13 +0100 Subject: [PATCH 4/5] Reword generic error message on config load --- spec/ameba/config_spec.cr | 2 +- src/ameba/config.cr | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/ameba/config_spec.cr b/spec/ameba/config_spec.cr index 7b08f64cc..20c6e95f9 100644 --- a/spec/ameba/config_spec.cr +++ b/spec/ameba/config_spec.cr @@ -85,7 +85,7 @@ module Ameba end it "raises when custom config file doesn't exist" do - expect_raises(Exception, "Config file is invalid: Config file does not exist foo.yml") do + expect_raises(Exception, "Unable to load config file: Config file does not exist foo.yml") do Config.load "foo.yml" end end diff --git a/src/ameba/config.cr b/src/ameba/config.cr index 2e5019de7..821fa0655 100644 --- a/src/ameba/config.cr +++ b/src/ameba/config.cr @@ -115,7 +115,7 @@ class Ameba::Config end Config.new YAML.parse(content) rescue e - raise "Config file is invalid: #{e.message}" + raise "Unable to load config file: #{e.message}" end protected def self.read_config(path = nil) From 88e04379024812beff793d03dcc2e622b79f7cc5 Mon Sep 17 00:00:00 2001 From: Stuart Frost Date: Tue, 25 Jul 2023 10:00:16 +0100 Subject: [PATCH 5/5] Move fixture file to spec/fixtures directory --- spec/ameba/config_spec.cr | 2 +- spec/{ameba_fixture.yml => fixtures/config.yml} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename spec/{ameba_fixture.yml => fixtures/config.yml} (100%) diff --git a/spec/ameba/config_spec.cr b/spec/ameba/config_spec.cr index 20c6e95f9..1c1d10b21 100644 --- a/spec/ameba/config_spec.cr +++ b/spec/ameba/config_spec.cr @@ -2,7 +2,7 @@ require "../spec_helper" module Ameba describe Config do - config_sample = "spec/ameba_fixture.yml" + config_sample = "spec/fixtures/config.yml" it "should have a list of available formatters" do Config::AVAILABLE_FORMATTERS.should_not be_nil diff --git a/spec/ameba_fixture.yml b/spec/fixtures/config.yml similarity index 100% rename from spec/ameba_fixture.yml rename to spec/fixtures/config.yml