Merge pull request #80 from SUSE/review_140526_include_global_declara…

…tion_for_root_parameter

Move global declaration to one place where it will be required, and ensure that is required during tests

features/help.feature

@@ -8,37 +8,37 @@ 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
 
   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
       """

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

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
@@ -21,7 +19,7 @@ def execute! # rubocop:disable MethodLength
 
         unless @options[:token]
           puts @opts
-          exit
+          exit(1)
         end
         Client.new(@options).register!
 
@@ -86,7 +84,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

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

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_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_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
 

lib/suse/connect/system.rb

@@ -5,12 +5,14 @@ class System
 
       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!)

lib/suse/connect/zypper.rb

@@ -82,7 +82,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}' " if SUSE::Connect::System.filesystem_root
         end
 
         def call_zypper(silent, args)

rubocop.yml

@@ -39,7 +39,7 @@ ClassAndModuleChildren:
   Enabled: false
 
 GlobalVars:
-  AllowedVariables: ['$suse_connect_filesystem_root']
+  AllowedVariables: []
 
 AllCops:
   Exclude:

spec/connect/cli_spec.rb

@@ -109,8 +109,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 = nil
     end
 
   end

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 = nil
     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 = nil
     end
 
     it 'raises an error when file name is not set' do

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_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 = ''
+          Object.should_receive(:'`').and_return(xml)
         end
 
         it 'returns valid list of products based on proper XML' do
@@ -91,9 +90,8 @@
       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 = ''
     end
 
   end
@@ -109,9 +107,8 @@
     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 = ''
     end
 
   end
@@ -124,10 +121,9 @@
     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 = ''
     end
 
   end
@@ -143,9 +139,8 @@
     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 = ''
     end
 
   end
@@ -161,9 +156,8 @@
     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 = ''
     end
 
   end
@@ -276,9 +270,8 @@
 
     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 = ''
     end
   end