Skip to content

Commit

Permalink
(PDK-547) Ensure all PDK created files use LF line endings
Browse files Browse the repository at this point in the history
  • Loading branch information
rodjek committed Nov 8, 2018
1 parent a09f37e commit 85f4570
Show file tree
Hide file tree
Showing 16 changed files with 66 additions and 33 deletions.
3 changes: 3 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ RSpec/ExampleLength:
RSpec/MessageSpies:
EnforcedStyle: receive

RSpec/ScatteredSetup:
Enabled: False

# Style Cops
Style/AsciiComments:
Description: Names, non-english speaking communities.
Expand Down
7 changes: 4 additions & 3 deletions lib/pdk/answer_file.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'json'
require 'pdk/util/filesystem'

module PDK
# Singleton accessor to the current answer file being used by the PDK.
Expand All @@ -21,6 +22,8 @@ class AnswerFile
attr_reader :answers
attr_reader :answer_file_path

include PDK::Util::Filesystem

# Initialises the AnswerFile object, which stores the responses to certain
# interactive questions.
#
Expand Down Expand Up @@ -107,9 +110,7 @@ def read_from_disk
def save_to_disk
FileUtils.mkdir_p(File.dirname(answer_file_path))

File.open(answer_file_path, 'w') do |answer_file|
answer_file.puts JSON.pretty_generate(answers)
end
write_file(answer_file_path, JSON.pretty_generate(answers))
rescue SystemCallError, IOError => e
raise PDK::CLI::FatalError, _("Unable to write '%{file}': %{msg}") % {
file: answer_file_path,
Expand Down
6 changes: 4 additions & 2 deletions lib/pdk/generate/module.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
module PDK
module Generate
class Module
extend PDK::Util::Filesystem

def self.validate_options(opts)
unless PDK::CLI::Util::OptionValidator.valid_module_name?(opts[:module_name])
error_msg = _(
Expand All @@ -40,7 +42,7 @@ def self.invoke(opts = {})

begin
test_file = File.join(parent_dir, '.pdk-test-writable')
File.open(test_file, 'w') { |f| f.write('This file was created by the Puppet Development Kit to test if this folder was writable, you can safely remove this file.') }
write_file(test_file, 'This file was created by the Puppet Development Kit to test if this folder was writable, you can safely remove this file.')
FileUtils.rm_f(test_file)
rescue Errno::EACCES
raise PDK::CLI::FatalError, _("You do not have permission to write to '%{parent_dir}'") % {
Expand All @@ -59,7 +61,7 @@ def self.invoke(opts = {})
templates.render do |file_path, file_content|
file = Pathname.new(temp_target_dir) + file_path
file.dirname.mkpath
file.write(file_content)
write_file(file, file_content)
end

# Add information about the template used to generate the module to the
Expand Down
3 changes: 2 additions & 1 deletion lib/pdk/generate/puppet_object.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
require 'pdk/module/metadata'
require 'pdk/module/templatedir'
require 'pdk/template_file'
require 'pdk/util/filesystem'

module PDK
module Generate
Expand Down Expand Up @@ -180,7 +181,7 @@ def write_file(dest_path)
}
end

File.open(dest_path, 'w') { |f| f.write file_content }
PDK::Util::Filesystem.write_file(dest_path, file_content)
rescue SystemCallError => e
raise PDK::CLI::FatalError, _("Unable to write to file '%{path}': %{message}") % {
path: dest_path,
Expand Down
7 changes: 4 additions & 3 deletions lib/pdk/module/metadata.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
require 'json'
require 'pdk/util/filesystem'

module PDK
module Module
class Metadata
attr_accessor :data

include PDK::Util::Filesystem

OPERATING_SYSTEMS = {
'RedHat based Linux' => [
{
Expand Down Expand Up @@ -120,9 +123,7 @@ def to_json
end

def write!(path)
File.open(path, 'w') do |file|
file.puts to_json
end
write_file(path, to_json)
end

def forge_ready?
Expand Down
3 changes: 2 additions & 1 deletion lib/pdk/module/update_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
require 'English'
require 'fileutils'
require 'set'
require 'pdk/util/filesystem'

module PDK
module Module
Expand Down Expand Up @@ -148,7 +149,7 @@ def calculate_diffs
def write_file(path, content)
FileUtils.mkdir_p(File.dirname(path))
PDK.logger.debug(_("writing '%{path}'") % { path: path })
File.open(path, 'w') { |f| f.puts content }
PDK::Util::Filesystem.write_file(path, content)
rescue Errno::EACCES
raise PDK::CLI::ExitWithError, _("You do not have permission to write to '%{path}'") % { path: path }
end
Expand Down
1 change: 1 addition & 0 deletions lib/pdk/util.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
require 'pdk/util/version'
require 'pdk/util/windows'
require 'pdk/util/vendored_file'
require 'pdk/util/filesystem'

module PDK
module Util
Expand Down
12 changes: 12 additions & 0 deletions lib/pdk/util/filesystem.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
module PDK
module Util
module Filesystem
def write_file(path, content)
raise ArgumentError unless path.is_a?(String) || path.respond_to?(:to_path)

File.open(path, 'wb') { |f| f.write(content.encode(universal_newline: true)) }
end
module_function :write_file
end
end
end
7 changes: 4 additions & 3 deletions lib/pdk/util/vendored_file.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
require 'net/https'
require 'openssl'
require 'fileutils'
require 'pdk/util/filesystem'

module PDK
module Util
Expand All @@ -22,6 +23,8 @@ class DownloadError < StandardError; end
attr_reader :file_name
attr_reader :url

include PDK::Util::Filesystem

def initialize(file_name, url)
@file_name = file_name
@url = url
Expand All @@ -36,9 +39,7 @@ def read
# TODO: should only write if it's valid JSON
# TODO: need a way to invalidate if out of date
FileUtils.mkdir_p(File.dirname(gem_vendored_path))
File.open(gem_vendored_path, 'w') do |fd|
fd.write(content)
end
write_file(gem_vendored_path, content)
content
end

Expand Down
5 changes: 5 additions & 0 deletions spec/acceptance/new_module_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,10 @@
it { is_expected.to be_file }
it { is_expected.to contain(%r{## Release 0.1.0}i) }
end

eol_check = '(Get-Content .\foo\spec\spec_helper.rb -Delimiter [String].Empty) -Match "`r`n"'
describe command(eol_check), if: Gem.win_platform? do
its(:stdout) { is_expected.to match(%r{\AFalse$}) }
end
end
end
6 changes: 3 additions & 3 deletions spec/unit/pdk/answer_file_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@
let(:fake_file) { StringIO.new }

before(:each) do
allow(File).to receive(:open).with(default_path, 'w').and_yield(fake_file)
allow(File).to receive(:open).with(default_path, 'wb').and_yield(fake_file)
end

it 'writes the answer set to disk' do
Expand All @@ -185,7 +185,7 @@
context 'when an IOError is raised' do
before(:each) do
allow(File).to receive(:open).with(any_args).and_call_original
allow(File).to receive(:open).with(default_path, 'w').and_raise(IOError, 'some error message')
allow(File).to receive(:open).with(default_path, 'wb').and_raise(IOError, 'some error message')
end

it 'raises a FatalError' do
Expand All @@ -197,7 +197,7 @@
context 'when a SystemCallError is raised' do
before(:each) do
allow(File).to receive(:open).with(any_args).and_call_original
allow(File).to receive(:open).with(default_path, 'w').and_raise(SystemCallError, 'some other error')
allow(File).to receive(:open).with(default_path, 'wb').and_raise(SystemCallError, 'some other error')
end

it 'raises a FatalError' do
Expand Down
21 changes: 13 additions & 8 deletions spec/unit/pdk/generate/module_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,24 +31,28 @@
before(:each) do
allow(PDK::Module::TemplateDir).to receive(:new).with(anything, anything, anything).and_yield(test_template_dir)

dir_double = instance_double(Pathname, mkpath: true)
allow(dir_double).to receive(:+).with(anything).and_return(test_template_file)
allow(test_template_file).to receive(:dirname).and_return(dir_double)
dir_double = instance_double(Pathname, mkpath: true, to_path: '/a/path')
allow(dir_double).to receive(:+).with(anything).and_return(dir_double)
allow(dir_double).to receive(:dirname).and_return(dir_double)
allow(Pathname).to receive(:new).with(temp_target_dir).and_return(dir_double)
allow(File).to receive(:open).with(dir_double, 'wb').and_yield(test_template_file)
end
end

shared_context 'mock metadata.json' do
let(:metadata_json) { StringIO.new }

before(:each) do
allow(File).to receive(:open).with(any_args).and_call_original
allow(File).to receive(:open).with(a_string_matching(%r{metadata\.json\Z}), 'w').and_yield(metadata_json)
allow(File).to receive(:open).with(a_string_matching(%r{metadata\.json\Z}), 'wb').and_yield(metadata_json)
end
end

describe PDK::Generate::Module do
describe '.invoke' do
before(:each) do
allow(File).to receive(:open).with(any_args).and_call_original
end

let(:target_dir) { File.expand_path('/path/to/target/module') }
let(:invoke_opts) do
{
Expand All @@ -59,11 +63,12 @@
end

context 'when the target module directory already exists' do
before(:each) do
allow(File).to receive(:exist?).with(target_dir).and_return(true)
end
# before(:each) do
# allow(File).to receive(:exist?).with(target_dir).and_return(true)
# end

it 'raises a FatalError' do
allow(File).to receive(:exist?).with(target_dir).and_return(true)
expect(logger).not_to receive(:info).with(a_string_matching(%r{generated at path}i))
expect(logger).not_to receive(:info).with(a_string_matching(%r{In your new module directory, add classes with the 'pdk new class' command}i))

Expand Down
4 changes: 2 additions & 2 deletions spec/unit/pdk/generate/puppet_object_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@
allow(logger).to receive(:info).with(a_string_matching(%r{creating '#{dest_path}' from template}i))
allow(PDK::TemplateFile).to receive(:new).with(template_path, template_data).and_return(template_file)
allow(File).to receive(:open).with(any_args).and_call_original
allow(File).to receive(:open).with(dest_path, 'w').and_yield(rendered_file)
allow(File).to receive(:open).with(dest_path, 'wb').and_yield(rendered_file)
end

it 'creates the parent directories for the destination path if needed' do
Expand Down Expand Up @@ -112,7 +112,7 @@

context 'when it fails to write the destination file' do
before(:each) do
allow(File).to receive(:open).with(dest_path, 'w').and_raise(SystemCallError, 'some message')
allow(File).to receive(:open).with(dest_path, 'wb').and_raise(SystemCallError, 'some message')
allow(FileUtils).to receive(:mkdir_p).with(dest_dir)
end

Expand Down
2 changes: 1 addition & 1 deletion spec/unit/pdk/generate/task_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@

before(:each) do
metadata_file = File.join(module_dir, 'tasks', "#{given_name}.json")
allow(File).to receive(:open).with(metadata_file, 'w').and_yield(mock_file)
allow(File).to receive(:open).with(metadata_file, 'wb').and_yield(mock_file)
generator.write_task_metadata
mock_file.rewind
end
Expand Down
10 changes: 5 additions & 5 deletions spec/unit/pdk/module/update_manager_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@

before(:each) do
allow(File).to receive(:open).with(any_args).and_call_original
allow(File).to receive(:open).with(dummy_file, 'w').and_yield(dummy_file_io)
allow(File).to receive(:open).with(dummy_file, 'wb').and_yield(dummy_file_io)
update_manager.sync_changes!
dummy_file_io.rewind
end
Expand All @@ -47,7 +47,7 @@

context 'but if the file can not be written to' do
before(:each) do
allow(File).to receive(:open).with(dummy_file, 'w').and_raise(Errno::EACCES)
allow(File).to receive(:open).with(dummy_file, 'wb').and_raise(Errno::EACCES)
end

it 'exits with an error' do
Expand Down Expand Up @@ -208,7 +208,7 @@

before(:each) do
allow(File).to receive(:open).with(any_args).and_call_original
allow(File).to receive(:open).with(dummy_file, 'w').and_yield(dummy_file_io)
allow(File).to receive(:open).with(dummy_file, 'wb').and_yield(dummy_file_io)
update_manager.sync_changes!
dummy_file_io.rewind
end
Expand All @@ -219,7 +219,7 @@

context 'but if the file can not be written to' do
before(:each) do
allow(File).to receive(:open).with(dummy_file, 'w').and_raise(Errno::EACCES)
allow(File).to receive(:open).with(dummy_file, 'wb').and_raise(Errno::EACCES)
end

it 'exits with an error' do
Expand Down Expand Up @@ -250,7 +250,7 @@

context 'when syncing the changes' do
it 'does not modify the file' do
expect(File).not_to receive(:open).with(dummy_file, 'w')
expect(File).not_to receive(:open).with(dummy_file, 'wb')
update_manager.sync_changes!
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/unit/pdk/util/vendored_file_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@
allow(mock_response).to receive(:body).and_return(gem_vendored_content)
allow(FileUtils).to receive(:mkdir_p).with(File.dirname(gem_vendored_path))
allow(File).to receive(:open).with(any_args).and_call_original
allow(File).to receive(:open).with(gem_vendored_path, 'w').and_yield(cached_file)
allow(File).to receive(:open).with(gem_vendored_path, 'wb').and_yield(cached_file)
end

let(:cached_file) { StringIO.new }
Expand Down

0 comments on commit 85f4570

Please sign in to comment.