Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(PDK-1363) Apply init templates during module convert #729

Merged
merged 3 commits into from
Aug 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 18 additions & 9 deletions lib/pdk/module/convert.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ def initialize(options = {})
@options = options
end

def convert?
instance_of?(PDK::Module::Convert)
end

def run
stage_changes!

Expand Down Expand Up @@ -68,25 +72,30 @@ def needs_bundle_update?
def stage_changes!
metadata_path = 'metadata.json'

PDK::Module::TemplateDir.new(template_uri, nil, false) do |templates|
PDK::Module::TemplateDir.new(template_uri, nil, true) do |templates|
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PDK::Module::TemplateDir constructor seems like a good candidate for keyword arguments. Not necessarily now though.

new_metadata = update_metadata(metadata_path, templates.metadata)
templates.module_metadata = new_metadata.data unless new_metadata.nil?

if options[:noop] && new_metadata.nil?
update_manager.add_file(metadata_path, '')
elsif File.file?(metadata_path)
elsif PDK::Util::Filesystem.file?(metadata_path)
update_manager.modify_file(metadata_path, new_metadata.to_json)
else
update_manager.add_file(metadata_path, new_metadata.to_json)
end

templates.render do |file_path, file_content, file_status|
if file_status == :unmanage
case file_status
when :unmanage
PDK.logger.debug(_("skipping '%{path}'") % { path: file_path })
elsif file_status == :delete
when :delete
update_manager.remove_file(file_path)
elsif file_status == :manage
if File.exist? file_path
when :init
if convert? && !PDK::Util::Filesystem.exist?(file_path)
update_manager.add_file(file_path, file_content)
end
when :manage
if PDK::Util::Filesystem.exist?(file_path)
update_manager.modify_file(file_path, file_content)
else
update_manager.add_file(file_path, file_content)
Expand All @@ -107,8 +116,8 @@ def template_uri
end

def update_metadata(metadata_path, template_metadata)
if File.file?(metadata_path)
unless File.readable?(metadata_path)
if PDK::Util::Filesystem.file?(metadata_path)
unless PDK::Util::Filesystem.readable?(metadata_path)
raise PDK::CLI::ExitWithError, _('Unable to update module metadata; %{path} exists but it is not readable.') % {
path: metadata_path,
}
Expand All @@ -124,7 +133,7 @@ def update_metadata(metadata_path, template_metadata)
rescue ArgumentError
metadata = PDK::Generate::Module.prepare_metadata(options) unless options[:noop]
end
elsif File.exist?(metadata_path)
elsif PDK::Util::Filesystem.exist?(metadata_path)
raise PDK::CLI::ExitWithError, _('Unable to update module metadata; %{path} exists but it is not a file.') % {
path: metadata_path,
}
Expand Down
6 changes: 3 additions & 3 deletions lib/pdk/module/metadata.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,16 +94,16 @@ def self.from_file(metadata_json_path)
raise ArgumentError, _('Cannot read metadata from file: no path to file was given.')
end

unless File.file?(metadata_json_path)
unless PDK::Util::Filesystem.file?(metadata_json_path)
raise ArgumentError, _("'%{file}' does not exist or is not a file.") % { file: metadata_json_path }
end

unless File.readable?(metadata_json_path)
unless PDK::Util::Filesystem.readable?(metadata_json_path)
raise ArgumentError, _("Unable to open '%{file}' for reading.") % { file: metadata_json_path }
end

begin
data = JSON.parse(File.read(metadata_json_path))
data = JSON.parse(PDK::Util::Filesystem.read_file(metadata_json_path))
rescue JSON::JSONError => e
raise ArgumentError, _('Invalid JSON in metadata.json: %{msg}') % { msg: e.message }
end
Expand Down
7 changes: 6 additions & 1 deletion lib/pdk/module/templatedir.rb
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,12 @@ def render
PDK.logger.debug(_("Rendering '%{template}'...") % { template: template_file })
dest_path = template_file.sub(%r{\.erb\Z}, '')
config = config_for(dest_path)
dest_status = :manage

dest_status = if template_loc.start_with?(@moduleroot_init)
:init
else
:manage
end

if config['unmanaged']
dest_status = :unmanage
Expand Down
10 changes: 10 additions & 0 deletions lib/pdk/util/filesystem.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,16 @@ def fnmatch(*args)
File.fnmatch(*args)
end
module_function :fnmatch

def readable?(*args)
File.readable?(*args)
end
module_function :readable?

def exist?(*args)
File.exist?(*args)
end
module_function :exist?
#:nocov:
end
end
Expand Down
17 changes: 17 additions & 0 deletions spec/acceptance/convert_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -135,4 +135,21 @@
it { is_expected.not_to be_file }
end
end

context 'when an init-only templated file is missing' do
include_context 'in a new module', 'init_missing', template: template_repo

before(:all) do
FileUtils.rm_f('README.md')
end

describe command("#{pdk_convert_base} --force --skip-interview") do
its(:exit_status) { is_expected.to eq(0) }
its(:stderr) { is_expected.to have_no_output }
its(:stdout) { is_expected.to match(%r{-+files to be added-+\nREADME\.md}mi) }
describe file('README.md') do
it { is_expected.to be_file }
end
end
end
end
16 changes: 16 additions & 0 deletions spec/acceptance/update_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,5 +62,21 @@
end
end
end

context 'that is missing an init-only templated file' do
before(:all) do
FileUtils.rm_f('README.md')
end

describe command('pdk update --force') do
its(:exit_status) { is_expected.to eq(0) }
its(:stderr) { is_expected.to have_no_output }
its(:stdout) { is_expected.to match(%r{no changes required}i) }

describe file('README.md') do
it { is_expected.not_to be_file }
end
end
end
end
end
58 changes: 48 additions & 10 deletions spec/unit/pdk/module/convert_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@
include_context 'no changes in the summary'

before(:each) do
allow(File).to receive(:exist?).with('a/path/to/file').and_return(true)
allow(PDK::Util::Filesystem).to receive(:exist?).with('a/path/to/file').and_return(true)
allow(update_manager).to receive(:changes?).and_return(false)
allow(template_dir).to receive(:render)
allow(PDK::Module::TemplateDir).to receive(:files_in_template).and_return({})
Expand All @@ -141,7 +141,7 @@
include_context 'completes a convert'

before(:each) do
allow(File).to receive(:exist?).with('a/path/to/file').and_return(true)
allow(PDK::Util::Filesystem).to receive(:exist?).with('a/path/to/file').and_return(true)
allow(update_manager).to receive(:modify_file).with(any_args)
allow(update_manager).to receive(:changes?).and_return(true)
allow($stdout).to receive(:puts).with(['Gemfile'])
Expand Down Expand Up @@ -182,7 +182,7 @@
include_context 'completes a convert'

before(:each) do
allow(File).to receive(:exist?).with('a/path/to/file').and_return(true)
allow(PDK::Util::Filesystem).to receive(:exist?).with('a/path/to/file').and_return(true)
allow(update_manager).to receive(:modify_file).with(any_args)
allow(update_manager).to receive(:changes?).and_return(true)
allow($stdout).to receive(:puts).with(['some/file'])
Expand Down Expand Up @@ -258,6 +258,38 @@
end
end

context 'when there are init files to add' do
let(:options) { { noop: true } }
let(:template_files) do
{ path: 'a/path/to/file', content: 'file contents', status: :init }
end

context 'and the files already exist' do
include_context 'no changes in the summary'

before(:each) do
allow(PDK::Util::Filesystem).to receive(:exist?).with(template_files[:path]).and_return(true)
allow(update_manager).to receive(:changes?).and_return(false)
end

it 'does not stage the file for addition' do
expect(update_manager).not_to receive(:add_file).with(template_files[:path], anything)
end
end

context 'and the files do not exist' do
before(:each) do
allow(PDK::Util::Filesystem).to receive(:exist?).with(template_files[:path]).and_return(false)
allow(update_manager).to receive(:changes?).and_return(true)
allow(update_manager).to receive(:add_file)
end

it 'stages the file for addition' do
expect(update_manager).to receive(:add_file).with(template_files[:path], template_files[:content])
end
end
end

context 'when there are files to add' do
include_context 'has changes in the summary'
include_context 'added files in the summary'
Expand All @@ -274,7 +306,7 @@
end

before(:each) do
allow(File).to receive(:exist?).with('a/path/to/file').and_return(false)
allow(PDK::Util::Filesystem).to receive(:exist?).with('a/path/to/file').and_return(false)
allow(update_manager).to receive(:changes?).and_return(true)
allow($stdout).to receive(:puts).with(['path/to/file'])

Expand Down Expand Up @@ -344,6 +376,12 @@
end
end

describe '#convert?' do
subject { described_class.new.convert? }

it { is_expected.to be_truthy }
end

describe '#template_uri' do
subject { described_class.new(options).template_uri }

Expand Down Expand Up @@ -396,18 +434,18 @@

context 'when the metadata file exists' do
before(:each) do
allow(File).to receive(:exist?).with(metadata_path).and_return(true)
allow(PDK::Util::Filesystem).to receive(:exist?).with(metadata_path).and_return(true)
end

context 'and is a file' do
before(:each) do
allow(File).to receive(:file?).with(metadata_path).and_return(true)
allow(PDK::Util::Filesystem).to receive(:file?).with(metadata_path).and_return(true)
end

context 'and is readable' do
before(:each) do
allow(File).to receive(:readable?).with(metadata_path).and_return(true)
allow(File).to receive(:read).with(metadata_path).and_return(existing_metadata)
allow(PDK::Util::Filesystem).to receive(:readable?).with(metadata_path).and_return(true)
allow(PDK::Util::Filesystem).to receive(:read_file).with(metadata_path).and_return(existing_metadata)
end

let(:existing_metadata) do
Expand Down Expand Up @@ -453,7 +491,7 @@

context 'and is not readable' do
before(:each) do
allow(File).to receive(:readable?).with(metadata_path).and_return(false)
allow(PDK::Util::Filesystem).to receive(:readable?).with(metadata_path).and_return(false)
end

it 'exits with an error' do
Expand All @@ -466,7 +504,7 @@

context 'and is not a file' do
before(:each) do
allow(File).to receive(:file?).with(metadata_path).and_return(false)
allow(PDK::Util::Filesystem).to receive(:file?).with(metadata_path).and_return(false)
end

it 'exits with an error' do
Expand Down
24 changes: 12 additions & 12 deletions spec/unit/pdk/module/metadata_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,18 @@
end

it 'can populate itself from a metadata.json file on disk' do
allow(File).to receive(:file?).with(metadata_json_path).and_return(true)
allow(File).to receive(:readable?).with(metadata_json_path).and_return(true)
allow(PDK::Util::Filesystem).to receive(:file?).with(metadata_json_path).and_return(true)
allow(PDK::Util::Filesystem).to receive(:readable?).with(metadata_json_path).and_return(true)
allow(PDK::Util).to receive(:package_install?).and_return(false)
allow(File).to receive(:read).with(metadata_json_path).and_return(metadata_json_content)
allow(PDK::Util::Filesystem).to receive(:read_file).with(metadata_json_path).and_return(metadata_json_content)

expect(described_class.from_file(metadata_json_path).data).to include('name' => 'foo-bar', 'version' => '0.1.0')
end

it 'can populate itself from a metadata.json file on disk with a trailing newline' do
allow(File).to receive(:file?).with(metadata_json_path).and_return(true)
allow(File).to receive(:readable?).with(metadata_json_path).and_return(true)
allow(File).to receive(:read).with(metadata_json_path).and_return(metadata_json_content + "\n")
allow(PDK::Util::Filesystem).to receive(:file?).with(metadata_json_path).and_return(true)
allow(PDK::Util::Filesystem).to receive(:readable?).with(metadata_json_path).and_return(true)
allow(PDK::Util::Filesystem).to receive(:read_file).with(metadata_json_path).and_return(metadata_json_content + "\n")

expect(described_class.from_file(metadata_json_path).data).to include('name' => 'foo-bar', 'version' => '0.1.0')
end
Expand All @@ -36,21 +36,21 @@
end

it 'raises an ArgumentError if the file does not exist' do
allow(File).to receive(:file?).with(metadata_json_path).and_return(false)
allow(PDK::Util::Filesystem).to receive(:file?).with(metadata_json_path).and_return(false)
expect { described_class.from_file(metadata_json_path) }.to raise_error(ArgumentError, %r{'#{metadata_json_path}'.*not exist})
end

it 'raises an ArgumentError if the file exists but is not readable' do
allow(File).to receive(:file?).with(metadata_json_path).and_return(true)
allow(File).to receive(:readable?).with(metadata_json_path).and_return(false)
allow(PDK::Util::Filesystem).to receive(:file?).with(metadata_json_path).and_return(true)
allow(PDK::Util::Filesystem).to receive(:readable?).with(metadata_json_path).and_return(false)

expect { described_class.from_file(metadata_json_path) }.to raise_error(ArgumentError, %r{Unable to open '#{metadata_json_path}'})
end

it 'raises an ArgumentError if the file contains invalid JSON' do
allow(File).to receive(:file?).with(metadata_json_path).and_return(true)
allow(File).to receive(:readable?).with(metadata_json_path).and_return(true)
allow(File).to receive(:read).with(metadata_json_path).and_return('{"foo": }')
allow(PDK::Util::Filesystem).to receive(:file?).with(metadata_json_path).and_return(true)
allow(PDK::Util::Filesystem).to receive(:readable?).with(metadata_json_path).and_return(true)
allow(PDK::Util::Filesystem).to receive(:read_file).with(metadata_json_path).and_return('{"foo": }')

expect { described_class.from_file(metadata_json_path) }.to raise_error(ArgumentError, %r{Invalid JSON.*unexpected token})
end
Expand Down
6 changes: 6 additions & 0 deletions spec/unit/pdk/module/update_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -344,4 +344,10 @@
end
end
end

describe '#convert?' do
subject { described_class.new.convert? }

it { is_expected.to be_falsey }
end
end