(PDK-1047) Add --add-tests to pdk convert

lib/pdk/cli/convert.rb

@@ -12,6 +12,7 @@ module PDK::CLI
     PDK::CLI.full_interview_option(self)
     flag nil, :noop, _('Do not convert the module, just output what would be done.')
     flag nil, :force, _('Convert the module automatically, with no prompts.')
+    flag nil, :'add-tests', _('Add any missing tests while converting the module.')
 
     run do |opts, _args, _cmd|
       require 'pdk/module/convert'

lib/pdk/generate/puppet_object.rb

@@ -151,6 +151,18 @@ def check_preconditions
         end
       end
 
+      # Check the preconditions of this template group, behaving as a
+      # predicate rather than raising an exception.
+      #
+      # @return [Boolean] true if the generator is safe to run, otherwise
+      #   false.
+      def can_run?
+        check_preconditions
+        true
+      rescue PDK::CLI::ExitWithError
+        false
+      end
+
       # Check that the templates can be rendered. Find an appropriate template
       # and create the target files from the template. This is the main entry
       # point for the class.
@@ -299,28 +311,13 @@ def templates
       # Retrieves the name of the module (without the forge username) from the
       # module metadata.
       #
-      # @raise (see #module_metadata)
       # @return [String] The name of the module.
       #
       # @api private
       def module_name
-        @module_name ||= module_metadata.data['name'].rpartition('-').last
-      end
-
-      # Parses the metadata.json file for the module.
-      #
-      # @raise [PDK::CLI::FatalError] if the metadata.json file does not exist,
-      #   can not be read, or contains invalid metadata.
-      #
-      # @return [PDK::Module::Metadata] the parsed module metadata.
-      #
-      # @api private
-      def module_metadata
-        @module_metadata ||= begin
-          PDK::Module::Metadata.from_file(File.join(module_dir, 'metadata.json'))
-        rescue ArgumentError => e
-          raise PDK::CLI::FatalError, _("'%{dir}' does not contain valid Puppet module metadata: %{msg}") % { dir: module_dir, msg: e.message }
-        end
+        @module_name ||= PDK::Util.module_metadata['name'].rpartition('-').last
+      rescue ArgumentError => e
+        raise PDK::CLI::FatalError, e
       end
 
       # transform a object name into a ruby class name

lib/pdk/module/convert.rb

@@ -24,7 +24,12 @@ def run
         stage_changes!
 
         unless update_manager.changes?
-          PDK::Report.default_target.puts(_('No changes required.'))
+          if adding_tests?
+            add_tests!
+          else
+            PDK::Report.default_target.puts(_('No changes required.'))
+          end
+
           return
         end
 
@@ -54,6 +59,8 @@ def run
 
         PDK::Util::Bundler.ensure_bundle! if needs_bundle_update?
 
+        add_tests! if adding_tests?
+
         print_result 'Convert completed'
       end
 
@@ -65,6 +72,36 @@ def force?
         options[:force]
       end
 
+      def add_tests?
+        options[:'add-tests']
+      end
+
+      def adding_tests?
+        add_tests? && missing_tests?
+      end
+
+      def missing_tests?
+        test_generators.any? { |gen| gen.can_run? }
+      end
+
+      def test_generators
+        return @test_generators unless @test_generators.nil?
+
+        test_gens = PDK::Util::PuppetStrings.all_objects.map do |generator, objects|
+          (objects || []).map do |obj|
+            generator.new(Dir.pwd, obj['name'], spec_only: true)
+          end
+        end
+
+        @test_generators = test_gens.flatten
+      end
+
+      def add_tests!
+        test_generators.each do |gen|
+          gen.run if gen.can_run?
+        end
+      end
+
       def needs_bundle_update?
         update_manager.changed?('Gemfile')
       end

lib/pdk/util/puppet_strings.rb

@@ -80,6 +80,24 @@ def self.find_object(name)
         [generator, known_objects[object_type].find { |obj| object_names.include?(obj['name']) }]
       end
 
+      # Generate a list of all objects that PDK has a generator for.
+      #
+      # @returns [Array[Array[PDK::Generate::PuppetObject,
+      #   Array[Hash{String=>Object}]]]] an associative array where the first
+      #   element of each pair is the generator class and the second element of
+      #   each pair is an array of description hashes from puppet-strings.
+      def self.all_objects
+        generators = PDK::Generate::GENERATORS.select do |gen|
+          gen.respond_to?(:puppet_strings_type) && !gen.puppet_strings_type.nil?
+        end
+
+        known_objects = generate_hash
+
+        generators.map { |gen| [gen, known_objects[gen.puppet_strings_type]] }.reject do |_, obj|
+          obj.nil? || obj.empty?
+        end
+      end
+
       # Evaluates the mapping of puppet-strings object type to PDK generator
       # class.
       #

spec/acceptance/convert_spec.rb

@@ -152,4 +152,80 @@
       end
     end
   end
+
+  context 'when adding missing tests' do
+    # A real bare bones modules here. Testing to ensure that the functionality
+    # works even when the module didn't have puppet-strings installed before
+    # the convert.
+
+    context 'to a module with no missing tests' do
+      before(:all) do
+        FileUtils.mkdir('module_with_all_tests')
+        Dir.chdir('module_with_all_tests')
+
+        FileUtils.mkdir_p('manifests')
+        FileUtils.mkdir_p(File.join('spec', 'classes'))
+
+        File.open(File.join('manifests', 'some_class.pp'), 'wb') do |f|
+          f.puts 'class module_with_all_tests::some_class { }'
+        end
+
+        File.open(File.join('spec', 'classes', 'some_class_spec.rb'), 'wb') do |f|
+          f.puts "require 'spec_helper'"
+          f.puts "describe 'module_with_all_tests::some_class' do"
+          f.puts 'end'
+        end
+      end
+
+      after(:all) do
+        Dir.chdir('..')
+        FileUtils.rm_rf('module_with_all_tests')
+      end
+
+      describe command("#{pdk_convert_base} --force --skip-interview --add-tests") do
+        its(:exit_status) { is_expected.to eq(0) }
+        its(:stderr) { is_expected.not_to match(%r{some_class_spec\.rb}m) }
+      end
+    end
+
+    context 'to a module with missing tests' do
+      before(:all) do
+        FileUtils.mkdir('module_with_missing_tests')
+        Dir.chdir('module_with_missing_tests')
+
+        FileUtils.mkdir_p(File.join('manifests', 'namespaced'))
+
+        File.open(File.join('manifests', 'some_class.pp'), 'wb') do |f|
+          f.puts 'class module_with_missing_tests::some_class { }'
+        end
+        File.open(File.join('manifests', 'namespaced', 'some_define.pp'), 'wb') do |f|
+          f.puts 'define module_with_missing_tests::namespaced::some_define() { }'
+        end
+      end
+
+      after(:all) do
+        Dir.chdir('..')
+        FileUtils.rm_rf('module_with_missing_tests')
+      end
+
+      class_path = File.join('spec', 'classes', 'some_class_spec.rb')
+      define_path = File.join('spec', 'defines', 'namespaced', 'some_define_spec.rb')
+
+      describe command("#{pdk_convert_base} --force --skip-interview --add-tests") do
+        its(:exit_status) { is_expected.to eq(0) }
+        its(:stderr) { is_expected.to match(%r{#{Regexp.escape(class_path)}}m) }
+        its(:stderr) { is_expected.to match(%r{#{Regexp.escape(define_path)}}m) }
+
+        describe file(class_path) do
+          it { is_expected.to be_file }
+          its(:content) { is_expected.to match(%r{describe 'module_with_missing_tests::some_class'}m) }
+        end
+
+        describe file(define_path) do
+          it { is_expected.to be_file }
+          its(:content) { is_expected.to match(%r{describe 'module_with_missing_tests::namespaced::some_define'}m) }
+        end
+      end
+    end
+  end
 end

spec/unit/pdk/generate/defined_type_spec.rb

@@ -15,8 +15,8 @@
   let(:expected_name) { given_name }
 
   before(:each) do
-    test_metadata = instance_double(PDK::Module::Metadata, data: { 'name' => module_name })
-    allow(PDK::Module::Metadata).to receive(:from_file).with(File.join(module_dir, 'metadata.json')).and_return(test_metadata)
+    test_metadata = { 'name' => module_name }
+    allow(PDK::Util).to receive(:module_metadata).and_return(test_metadata)
   end
 
   shared_examples 'it generates the template data' do

spec/unit/pdk/generate/puppet_class_spec.rb

@@ -15,8 +15,8 @@
   let(:expected_class_name) { given_class_name }
 
   before(:each) do
-    test_metadata = instance_double(PDK::Module::Metadata, data: { 'name' => module_name })
-    allow(PDK::Module::Metadata).to receive(:from_file).with(File.join(module_dir, 'metadata.json')).and_return(test_metadata)
+    test_metadata = { 'name' => module_name }
+    allow(PDK::Util).to receive(:module_metadata).and_return(test_metadata)
   end
 
   context 'when the class name is the same as the module name' do

spec/unit/pdk/generate/puppet_object_spec.rb

@@ -87,8 +87,8 @@
 
       it 'raises a fatal error' do
         expect {
-          templated_object.module_metadata
-        }.to raise_error(PDK::CLI::FatalError, %r{'#{module_dir}'.*not contain.*metadata})
+          templated_object.module_name
+        }.to raise_error(PDK::CLI::FatalError, %r{'#{metadata_path}'.*not exist})
       end
     end
   end
@@ -235,6 +235,78 @@
     end
   end
 
+  describe '#can_run?' do
+    subject { templated_object.can_run? }
+
+    include_context :with_puppet_object_module_metadata
+
+    let(:target_object_path) { '/tmp/test_module/object_file' }
+    let(:target_spec_path) { '/tmp/test_module/spec_file' }
+    let(:target_object_exists) { false }
+    let(:target_spec_exists) { false }
+
+    before(:each) do
+      allow(File).to receive(:exist?).with(target_object_path).and_return(target_object_exists)
+      allow(File).to receive(:exist?).with(target_spec_path).and_return(target_spec_exists)
+      allow(templated_object).to receive(:target_object_path).and_return(target_object_path)
+      allow(templated_object).to receive(:target_spec_path).and_return(target_spec_path)
+    end
+
+    context 'when :spec_only => false' do
+      let(:options) { { spec_only: false } }
+
+      context 'and the target object exists' do
+        let(:target_object_exists) { true }
+
+        it { is_expected.to be_falsey }
+      end
+
+      context 'and the target spec exists' do
+        let(:target_spec_exists) { true }
+
+        it { is_expected.to be_falsey }
+      end
+
+      context 'and both target object and spec exist' do
+        let(:target_object_exists) { true }
+        let(:target_spec_exists) { true }
+
+        it { is_expected.to be_falsey }
+      end
+
+      context 'and neither target object nor spec exist' do
+        it { is_expected.to be_truthy }
+      end
+    end
+
+    context 'when :spec_only => true' do
+      let(:options) { { spec_only: true } }
+
+      context 'and the target object exists' do
+        let(:target_object_exists) { true }
+
+        it { is_expected.to be_truthy }
+      end
+
+      context 'and the target spec exists' do
+        let(:target_spec_exists) { true }
+
+        it { is_expected.to be_falsey }
+      end
+
+      context 'and both target object and spec exist' do
+        let(:target_object_exists) { true }
+        let(:target_spec_exists) { true }
+
+        it { is_expected.to be_falsey }
+      end
+
+      context 'and neither target object nor spec exist' do
+        it { is_expected.to be_truthy }
+      end
+    end
+  end
+
   describe '#run' do
     include_context :with_puppet_object_module_metadata
 

spec/unit/pdk/generate/task_spec.rb

@@ -13,8 +13,8 @@
   let(:expected_name) { given_name }
 
   before(:each) do
-    test_metadata = instance_double(PDK::Module::Metadata, data: { 'name' => module_name })
-    allow(PDK::Module::Metadata).to receive(:from_file).with(File.join(module_dir, 'metadata.json')).and_return(test_metadata)
+    test_metadata = { 'name' => module_name }
+    allow(PDK::Util).to receive(:module_metadata).and_return(test_metadata)
   end
 
   describe '#target_object_path' do