From ee14069fb9f09e9bead323bf58b33696b5fec06b Mon Sep 17 00:00:00 2001 From: BogdanIrimie Date: Fri, 5 Jun 2020 11:47:30 +0300 Subject: [PATCH 1/5] (FACT-2651) Reorder arguments based on priority. --- lib/framework/cli/cli_launcher.rb | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/lib/framework/cli/cli_launcher.rb b/lib/framework/cli/cli_launcher.rb index 1a8fdc1d3..cc39c681a 100755 --- a/lib/framework/cli/cli_launcher.rb +++ b/lib/framework/cli/cli_launcher.rb @@ -7,10 +7,38 @@ require "#{ROOT_DIR}/lib/facter" require "#{ROOT_DIR}/lib/framework/cli/cli" +def check(known_arguments, program_arguments) + program_arguments.each do |argument| + return true if known_arguments.key?(argument) + end + + return false +end + +def reorder!(program_arguments) + priority_arguments = Facter::Cli.instance_variable_get(:@map) + + priority_args = [] + normal_args = [] + + program_arguments.each do |argument| + if priority_arguments.include?(argument) + priority_args << (argument) + else + normal_args << argument + end + end + + priority_args.concat(normal_args) +end + + Facter::OptionsValidator.validate(ARGV) ARGV.unshift(Facter::Cli.default_task) unless Facter::Cli.all_tasks.key?(ARGV[0]) || - Facter::Cli.instance_variable_get(:@map).key?(ARGV[0]) + check(Facter::Cli.instance_variable_get(:@map), ARGV) + +ARGV = reorder!(ARGV) begin Facter::Cli.start(ARGV, debug: true) From a89d8683a21b43ba99032e142b97cc1776977e6d Mon Sep 17 00:00:00 2001 From: BogdanIrimie Date: Mon, 8 Jun 2020 15:27:32 +0300 Subject: [PATCH 2/5] (FACT-2651) Rubocop fixes. --- lib/framework/cli/cli_launcher.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/framework/cli/cli_launcher.rb b/lib/framework/cli/cli_launcher.rb index cc39c681a..a6b448a78 100755 --- a/lib/framework/cli/cli_launcher.rb +++ b/lib/framework/cli/cli_launcher.rb @@ -12,7 +12,7 @@ def check(known_arguments, program_arguments) return true if known_arguments.key?(argument) end - return false + false end def reorder!(program_arguments) @@ -23,7 +23,7 @@ def reorder!(program_arguments) program_arguments.each do |argument| if priority_arguments.include?(argument) - priority_args << (argument) + priority_args << argument else normal_args << argument end @@ -32,7 +32,6 @@ def reorder!(program_arguments) priority_args.concat(normal_args) end - Facter::OptionsValidator.validate(ARGV) ARGV.unshift(Facter::Cli.default_task) unless Facter::Cli.all_tasks.key?(ARGV[0]) || From 0f7c1473a2b715d0228ebbc33eab18c2a83f7f6e Mon Sep 17 00:00:00 2001 From: BogdanIrimie Date: Mon, 8 Jun 2020 15:39:04 +0300 Subject: [PATCH 3/5] (FACT-2651) Reorganise cli launcher in OO manner. --- lib/framework/cli/cli_launcher.rb | 69 ++++++++++++++++++++----------- 1 file changed, 44 insertions(+), 25 deletions(-) diff --git a/lib/framework/cli/cli_launcher.rb b/lib/framework/cli/cli_launcher.rb index a6b448a78..75d83533b 100755 --- a/lib/framework/cli/cli_launcher.rb +++ b/lib/framework/cli/cli_launcher.rb @@ -7,40 +7,59 @@ require "#{ROOT_DIR}/lib/facter" require "#{ROOT_DIR}/lib/framework/cli/cli" -def check(known_arguments, program_arguments) - program_arguments.each do |argument| - return true if known_arguments.key?(argument) +class CliLauncher + def initialize(args) + @args = args end - false -end + def validate_options + Facter::OptionsValidator.validate(@args) + end + + def prepare_arguments + @args.unshift(Facter::Cli.default_task) unless + Facter::Cli.all_tasks.key?(@args[0]) || + check_if_arguments_is_known(Facter::Cli.instance_variable_get(:@map), @args) -def reorder!(program_arguments) - priority_arguments = Facter::Cli.instance_variable_get(:@map) + @args = reorder_program_arguments(@args) + end + + def start + Facter::Cli.start(@args, debug: true) + rescue Thor::UnknownArgumentError => e + Facter::OptionsValidator.write_error_and_exit("unrecognised option '#{e.unknown.first}'") + end - priority_args = [] - normal_args = [] + private - program_arguments.each do |argument| - if priority_arguments.include?(argument) - priority_args << argument - else - normal_args << argument + def check_if_arguments_is_known(known_arguments, program_arguments) + program_arguments.each do |argument| + return true if known_arguments.key?(argument) end + + false end - priority_args.concat(normal_args) -end + def reorder_program_arguments(program_arguments) + priority_arguments = Facter::Cli.instance_variable_get(:@map) -Facter::OptionsValidator.validate(ARGV) -ARGV.unshift(Facter::Cli.default_task) unless - Facter::Cli.all_tasks.key?(ARGV[0]) || - check(Facter::Cli.instance_variable_get(:@map), ARGV) + priority_args = [] + normal_args = [] -ARGV = reorder!(ARGV) + program_arguments.each do |argument| + if priority_arguments.include?(argument) + priority_args << argument + else + normal_args << argument + end + end -begin - Facter::Cli.start(ARGV, debug: true) -rescue Thor::UnknownArgumentError => e - Facter::OptionsValidator.write_error_and_exit("unrecognised option '#{e.unknown.first}'") + priority_args.concat(normal_args) + end end + +cli_launcher = CliLauncher.new(ARGV) + +cli_launcher.validate_options +cli_launcher.prepare_arguments +cli_launcher.start From 853be9167a6e6f30d36ed31813f8fad60ffb5b49 Mon Sep 17 00:00:00 2001 From: BogdanIrimie Date: Mon, 8 Jun 2020 15:45:07 +0300 Subject: [PATCH 4/5] (FACT-2651) Update bundler. --- .github/workflows/unit_tests.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/unit_tests.yaml b/.github/workflows/unit_tests.yaml index dfb56492d..63245933e 100644 --- a/.github/workflows/unit_tests.yaml +++ b/.github/workflows/unit_tests.yaml @@ -24,6 +24,7 @@ jobs: uses: ruby/setup-ruby@v1 with: ruby-version: ${{ matrix.ruby }} + - run: gem update bundler - run: bundle install --jobs 3 --retry 3 - run: bundle exec rake spec @@ -37,5 +38,6 @@ jobs: uses: ruby/setup-ruby@v1 with: ruby-version: 2.7 + - run: gem update bundler - run: bundle install --jobs 3 --retry 3 - run: bundle exec rake spec From 3367d85b69e96b08b2fab075b2b47081e9a625a9 Mon Sep 17 00:00:00 2001 From: BogdanIrimie Date: Mon, 8 Jun 2020 17:30:42 +0300 Subject: [PATCH 5/5] (FACT-2651) Add unit test for cli_launcher. --- bin/facter | 6 ++ bin/facter-ng | 6 ++ lib/framework/cli/cli_launcher.rb | 8 +-- spec/framework/cli/cli_launcher_spec.rb | 96 +++++++++++++++++++++++++ 4 files changed, 109 insertions(+), 7 deletions(-) create mode 100644 spec/framework/cli/cli_launcher_spec.rb diff --git a/bin/facter b/bin/facter index ccdd74672..477d6cf29 100755 --- a/bin/facter +++ b/bin/facter @@ -4,3 +4,9 @@ require 'pathname' ROOT_DIR = Pathname.new(File.expand_path('..', __dir__)) unless defined?(ROOT_DIR) require "#{ROOT_DIR}/lib/framework/cli/cli_launcher.rb" + +cli_launcher = CliLauncher.new(ARGV) + +cli_launcher.validate_options +cli_launcher.prepare_arguments +cli_launcher.start diff --git a/bin/facter-ng b/bin/facter-ng index ccdd74672..477d6cf29 100755 --- a/bin/facter-ng +++ b/bin/facter-ng @@ -4,3 +4,9 @@ require 'pathname' ROOT_DIR = Pathname.new(File.expand_path('..', __dir__)) unless defined?(ROOT_DIR) require "#{ROOT_DIR}/lib/framework/cli/cli_launcher.rb" + +cli_launcher = CliLauncher.new(ARGV) + +cli_launcher.validate_options +cli_launcher.prepare_arguments +cli_launcher.start diff --git a/lib/framework/cli/cli_launcher.rb b/lib/framework/cli/cli_launcher.rb index 75d83533b..1bf3f03d2 100755 --- a/lib/framework/cli/cli_launcher.rb +++ b/lib/framework/cli/cli_launcher.rb @@ -18,7 +18,7 @@ def validate_options def prepare_arguments @args.unshift(Facter::Cli.default_task) unless - Facter::Cli.all_tasks.key?(@args[0]) || + check_if_arguments_is_known(Facter::Cli.all_tasks, @args) || check_if_arguments_is_known(Facter::Cli.instance_variable_get(:@map), @args) @args = reorder_program_arguments(@args) @@ -57,9 +57,3 @@ def reorder_program_arguments(program_arguments) priority_args.concat(normal_args) end end - -cli_launcher = CliLauncher.new(ARGV) - -cli_launcher.validate_options -cli_launcher.prepare_arguments -cli_launcher.start diff --git a/spec/framework/cli/cli_launcher_spec.rb b/spec/framework/cli/cli_launcher_spec.rb new file mode 100644 index 000000000..d677e06d8 --- /dev/null +++ b/spec/framework/cli/cli_launcher_spec.rb @@ -0,0 +1,96 @@ +# frozen_string_literal: true + +require "#{ROOT_DIR}/lib/framework/cli/cli_launcher" + +describe CliLauncher do + subject(:cli_launcher) { CliLauncher.new(args) } + + let(:args) { [] } + + describe '#validate_options' do + it 'calls Facter::OptionsValidator.validate' do + allow(Facter::OptionsValidator).to receive(:validate) + cli_launcher.validate_options + + expect(Facter::OptionsValidator).to have_received(:validate).with(args) + end + end + + describe '#prepare_arguments' do + let(:task_list) do + { 'help' => Thor::Command.new('help', 'description', 'long_description', 'usage'), + 'query' => Thor::Command.new('query', 'description', 'long_description', 'usage'), + 'version' => Thor::Command.new('version', 'description', 'long_description', 'usage'), + 'list_block_groups' => Thor::Command.new('list_block_groups', 'description', 'long_description', 'usage'), + 'list_cache_groups' => Thor::Command.new('list_cache_groups', 'description', 'long_description', 'usage') } + end + + let(:map) do + { '-h' => :help, '--version' => :version, '--list-block-groups' => :list_block_groups, + '--list-cache-groups' => :list_cache_groups } + end + + before do + allow(Facter::Cli).to receive(:all_tasks).and_return(task_list) + allow(Facter::Cli).to receive(:instance_variable_get).with(:@map).and_return(map) + end + + context 'when arguments should be reordered' do + let(:args) { %w[--debug --list-cache-groups --list-block-groups] } + let(:expected_arguments) { %w[--list-cache-groups --list-block-groups --debug] } + + it 'reorders arguments' do + prepare_arguments = cli_launcher.prepare_arguments + + expect(prepare_arguments).to eq(expected_arguments) + end + end + + context 'when arguments should not be reordered' do + let(:args) { %w[--list-cache-groups --list-block-groups --debug] } + + it 'does not reorder arguments' do + prepare_arguments = cli_launcher.prepare_arguments + + expect(prepare_arguments).to eq(args) + end + end + + context 'when default task should be added' do + let(:args) { %w[fact1 fact2] } + let(:expected_args) { %w[query fact1 fact2] } + + it 'adds default (query) task' do + prepare_arguments = cli_launcher.prepare_arguments + expect(prepare_arguments).to eq(expected_args) + end + end + end + + describe '#start' do + context 'when no errors' do + before do + allow(Facter::Cli).to receive(:start) + end + + it 'calls Facter::Cli.start' do + cli_launcher.start + + expect(Facter::Cli).to have_received(:start).with(args, debug: true) + end + end + + context 'when errors' do + before do + allow(Facter::OptionsValidator).to receive(:write_error_and_exit) + allow(Facter::Cli).to receive(:start).with(any_args).and_raise(Thor::UnknownArgumentError.new({}, {})) + end + + it 'calls Facter::OptionsValidator.write_error_and_exit' do + cli_launcher.start + + expect(Facter::OptionsValidator).to have_received(:write_error_and_exit) + end + end + end +end