Merge pull request #92 from github/kpaulisse-environment-without-preserve

Bump version and changelog for #86

Author: kpaulisse

.version

@@ -1 +1 @@
-1.0.2
+1.0.3

doc/CHANGELOG.md

@@ -8,6 +8,13 @@
 </tr>
 </thead><tbody>
 <tr valign=top>
+<td>1.0.3</td>
+<td>2017-03-15</td>
+<td>
+<li><a href="https://github.com/github/octocatalog-diff/pull/86">#86</a>: Ability to use `--environment` without `--preserve-environments`</li>
+</td>
+</tr>
+<tr valign=top>
 <td>1.0.2</td>
 <td>2017-03-08</td>
 <td>

lib/octocatalog-diff/catalog-util/builddir.rb

@@ -84,11 +84,14 @@ def create_symlinks(logger = nil)
             install_directory_symlink(logger, File.join(@options[:basedir], x), x)
           end
         else
-          if @options[:environment]
-            logger.warn '--environment is ignored unless --preserve-environments is used' unless logger.nil?
-          end
-          if @options[:create_symlinks]
+          if @options[:create_symlinks] && @options[:environment]
+            unless logger.nil?
+              logger.warn '--create-symlinks with --environment ignored unless --preserve-environments is used'
+            end
+          elsif @options[:create_symlinks]
             logger.warn '--create-symlinks is ignored unless --preserve-environments is used' unless logger.nil?
+          elsif @options[:environment]
+            return install_directory_symlink(logger, @options[:basedir], "environments/#{@options[:environment]}")
           end
           install_directory_symlink(logger, @options[:basedir])
         end
@@ -178,6 +181,7 @@ def install_fact_file(logger, options)
       def install_directory_symlink(logger, dir, target = 'environments/production')
         raise ArgumentError, "Called install_directory_symlink with #{dir.class} argument" unless dir.is_a?(String)
         raise Errno::ENOENT, "Specified directory #{dir} doesn't exist" unless File.directory?(dir)
+
         symlink_target = File.join(@tempdir, target)
 
         if target =~ %r{/}
@@ -330,7 +334,7 @@ def install_ssl_client_password(logger, password)
       end
 
       def environment
-        @options[:preserve_environments] ? @options.fetch(:environment, 'production') : 'production'
+        @options.fetch(:environment, 'production')
       end
     end
   end

lib/octocatalog-diff/catalog-util/command.rb

@@ -78,7 +78,7 @@ def puppet_command
         # Add environment - only make this variable if preserve_environments is used.
         # If preserve_environments is not used, the hard-coded 'production' here matches
         # up with the symlink created under the temporary directory structure.
-        environ = @options[:preserve_environments] ? @options.fetch(:environment, 'production') : 'production'
+        environ = @options.fetch(:environment, 'production')
         cmdline << "--environment=#{Shellwords.escape(environ)}"
 
         # For people who aren't running hiera, a hiera-config will not be generated when @options[:hiera_config]

lib/octocatalog-diff/catalog/computed.rb

@@ -80,7 +80,7 @@ def compilation_dir
 
       # Environment used to compile catalog
       def environment
-        @opts[:preserve_environments] ? @opts.fetch(:environment, 'production') : 'production'
+        @opts.fetch(:environment, 'production')
       end
 
       private

spec/octocatalog-diff/integration/preserve_environments_spec.rb

@@ -40,7 +40,7 @@
             '-n', 'rspec-node.github.net',
             '--environment', 'asdfgh',
             '--to-catalog', OctocatalogDiff::Spec.fixture_path('catalogs/default-catalog-v4.json'),
-            '--hiera-config', 'environments/production/config/hiera.yaml',
+            '--hiera-config', 'config/hiera.yaml',
             '--hiera-path-strip', '/var/lib/puppet', '--no-parallel'
           ]
         )
@@ -50,10 +50,6 @@
         expect(@result.exitcode).to eq(0), OctocatalogDiff::Integration.format_exception(@result)
         expect(@result.exception).to be_nil, OctocatalogDiff::Integration.format_exception(@result)
       end
-
-      it 'should log warning about --environment being useless in this context' do
-        expect(@result.logs).to match(/WARN -- : --environment is ignored unless --preserve-environments is used/)
-      end
     end
 
     context 'with --create-symlinks set' do

spec/octocatalog-diff/tests/catalog-util/builddir_spec.rb

@@ -87,7 +87,7 @@
       context 'with --create-symlinks' do
         context 'with logger' do
           it 'should log a warning message and install default symlink' do
-            logger = double('Logger')
+            logger, = OctocatalogDiff::Spec.setup_logger
             expect(logger).to receive(:warn).with('--create-symlinks is ignored unless --preserve-environments is used')
 
             @described_object.instance_variable_set(
@@ -121,15 +121,18 @@
         end
       end
 
-      context 'with --environment' do
+      context 'with --create-symlinks and --environment' do
         context 'with logger' do
           it 'should log a warning message and install default symlink' do
-            logger = double('Logger')
-            expect(logger).to receive(:warn).with('--environment is ignored unless --preserve-environments is used')
+            logger, = OctocatalogDiff::Spec.setup_logger
+            expect(logger).to receive(:warn).with(
+              '--create-symlinks with --environment ignored unless --preserve-environments is used'
+            )
 
             @described_object.instance_variable_set(
               '@options',
               basedir: '/tmp/basedir',
+              create_symlinks: %w(foo bar),
               environment: 'baz'
             )
 
@@ -146,6 +149,7 @@
             @described_object.instance_variable_set(
               '@options',
               basedir: '/tmp/basedir',
+              create_symlinks: %w(foo bar),
               environment: 'baz'
             )
 
@@ -157,10 +161,45 @@
           end
         end
       end
+      context 'with --environment' do
+        context 'with logger' do
+          it 'should install a symlink to the given environment' do
+            logger, = OctocatalogDiff::Spec.setup_logger
+
+            @described_object.instance_variable_set(
+              '@options',
+              basedir: '/tmp/basedir',
+              environment: 'baz'
+            )
+
+            expect(@described_object)
+              .to receive(:install_directory_symlink)
+              .with(logger, '/tmp/basedir', 'environments/baz')
+
+            expect { @described_object.send(:create_symlinks, logger) }.not_to raise_error
+          end
+        end
+
+        context 'without logger' do
+          it 'should install default symlink' do
+            @described_object.instance_variable_set(
+              '@options',
+              basedir: '/tmp/basedir',
+              environment: 'baz'
+            )
+
+            expect(@described_object)
+              .to receive(:install_directory_symlink)
+              .with(nil, '/tmp/basedir', 'environments/baz')
+
+            expect { @described_object.send(:create_symlinks) }.not_to raise_error
+          end
+        end
+      end
 
       context 'without --create-symlinks or --environment' do
         it 'should install directory symlink' do
-          logger = double('Logger')
+          logger, = OctocatalogDiff::Spec.setup_logger
 
           @described_object.instance_variable_set(
             '@options',

spec/octocatalog-diff/tests/catalog/computed_spec.rb

@@ -422,17 +422,19 @@
     context 'when preserve_environments is not set' do
       context 'and environment is specified' do
         it 'should return without raising error when directory exists' do
-          allow(File).to receive(:"directory?").with('/tmp/assert/environments/production').and_return(true)
+          allow(File).to receive(:"directory?").with('/tmp/assert/environments/production').and_return(false)
+          allow(File).to receive(:"directory?").with('/tmp/assert/environments/custom').and_return(true)
           described_object = described_class.allocate
-          described_object.instance_variable_set('@opts', environment: 'nope')
+          described_object.instance_variable_set('@opts', environment: 'custom')
           described_object.instance_variable_set('@builddir', OpenStruct.new(tempdir: '/tmp/assert'))
           expect { described_object.send(:assert_that_puppet_environment_directory_exists) }.not_to raise_error
         end
 
         it 'should raise Errno::ENOENT when directory does not exist' do
-          allow(File).to receive(:"directory?").with('/tmp/assert/environments/production').and_return(false)
+          allow(File).to receive(:"directory?").with('/tmp/assert/environments/production').and_return(true)
+          allow(File).to receive(:"directory?").with('/tmp/assert/environments/custom').and_return(false)
           described_object = described_class.allocate
-          described_object.instance_variable_set('@opts', environment: 'yup')
+          described_object.instance_variable_set('@opts', environment: 'custom')
           described_object.instance_variable_set('@builddir', OpenStruct.new(tempdir: '/tmp/assert'))
           expect { described_object.send(:assert_that_puppet_environment_directory_exists) }.to raise_error(Errno::ENOENT)
         end