Skip to content

Commit 7a6ab3e

Browse files
authored
Merge pull request #52 from github/kpaulisse-test-coverage
Additional test coverage
2 parents b4cfb75 + 2fc2b20 commit 7a6ab3e

6 files changed

Lines changed: 236 additions & 22 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ def self.git_checkout(logger, dir_opts)
101101
logger.debug("Begin git checkout #{dir_opts[:basedir]}:#{dir_opts[:branch]} -> #{dir_opts[:path]}")
102102
OctocatalogDiff::CatalogUtil::Git.check_out_git_archive(dir_opts.merge(logger: logger))
103103
logger.debug("Success git checkout #{dir_opts[:basedir]}:#{dir_opts[:branch]} -> #{dir_opts[:path]}")
104-
rescue OctocatalogDiff::CatalogUtil::Git::GitCheckoutError => exc
104+
rescue OctocatalogDiff::Errors::GitCheckoutError => exc
105105
logger.error("Git checkout error: #{exc}")
106106
raise OctocatalogDiff::Errors::BootstrapError, exc
107107
end

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

Lines changed: 27 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -29,30 +29,36 @@ def self.check_out_git_archive(options = {})
2929
if dir.nil? || !File.directory?(dir)
3030
raise OctocatalogDiff::Errors::GitCheckoutError, "Source directory #{dir.inspect} does not exist"
3131
end
32-
if dir.nil? || !File.directory?(path)
32+
if path.nil? || !File.directory?(path)
3333
raise OctocatalogDiff::Errors::GitCheckoutError, "Target directory #{path.inspect} does not exist"
3434
end
3535

36-
# To get the options working correctly (-o pipefail in particular) this needs to run under
37-
# bash. It's just creating a script, rather than figuring out all the shell escapes...
38-
begin
39-
tmp_script = Tempfile.new(['git-checkout', '.sh'])
40-
tmp_script.write "#!/bin/bash\n"
41-
tmp_script.write "set -euf -o pipefail\n"
42-
tmp_script.write "git archive --format=tar #{Shellwords.escape(branch)} | \\\n"
43-
tmp_script.write " ( cd #{Shellwords.escape(path)} && tar -xf - )\n"
44-
tmp_script.close
45-
FileUtils.chmod 0o755, tmp_script.path
46-
47-
logger.debug("Begin git archive #{dir}:#{branch} -> #{path}")
48-
output, status = Open3.capture2e(tmp_script.path, chdir: dir)
49-
unless status.exitstatus.zero?
50-
raise OctocatalogDiff::Errors::GitCheckoutError, "Git archive #{branch}->#{path} failed: #{output}"
51-
end
52-
logger.debug("Success git archive #{dir}:#{branch}")
53-
ensure
54-
FileUtils.rm_f tmp_script.path if File.exist?(tmp_script.path)
36+
# Create and execute checkout script
37+
script = create_git_checkout_script(branch, path)
38+
logger.debug("Begin git archive #{dir}:#{branch} -> #{path}")
39+
output, status = Open3.capture2e(script, chdir: dir)
40+
unless status.exitstatus.zero?
41+
raise OctocatalogDiff::Errors::GitCheckoutError, "Git archive #{branch}->#{path} failed: #{output}"
5542
end
43+
logger.debug("Success git archive #{dir}:#{branch}")
44+
end
45+
46+
# Create the temporary file used to interact with the git command line.
47+
# To get the options working correctly (-o pipefail in particular) this needs to run under
48+
# bash. It's just creating a script, rather than figuring out all the shell escapes...
49+
# @param branch [String] Branch name
50+
# @param path [String] Target directory
51+
# @return [String] Name of script
52+
def self.create_git_checkout_script(branch, path)
53+
tmp_script = Tempfile.new(['git-checkout', '.sh'])
54+
tmp_script.write "#!/bin/bash\n"
55+
tmp_script.write "set -euf -o pipefail\n"
56+
tmp_script.write "git archive --format=tar #{Shellwords.escape(branch)} | \\\n"
57+
tmp_script.write " ( cd #{Shellwords.escape(path)} && tar -xf - )\n"
58+
tmp_script.close
59+
FileUtils.chmod 0o755, tmp_script.path
60+
at_exit { FileUtils.rm_f tmp_script.path if File.exist?(tmp_script.path) }
61+
tmp_script.path
5662
end
5763

5864
# Determine the SHA of origin/master (or any other branch really) in the git repo
@@ -63,7 +69,7 @@ def self.branch_sha(options = {})
6369
branch = options.fetch(:branch)
6470
dir = options.fetch(:basedir)
6571
if dir.nil? || !File.directory?(dir)
66-
raise OctocatalogDiff::Errors::GitCheckoutError, "Git directory #{dir.inspect} does not exist"
72+
raise Errno::ENOENT, "Git directory #{dir.inspect} does not exist"
6773
end
6874
repo = Rugged::Repository.new(dir)
6975
repo.branches[branch].target_id

lib/octocatalog-diff/util/catalogs.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,11 @@ def build_catalog_parallelizer
115115
# Things have succeeded if the :to and :from catalogs exist at this point. If not, things have
116116
# failed, and an exception should be thrown.
117117
return result if result.key?(:to) && result.key?(:from)
118+
119+
# This is believed to be a bug condition.
120+
# :nocov:
118121
raise OctocatalogDiff::Errors::CatalogError, 'One or more catalogs failed to compile.'
122+
# :nocov:
119123
end
120124

121125
# Get catalog compilation tasks.

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

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,4 +164,73 @@
164164
end
165165
end
166166
end
167+
168+
describe '#git_checkout' do
169+
context 'with successful git checkout' do
170+
it 'should log success messages' do
171+
expect(OctocatalogDiff::CatalogUtil::Git).to receive(:check_out_git_archive)
172+
logger, logger_str = OctocatalogDiff::Spec.setup_logger
173+
opts = { basedir: '/tmp/foo', branch: 'foo', path: '/tmp/bar' }
174+
described_class.git_checkout(logger, opts)
175+
expect(logger_str.string).to match(%r{Begin git checkout /tmp/foo:foo -> /tmp/bar})
176+
expect(logger_str.string).to match(%r{Success git checkout /tmp/foo:foo -> /tmp/bar})
177+
end
178+
end
179+
180+
context 'with failed git checkout' do
181+
it 'should log error messages and raise OctocatalogDiff::Errors::BootstrapError' do
182+
expect(OctocatalogDiff::CatalogUtil::Git).to receive(:check_out_git_archive)
183+
.and_raise(OctocatalogDiff::Errors::GitCheckoutError, 'Oopsie')
184+
logger, logger_str = OctocatalogDiff::Spec.setup_logger
185+
opts = { basedir: '/tmp/foo', branch: 'foo', path: '/tmp/bar' }
186+
expect { described_class.git_checkout(logger, opts) }.to raise_error(OctocatalogDiff::Errors::BootstrapError)
187+
expect(logger_str.string).to match(%r{Begin git checkout /tmp/foo:foo -> /tmp/bar})
188+
expect(logger_str.string).to match(/Git checkout error: Oopsie/)
189+
end
190+
end
191+
end
192+
193+
describe '#run_bootstrap' do
194+
before(:each) do
195+
@logger, @logger_str = OctocatalogDiff::Spec.setup_logger
196+
end
197+
198+
context 'with successful run' do
199+
before(:each) do
200+
expect(OctocatalogDiff::Bootstrap).to receive(:bootstrap).and_return(status_code: 0, output: 'worked')
201+
end
202+
203+
context 'with bootstrap debugging' do
204+
it 'should succeed and print debugging messages' do
205+
opts = { bootstrap_script: 'foo.sh', debug_bootstrap: true, path: '/tmp/bar' }
206+
result = described_class.run_bootstrap(@logger, opts)
207+
expect(result).to eq('worked')
208+
expect(@logger_str.string).to match(%r{Begin bootstrap with 'foo.sh' in /tmp/bar})
209+
expect(@logger_str.string).to match(/Bootstrap: worked/)
210+
expect(@logger_str.string).to match(%r{Success bootstrap in /tmp/bar})
211+
end
212+
end
213+
214+
context 'without bootstrap debugging' do
215+
it 'should succeed without debugging messages' do
216+
opts = { bootstrap_script: 'foo.sh', path: '/tmp/bar' }
217+
result = described_class.run_bootstrap(@logger, opts)
218+
expect(result).to eq('worked')
219+
expect(@logger_str.string).to match(%r{Begin bootstrap with 'foo.sh' in /tmp/bar})
220+
expect(@logger_str.string).not_to match(/Bootstrap: worked/)
221+
expect(@logger_str.string).to match(%r{Success bootstrap in /tmp/bar})
222+
end
223+
end
224+
end
225+
226+
context 'with failed run' do
227+
it 'should raise OctocatalogDiff::Errors::BootstrapError' do
228+
expect(OctocatalogDiff::Bootstrap).to receive(:bootstrap).and_return(status_code: 1, output: 'Oopsie')
229+
opts = { bootstrap_script: 'foo.sh', path: '/tmp/bar' }
230+
expect { described_class.run_bootstrap(@logger, opts) }.to raise_error(OctocatalogDiff::Errors::BootstrapError)
231+
expect(@logger_str.string).to match(%r{Begin bootstrap with 'foo.sh' in /tmp/bar})
232+
expect(@logger_str.string).to match(/Bootstrap: Oopsie/)
233+
end
234+
end
235+
end
167236
end
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
# frozen_string_literal: true
2+
3+
require_relative '../spec_helper'
4+
5+
require OctocatalogDiff::Spec.require_path('catalog-util/git')
6+
require OctocatalogDiff::Spec.require_path('errors')
7+
8+
require 'ostruct'
9+
10+
describe OctocatalogDiff::CatalogUtil::Git do
11+
before(:each) do
12+
@logger, @logger_str = OctocatalogDiff::Spec.setup_logger
13+
allow(File).to receive(:'directory?').with('/tmp/foo').and_return(false)
14+
allow(File).to receive(:'directory?').with('/tmp/bar').and_return(true)
15+
end
16+
17+
describe '#check_out_git_archive' do
18+
context 'with invalid directory' do
19+
it 'should raise OctocatalogDiff::Errors::GitCheckoutError if basedir is nil' do
20+
opts = { branch: 'foo', path: '/tmp/foo', basedir: nil, logger: @logger }
21+
expect do
22+
described_class.check_out_git_archive(opts)
23+
end.to raise_error(OctocatalogDiff::Errors::GitCheckoutError, /Source directory/)
24+
end
25+
26+
it 'should raise OctocatalogDiff::Errors::GitCheckoutError if basedir does not exist' do
27+
opts = { branch: 'foo', path: '/tmp/foo', basedir: '/tmp/foo', logger: @logger }
28+
expect do
29+
described_class.check_out_git_archive(opts)
30+
end.to raise_error(OctocatalogDiff::Errors::GitCheckoutError, /Source directory/)
31+
end
32+
33+
it 'should raise OctocatalogDiff::Errors::GitCheckoutError if path is nil' do
34+
opts = { branch: 'foo', path: nil, basedir: '/tmp/bar', logger: @logger }
35+
expect do
36+
described_class.check_out_git_archive(opts)
37+
end.to raise_error(OctocatalogDiff::Errors::GitCheckoutError, /Target directory/)
38+
end
39+
40+
it 'should raise OctocatalogDiff::Errors::GitCheckoutError if path does not exist' do
41+
opts = { branch: 'foo', path: '/tmp/foo', basedir: '/tmp/bar', logger: @logger }
42+
expect do
43+
described_class.check_out_git_archive(opts)
44+
end.to raise_error(OctocatalogDiff::Errors::GitCheckoutError, /Target directory/)
45+
end
46+
end
47+
48+
context 'with valid directory' do
49+
context 'with successful script run' do
50+
it 'should log proper messages and not raise error' do
51+
expect(described_class).to receive(:create_git_checkout_script).and_return('/tmp/baz.sh')
52+
expect(Open3).to receive(:capture2e)
53+
.with('/tmp/baz.sh', chdir: '/tmp/bar')
54+
.and_return(['asldfkj', OpenStruct.new(exitstatus: 0)])
55+
opts = { branch: 'foo', path: '/tmp/bar', basedir: '/tmp/bar', logger: @logger }
56+
described_class.check_out_git_archive(opts)
57+
expect(@logger_str.string).to match(%r{Success git archive /tmp/bar:foo})
58+
end
59+
end
60+
61+
context 'with failed script run' do
62+
it 'should raise OctocatalogDiff::Errors::GitCheckoutError' do
63+
expect(described_class).to receive(:create_git_checkout_script).and_return('/tmp/baz.sh')
64+
expect(Open3).to receive(:capture2e)
65+
.with('/tmp/baz.sh', chdir: '/tmp/bar')
66+
.and_return(['errors abound', OpenStruct.new(exitstatus: 1)])
67+
opts = { branch: 'foo', path: '/tmp/bar', basedir: '/tmp/bar', logger: @logger }
68+
expect do
69+
described_class.check_out_git_archive(opts)
70+
end.to raise_error(OctocatalogDiff::Errors::GitCheckoutError, 'Git archive foo->/tmp/bar failed: errors abound')
71+
end
72+
end
73+
end
74+
end
75+
76+
describe '#create_git_checkout_script' do
77+
it 'should create the temporary script' do
78+
result = described_class.create_git_checkout_script('foo', '/tmp/baz')
79+
expect(result).to be_a_kind_of(String)
80+
expect(File.file?(result)).to eq(true)
81+
82+
text = File.read(result)
83+
expect(text).to match(/git archive --format=tar foo \|/)
84+
expect(text).to match(%r{\( cd /tmp/baz && tar -xf - \)})
85+
86+
expect(File.executable?(result)).to eq(true)
87+
end
88+
end
89+
90+
describe '#branch_sha' do
91+
context 'with invalid directory' do
92+
it 'should raise Errno::ENOENT if basedir is nil' do
93+
opts = { branch: 'foo', basedir: nil }
94+
expect do
95+
described_class.branch_sha(opts)
96+
end.to raise_error(Errno::ENOENT, /Git directory/)
97+
end
98+
99+
it 'should raise Errno::ENOENT if basedir does not exist' do
100+
opts = { branch: 'foo', basedir: '/tmp/foo' }
101+
expect do
102+
described_class.branch_sha(opts)
103+
end.to raise_error(Errno::ENOENT, /Git directory/)
104+
end
105+
end
106+
107+
context 'with valid directory' do
108+
it 'should return the sha from rugged' do
109+
opts = { branch: 'foo', basedir: '/tmp/bar' }
110+
expect(Rugged::Repository).to receive(:new).with('/tmp/bar')
111+
.and_return(OpenStruct.new(branches: { 'foo' => OpenStruct.new(target_id: 'abcdef012345') }))
112+
result = described_class.branch_sha(opts)
113+
expect(result).to eq('abcdef012345')
114+
end
115+
end
116+
end
117+
end

spec/octocatalog-diff/tests/util/catalogs_spec.rb

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,24 @@ def valid?
290290
expect(result[:from].builder.to_s).to eq('OctocatalogDiff::Catalog::PuppetDB')
291291
expect(result[:to].builder.to_s).to eq('OctocatalogDiff::Catalog::JSON')
292292
end
293+
294+
it 'should raise OctocatalogDiff::Errors::CatalogError if either catalog fails' do
295+
options = {
296+
to_catalog: OctocatalogDiff::Spec.fixture_path('catalogs/tiny-catalog.json'),
297+
from_fact_file: OctocatalogDiff::Spec.fixture_path('facts/facts.yaml'),
298+
bootstrapped_from_dir: OctocatalogDiff::Spec.fixture_path('repos/failing-catalog'),
299+
node: 'tiny-catalog-2-puppetdb',
300+
from_branch: 'foo',
301+
from_env: 'foo-env',
302+
puppet_binary: OctocatalogDiff::Spec::PUPPET_BINARY
303+
}
304+
logger, logger_str = OctocatalogDiff::Spec.setup_logger
305+
testobj = OctocatalogDiff::Util::Catalogs.new(options, logger)
306+
expect do
307+
testobj.send(:build_catalog_parallelizer)
308+
end.to raise_error(OctocatalogDiff::Errors::CatalogError)
309+
expect(logger_str.string).to match(/Failed build_catalog for foo-env/)
310+
end
293311
end
294312

295313
describe '#add_parallel_result' do

0 commit comments

Comments
 (0)