(PDK-1308) Ensure PDK-written non-templated files have trailing newline

lib/pdk/util/filesystem.rb

@@ -4,7 +4,13 @@ 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)) }
+        # Harmonize newlines across platforms.
+        content = content.encode(universal_newline: true)
+
+        # Make sure all written files have a trailing newline.
+        content += "\n" unless content[-1] == "\n"
+
+        File.open(path, 'wb') { |f| f.write(content) }
       end
       module_function :write_file
     end

spec/unit/pdk/generate/module_spec.rb

@@ -117,15 +117,17 @@
       end
 
       context 'when the module template contains template files' do
+        let(:content) { 'test_file_content' }
+
         before(:each) do
-          allow(test_template_dir).to receive(:render).and_yield('test_file_path', 'test_file_content')
+          allow(test_template_dir).to receive(:render).and_yield('test_file_path', content)
         end
 
         it 'writes the rendered files from the template to the temporary directory' do
           described_class.invoke(invoke_opts)
 
           test_template_file.rewind
-          expect(test_template_file.read).to eq('test_file_content')
+          expect(test_template_file.read).to eq(content + "\n")
         end
       end
 

spec/unit/pdk/generate/puppet_object_spec.rb

@@ -76,7 +76,8 @@
     let(:dest_dir) { File.dirname(dest_path) }
     let(:template_path) { '/path/to/file/template' }
     let(:template_data) { { some: 'data', that: 'the', template: 'needs' } }
-    let(:template_file) { instance_double(PDK::TemplateFile, render: 'rendered file content') }
+    let(:template_content) { 'rendered file content' }
+    let(:template_file) { instance_double(PDK::TemplateFile, render: template_content) }
     let(:rendered_file) { StringIO.new }
 
     before(:each) do
@@ -95,7 +96,7 @@
       allow(FileUtils).to receive(:mkdir_p).with(dest_dir)
       templated_object.render_file(dest_path, template_path, template_data)
       rendered_file.rewind
-      expect(rendered_file.read).to eq('rendered file content')
+      expect(rendered_file.read).to eq(template_content + "\n")
     end
 
     context 'when it fails to create the parent directories' do

spec/unit/pdk/module/metadata_spec.rb

@@ -18,6 +18,14 @@
       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")
+
+      expect(described_class.from_file(metadata_json_path).data).to include('name' => 'foo-bar', 'version' => '0.1.0')
+    end
+
     it 'raises an ArgumentError if passed nil' do
       expect { described_class.from_file(nil) }.to raise_error(ArgumentError, %r{no path to file}i)
     end
@@ -216,4 +224,31 @@
       end
     end
   end
+
+  describe '#write!' do
+    let(:metadata_json_path) { '/tmp/metadata.json' }
+
+    let(:metadata) do
+      described_class.new.update!(
+        'name' => 'foo-bar',
+        'version' => '0.1.0',
+      )
+    end
+
+    let(:mock_file) { instance_double(File) }
+
+    before(:each) do
+      allow(File).to receive(:open).and_call_original
+      allow(File).to receive(:open).with(metadata_json_path, anything).and_yield(mock_file)
+    end
+
+    it 'writes metadata to disk with a trailing newline' do
+      # This is slightly roundabout but results in a much clearer failure description.
+      expect(mock_file).to receive(:write) do |content|
+        expect(content[-1]).to eq("\n")
+      end
+
+      metadata.write!(metadata_json_path)
+    end
+  end
 end

spec/unit/pdk/util/vendored_file_spec.rb

@@ -77,7 +77,7 @@
           it 'caches the download to disk' do
             vendored_file_read
 
-            expect(cached_file.string).to eq(gem_vendored_content)
+            expect(cached_file.string).to eq(gem_vendored_content + "\n")
           end
 
           it 'returns the downloaded content' do