From 25ec49bfa4ea26456eb4e0de204c862ae6c19adc Mon Sep 17 00:00:00 2001 From: Will Stephenson Date: Mon, 26 May 2014 09:27:30 +0200 Subject: [PATCH 1/7] Move global declaration to one place where it will be required, and ensure that is required during tests --- features/step_definition/helper_steps.rb | 2 ++ lib/suse/connect/cli.rb | 2 -- lib/suse/connect/version.rb | 2 ++ 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/features/step_definition/helper_steps.rb b/features/step_definition/helper_steps.rb index 13153df5..152219b1 100644 --- a/features/step_definition/helper_steps.rb +++ b/features/step_definition/helper_steps.rb @@ -1,3 +1,5 @@ +require 'suse/connect/version' + def service_name base_product = SUSE::Connect::Zypper.base_product if base_product[:name] == 'openSUSE' diff --git a/lib/suse/connect/cli.rb b/lib/suse/connect/cli.rb index 19a1a9ee..77fd02ac 100644 --- a/lib/suse/connect/cli.rb +++ b/lib/suse/connect/cli.rb @@ -1,8 +1,6 @@ require 'optparse' require 'suse/connect' -$suse_connect_filesystem_root = '' - module SUSE module Connect # Command line interface for interacting with SUSEConnect diff --git a/lib/suse/connect/version.rb b/lib/suse/connect/version.rb index c48ac110..49890fb1 100644 --- a/lib/suse/connect/version.rb +++ b/lib/suse/connect/version.rb @@ -1,3 +1,5 @@ +$suse_connect_filesystem_root = '' + module SUSE # Provides access to version number of a gem module Connect From c82a07e9a2adca95d68b65a786aee5719d20c11e Mon Sep 17 00:00:00 2001 From: Thomas Schmidt Date: Mon, 26 May 2014 13:52:49 +0200 Subject: [PATCH 2/7] using a class instance variable for the root parameter SUSE::Connect::System.filesystem_root --- features/step_definition/helper_steps.rb | 2 -- lib/suse/connect/cli.rb | 3 ++- lib/suse/connect/config.rb | 1 + lib/suse/connect/credentials.rb | 4 ++-- lib/suse/connect/system.rb | 12 ++++++---- lib/suse/connect/version.rb | 2 -- lib/suse/connect/zypper.rb | 2 +- rubocop.yml | 2 +- spec/connect/cli_spec.rb | 4 ++-- spec/connect/credentials_spec.rb | 10 ++++----- spec/connect/zyppper_spec.rb | 28 ++++++++++++------------ 11 files changed, 36 insertions(+), 34 deletions(-) diff --git a/features/step_definition/helper_steps.rb b/features/step_definition/helper_steps.rb index 152219b1..13153df5 100644 --- a/features/step_definition/helper_steps.rb +++ b/features/step_definition/helper_steps.rb @@ -1,5 +1,3 @@ -require 'suse/connect/version' - def service_name base_product = SUSE::Connect::Zypper.base_product if base_product[:name] == 'openSUSE' diff --git a/lib/suse/connect/cli.rb b/lib/suse/connect/cli.rb index 77fd02ac..cc343111 100644 --- a/lib/suse/connect/cli.rb +++ b/lib/suse/connect/cli.rb @@ -88,7 +88,8 @@ def extract_options # rubocop:disable MethodLength @opts.on('--root [PATH]', 'Path to the root folder, uses the same parameter for zypper.') do |opt| check_if_param(opt, 'Please provide path parameter') - $suse_connect_filesystem_root = opt + @options[:filesystem_root] = opt + SUSE::Connect::System.filesystem_root = opt end @opts.on('--version', 'print program version') do diff --git a/lib/suse/connect/config.rb b/lib/suse/connect/config.rb index 332404a6..fcd59ee3 100644 --- a/lib/suse/connect/config.rb +++ b/lib/suse/connect/config.rb @@ -5,6 +5,7 @@ module Connect # Class for handling SUSEConnect configuration class Config + DEFAULT_CONFIG_FILE = '/etc/SUSEConnect' class << self diff --git a/lib/suse/connect/credentials.rb b/lib/suse/connect/credentials.rb index 4c938482..79f2ebd1 100644 --- a/lib/suse/connect/credentials.rb +++ b/lib/suse/connect/credentials.rb @@ -24,7 +24,7 @@ def initialize(user, password, file = nil) end def self.system_credentials_file - File.join($suse_connect_filesystem_root, GLOBAL_CREDENTIALS_FILE) + File.join(SUSE::Connect::System.filesystem_root, GLOBAL_CREDENTIALS_FILE) end def self.read(file) @@ -38,7 +38,7 @@ def self.read(file) end def filename - default_dir = File.join($suse_connect_filesystem_root, DEFAULT_CREDENTIALS_DIR) + default_dir = File.join(SUSE::Connect::System.filesystem_root, DEFAULT_CREDENTIALS_DIR) Pathname.new(file).absolute? ? file : File.join(default_dir, file) end diff --git a/lib/suse/connect/system.rb b/lib/suse/connect/system.rb index a0c9d6f3..ea7b9459 100644 --- a/lib/suse/connect/system.rb +++ b/lib/suse/connect/system.rb @@ -3,14 +3,18 @@ module Connect # System class allowing to interact with underlying system class System + @filesystem_root = '' + class << self + attr_accessor :filesystem_root + def hwinfo info = { - :cpu_type => `uname -p`, - :cpu_count => `grep "processor" /proc/cpuinfo | wc -l`, - :platform_type => `uname -i`, - :hostname => `hostname` + :cpu_type => `uname -p`, + :cpu_count => `grep "processor" /proc/cpuinfo | wc -l`, + :platform_type => `uname -i`, + :hostname => `hostname` } info.values.each(&:chomp!) diff --git a/lib/suse/connect/version.rb b/lib/suse/connect/version.rb index 49890fb1..c48ac110 100644 --- a/lib/suse/connect/version.rb +++ b/lib/suse/connect/version.rb @@ -1,5 +1,3 @@ -$suse_connect_filesystem_root = '' - module SUSE # Provides access to version number of a gem module Connect diff --git a/lib/suse/connect/zypper.rb b/lib/suse/connect/zypper.rb index 85907b78..2c24556f 100644 --- a/lib/suse/connect/zypper.rb +++ b/lib/suse/connect/zypper.rb @@ -81,7 +81,7 @@ def write_base_credentials(login, password) private def root_arg - "--root '#{$suse_connect_filesystem_root}' " unless $suse_connect_filesystem_root.empty? + "--root '#{SUSE::Connect::System.filesystem_root}' " unless SUSE::Connect::System.filesystem_root.empty? end def call_zypper(silent, args) diff --git a/rubocop.yml b/rubocop.yml index e7cd7630..214f6495 100644 --- a/rubocop.yml +++ b/rubocop.yml @@ -39,7 +39,7 @@ ClassAndModuleChildren: Enabled: false GlobalVars: - AllowedVariables: ['$suse_connect_filesystem_root'] + AllowedVariables: [] AllCops: Exclude: diff --git a/spec/connect/cli_spec.rb b/spec/connect/cli_spec.rb index 2da34e8f..b1a7e011 100644 --- a/spec/connect/cli_spec.rb +++ b/spec/connect/cli_spec.rb @@ -115,8 +115,8 @@ it 'sets root option' do argv = %w{--root /path/to/root} subject.new(argv) - $suse_connect_filesystem_root.should eq '/path/to/root' - $suse_connect_filesystem_root = '' + SUSE::Connect::System.filesystem_root.should eq '/path/to/root' + SUSE::Connect::System.filesystem_root = '' end end diff --git a/spec/connect/credentials_spec.rb b/spec/connect/credentials_spec.rb index e2a2b80e..d50fa03e 100755 --- a/spec/connect/credentials_spec.rb +++ b/spec/connect/credentials_spec.rb @@ -12,10 +12,10 @@ end it 'with root folder set' do - $suse_connect_filesystem_root = '/path/to/root' - expected = File.join($suse_connect_filesystem_root, Credentials::GLOBAL_CREDENTIALS_FILE) + SUSE::Connect::System.filesystem_root = '/path/to/root' + expected = File.join('/path/to/root', Credentials::GLOBAL_CREDENTIALS_FILE) expect(Credentials.system_credentials_file).to eq expected - $suse_connect_filesystem_root = '' + SUSE::Connect::System.filesystem_root = '' end end @@ -64,10 +64,10 @@ end it 'compute filename to write properly --root case' do - $suse_connect_filesystem_root = '/path/to/root' + SUSE::Connect::System.filesystem_root = '/path/to/root' credentials = Credentials.new('name', '1234', 'SLES') credentials.filename.should start_with '/path/to/root/' - $suse_connect_filesystem_root = '' + SUSE::Connect::System.filesystem_root = '' end it 'raises an error when file name is not set' do diff --git a/spec/connect/zyppper_spec.rb b/spec/connect/zyppper_spec.rb index 9fc4466e..2e35cf62 100644 --- a/spec/connect/zyppper_spec.rb +++ b/spec/connect/zyppper_spec.rb @@ -16,12 +16,12 @@ let(:xml) { File.read('spec/fixtures/product_valid_sle11sp3.xml') } before do - $suse_connect_filesystem_root = '/path/to/root' + SUSE::Connect::System.filesystem_root = '/path/to/root' Object.should_receive(:'`').with(include "zypper --root '/path/to/root' ").and_return(xml) end after do - $suse_connect_filesystem_root = '' + SUSE::Connect::System.filesystem_root = '' end it 'returns valid list of products based on proper XML' do @@ -85,9 +85,9 @@ parameters = "zypper --root '/path/to/root' --quiet --non-interactive addservice " \ "-t ris http://example.com 'branding'" Object.should_receive(:system).with(parameters).and_return(true) - $suse_connect_filesystem_root = '/path/to/root' + SUSE::Connect::System.filesystem_root = '/path/to/root' subject.add_service('branding', 'http://example.com') - $suse_connect_filesystem_root = '' + SUSE::Connect::System.filesystem_root = '' end end @@ -103,9 +103,9 @@ it 'calls zypper with proper arguments --root case' do parameters = "zypper --root '/path/to/root' --quiet --non-interactive removeservice 'branding'" Object.should_receive(:system).with(parameters).and_return(true) - $suse_connect_filesystem_root = '/path/to/root' + SUSE::Connect::System.filesystem_root = '/path/to/root' subject.remove_service('branding') - $suse_connect_filesystem_root = '' + SUSE::Connect::System.filesystem_root = '' end end @@ -118,10 +118,10 @@ end it 'calls zypper with proper arguments --root case' do - $suse_connect_filesystem_root = '/path/to/root' + SUSE::Connect::System.filesystem_root = '/path/to/root' Object.should_receive(:system).with("zypper --root '/path/to/root' refresh").and_return(true) subject.refresh - $suse_connect_filesystem_root = '' + SUSE::Connect::System.filesystem_root = '' end end @@ -137,9 +137,9 @@ it 'calls zypper with proper arguments --root case' do parameters = "zypper --root '/path/to/root' --quiet modifyservice --ar-to-enable 'branding:tofu' 'branding'" Object.should_receive(:system).with(parameters).and_return(true) - $suse_connect_filesystem_root = '/path/to/root' + SUSE::Connect::System.filesystem_root = '/path/to/root' subject.enable_service_repository('branding', 'tofu') - $suse_connect_filesystem_root = '' + SUSE::Connect::System.filesystem_root = '' end end @@ -155,9 +155,9 @@ it 'calls zypper with proper arguments --root case' do parameters = "zypper --root '/path/to/root' --quiet modifyrepo --no-refresh 'branding:tofu'" Object.should_receive(:system).with(parameters).and_return(true) - $suse_connect_filesystem_root = '/path/to/root' + SUSE::Connect::System.filesystem_root = '/path/to/root' subject.disable_repository_autorefresh('branding', 'tofu') - $suse_connect_filesystem_root = '' + SUSE::Connect::System.filesystem_root = '' end end @@ -270,9 +270,9 @@ it 'return zypper targetos output --root case' do Object.should_receive(:'`').with("zypper --root '/path/to/root' targetos").and_return('openSUSE-13.1-x86_64') - $suse_connect_filesystem_root = '/path/to/root' + SUSE::Connect::System.filesystem_root = '/path/to/root' Zypper.distro_target.should eq 'openSUSE-13.1-x86_64' - $suse_connect_filesystem_root = '' + SUSE::Connect::System.filesystem_root = '' end end From 8f268ee4adb93471e0c5e40ec4cea8bd59774dd1 Mon Sep 17 00:00:00 2001 From: Thomas Schmidt Date: Mon, 26 May 2014 14:04:37 +0200 Subject: [PATCH 3/7] fix string checks in feature tests --- features/help.feature | 12 ++++++------ features/step_definition/registration_steps.rb | 2 +- lib/suse/connect/cli.rb | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/features/help.feature b/features/help.feature index 0b83db80..b4b102ba 100644 --- a/features/help.feature +++ b/features/help.feature @@ -8,13 +8,13 @@ Feature: Help output Scenario: help should contain host parameter Then the output should contain: """ - --url [URL] Connection base url (e.g. https://scc.suse.com). + --url [URL] """ Scenario: help should contain token parameter Then the output should contain: """ - -r, --regcode [REGCODE] Registration code. The repositories of the subscription with this registration code will get activated on this system. + -r, --regcode [REGCODE] """ # Common Options @@ -22,23 +22,23 @@ Feature: Help output Scenario: help should contain help option Then the output should contain: """ - --help Show this message. + --help """ Scenario: help should contain dry mode option Then the output should contain: """ - -d, --dry-mode Dry mode. Does not make any changes to the system. + -d, --dry-run """ Scenario: help should contain version option Then the output should contain: """ - --version Print version + --version """ Scenario: help should contain verbose option Then the output should contain: """ - -v, --verbose Run verbosely. + --debug """ diff --git a/features/step_definition/registration_steps.rb b/features/step_definition/registration_steps.rb index af554818..038eea36 100644 --- a/features/step_definition/registration_steps.rb +++ b/features/step_definition/registration_steps.rb @@ -7,7 +7,7 @@ end Then(/^output should inform us about you need an argument if running with url parameter$/) do - assert_exact_output('Please provide url parameter', all_output.chomp) + assert_exact_output('Please provide registration server URL', all_output.chomp) end Then(/^outputs should not contain info about required url param$/) do diff --git a/lib/suse/connect/cli.rb b/lib/suse/connect/cli.rb index cc343111..48708987 100644 --- a/lib/suse/connect/cli.rb +++ b/lib/suse/connect/cli.rb @@ -19,7 +19,7 @@ def execute! # rubocop:disable MethodLength unless @options[:token] puts @opts - exit + exit(1) end Client.new(@options).register! From e43822838a5f3ce4f1f98b29f8b04dfa958ccad4 Mon Sep 17 00:00:00 2001 From: Thomas Schmidt Date: Mon, 26 May 2014 15:21:54 +0200 Subject: [PATCH 4/7] using after filter --- spec/connect/zyppper_spec.rb | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/spec/connect/zyppper_spec.rb b/spec/connect/zyppper_spec.rb index 2e35cf62..19437f81 100644 --- a/spec/connect/zyppper_spec.rb +++ b/spec/connect/zyppper_spec.rb @@ -6,6 +6,10 @@ Object.stub(:system => true) end + after(:each) do + SUSE::Connect::System.filesystem_root = nil + end + subject { SUSE::Connect::Zypper } describe '.installed_products' do @@ -16,12 +20,7 @@ let(:xml) { File.read('spec/fixtures/product_valid_sle11sp3.xml') } before do - SUSE::Connect::System.filesystem_root = '/path/to/root' - Object.should_receive(:'`').with(include "zypper --root '/path/to/root' ").and_return(xml) - end - - after do - SUSE::Connect::System.filesystem_root = '' + Object.should_receive(:'`').and_return(xml) end it 'returns valid list of products based on proper XML' do @@ -87,7 +86,6 @@ Object.should_receive(:system).with(parameters).and_return(true) SUSE::Connect::System.filesystem_root = '/path/to/root' subject.add_service('branding', 'http://example.com') - SUSE::Connect::System.filesystem_root = '' end end @@ -105,7 +103,6 @@ Object.should_receive(:system).with(parameters).and_return(true) SUSE::Connect::System.filesystem_root = '/path/to/root' subject.remove_service('branding') - SUSE::Connect::System.filesystem_root = '' end end @@ -121,7 +118,6 @@ SUSE::Connect::System.filesystem_root = '/path/to/root' Object.should_receive(:system).with("zypper --root '/path/to/root' refresh").and_return(true) subject.refresh - SUSE::Connect::System.filesystem_root = '' end end @@ -139,7 +135,6 @@ Object.should_receive(:system).with(parameters).and_return(true) SUSE::Connect::System.filesystem_root = '/path/to/root' subject.enable_service_repository('branding', 'tofu') - SUSE::Connect::System.filesystem_root = '' end end @@ -157,7 +152,6 @@ Object.should_receive(:system).with(parameters).and_return(true) SUSE::Connect::System.filesystem_root = '/path/to/root' subject.disable_repository_autorefresh('branding', 'tofu') - SUSE::Connect::System.filesystem_root = '' end end @@ -272,7 +266,6 @@ Object.should_receive(:'`').with("zypper --root '/path/to/root' targetos").and_return('openSUSE-13.1-x86_64') SUSE::Connect::System.filesystem_root = '/path/to/root' Zypper.distro_target.should eq 'openSUSE-13.1-x86_64' - SUSE::Connect::System.filesystem_root = '' end end From f9e864e9f986905da859c14d4b80f1b586b8e734 Mon Sep 17 00:00:00 2001 From: Thomas Schmidt Date: Mon, 26 May 2014 15:27:09 +0200 Subject: [PATCH 5/7] setting filesystem root to nil by default --- lib/suse/connect/credentials.rb | 12 ++++++++++-- lib/suse/connect/system.rb | 2 -- lib/suse/connect/zypper.rb | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/lib/suse/connect/credentials.rb b/lib/suse/connect/credentials.rb index 79f2ebd1..9954eed5 100644 --- a/lib/suse/connect/credentials.rb +++ b/lib/suse/connect/credentials.rb @@ -24,7 +24,11 @@ def initialize(user, password, file = nil) end def self.system_credentials_file - File.join(SUSE::Connect::System.filesystem_root, GLOBAL_CREDENTIALS_FILE) + if SUSE::Connect::System.filesystem_root + File.join(SUSE::Connect::System.filesystem_root, GLOBAL_CREDENTIALS_FILE) + else + GLOBAL_CREDENTIALS_FILE + end end def self.read(file) @@ -38,7 +42,11 @@ def self.read(file) end def filename - default_dir = File.join(SUSE::Connect::System.filesystem_root, DEFAULT_CREDENTIALS_DIR) + if SUSE::Connect::System.filesystem_root + default_dir = File.join(SUSE::Connect::System.filesystem_root, DEFAULT_CREDENTIALS_DIR) + else + default_dir = DEFAULT_CREDENTIALS_DIR + end Pathname.new(file).absolute? ? file : File.join(default_dir, file) end diff --git a/lib/suse/connect/system.rb b/lib/suse/connect/system.rb index ea7b9459..487cfd8e 100644 --- a/lib/suse/connect/system.rb +++ b/lib/suse/connect/system.rb @@ -3,8 +3,6 @@ module Connect # System class allowing to interact with underlying system class System - @filesystem_root = '' - class << self attr_accessor :filesystem_root diff --git a/lib/suse/connect/zypper.rb b/lib/suse/connect/zypper.rb index 2c24556f..eba0c04d 100644 --- a/lib/suse/connect/zypper.rb +++ b/lib/suse/connect/zypper.rb @@ -81,7 +81,7 @@ def write_base_credentials(login, password) private def root_arg - "--root '#{SUSE::Connect::System.filesystem_root}' " unless SUSE::Connect::System.filesystem_root.empty? + "--root '#{SUSE::Connect::System.filesystem_root}' " if SUSE::Connect::System.filesystem_root end def call_zypper(silent, args) From de4bf8e7168866fd63e04daf105ba968c37cb873 Mon Sep 17 00:00:00 2001 From: Thomas Schmidt Date: Mon, 26 May 2014 16:59:10 +0200 Subject: [PATCH 6/7] fix indentation --- lib/suse/connect/credentials.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/suse/connect/credentials.rb b/lib/suse/connect/credentials.rb index 9954eed5..edaf7102 100644 --- a/lib/suse/connect/credentials.rb +++ b/lib/suse/connect/credentials.rb @@ -27,7 +27,7 @@ def self.system_credentials_file if SUSE::Connect::System.filesystem_root File.join(SUSE::Connect::System.filesystem_root, GLOBAL_CREDENTIALS_FILE) else - GLOBAL_CREDENTIALS_FILE + GLOBAL_CREDENTIALS_FILE end end From 065e1d1ca6f2dd026b923376d6e78fe8aff0bd22 Mon Sep 17 00:00:00 2001 From: Thomas Schmidt Date: Mon, 26 May 2014 18:23:56 +0200 Subject: [PATCH 7/7] reset filesystem_root to nil --- spec/connect/cli_spec.rb | 2 +- spec/connect/credentials_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/connect/cli_spec.rb b/spec/connect/cli_spec.rb index b1a7e011..9dd23951 100644 --- a/spec/connect/cli_spec.rb +++ b/spec/connect/cli_spec.rb @@ -116,7 +116,7 @@ argv = %w{--root /path/to/root} subject.new(argv) SUSE::Connect::System.filesystem_root.should eq '/path/to/root' - SUSE::Connect::System.filesystem_root = '' + SUSE::Connect::System.filesystem_root = nil end end diff --git a/spec/connect/credentials_spec.rb b/spec/connect/credentials_spec.rb index d50fa03e..058cd149 100755 --- a/spec/connect/credentials_spec.rb +++ b/spec/connect/credentials_spec.rb @@ -15,7 +15,7 @@ SUSE::Connect::System.filesystem_root = '/path/to/root' expected = File.join('/path/to/root', Credentials::GLOBAL_CREDENTIALS_FILE) expect(Credentials.system_credentials_file).to eq expected - SUSE::Connect::System.filesystem_root = '' + SUSE::Connect::System.filesystem_root = nil end end @@ -67,7 +67,7 @@ SUSE::Connect::System.filesystem_root = '/path/to/root' credentials = Credentials.new('name', '1234', 'SLES') credentials.filename.should start_with '/path/to/root/' - SUSE::Connect::System.filesystem_root = '' + SUSE::Connect::System.filesystem_root = nil end it 'raises an error when file name is not set' do