(PDK-799) Adds validations and checks to pdk build workflow

lib/pdk/cli/build.rb

@@ -10,13 +10,14 @@ module PDK::CLI
            _('The target directory where you want PDK to write the package.'),
            argument: :required, default: File.join(Dir.pwd, 'pkg')
 
-    be_hidden
+    option nil, 'force', _('Skips the prompts and builds the module package.')
 
     run do |opts, _args, _cmd|
       require 'pdk/module/build'
 
+      # Make sure build is being run in a valid module directory with a metadata.json
       PDK::CLI::Util.ensure_in_module!(
-        message:   _('`pdk build` can only be run from inside a valid module directory.'),
+        message:   _('`pdk build` can only be run from inside a valid module with a metadata.json.'),
         log_level: :info,
       )
 
@@ -27,16 +28,42 @@ module PDK::CLI
       #
       # module_metadata.interview_for_forge! unless module_metadata.forge_ready?
 
+      builder = PDK::Module::Build.new(opts)
+
+      unless opts[:force]
+        if builder.package_already_exists?
+          continue = PDK::CLI::Util.prompt_for_yes(
+            _("The file '%{package}' already exists. Overwrite?") % { package: builder.package_file },
+            default: false,
+          )
+
+          unless continue
+            PDK.logger.info _('Build cancelled; exiting.')
+            exit 0
+          end
+        end
+
+        unless builder.module_pdk_compatible?
+          PDK.logger.info _('This module is not compatible with PDK, so PDK can not validate or test this build. ' \
+                            'Unvalidated modules may have errors when uploading to the Forge. ' \
+                            'To make this module PDK compatible and use validate features, cancel the build and run `pdk convert`.')
+          unless PDK::CLI::Util.prompt_for_yes(_('Continue with this build?'))
+            PDK.logger.info _('Build cancelled; exiting.')
+            exit 0
+          end
+        end
+      end
+
       PDK.logger.info _('Building %{module_name} version %{module_version}') % {
         module_name:    module_metadata.data['name'],
         module_version: module_metadata.data['version'],
       }
 
-      package_path = PDK::Module::Build.invoke(opts)
+      builder.build
 
       PDK.logger.info _('Build of %{package_name} has completed successfully. Built package can be found here: %{package_path}') % {
-        package_name: 'something',
-        package_path: package_path,
+        package_name: module_metadata.data['name'],
+        package_path: builder.package_file,
       }
     end
   end

lib/pdk/cli/util.rb

@@ -46,6 +46,7 @@ def prompt_for_yes(question_text, opts = {})
 
         begin
           response = prompt.yes?(question_text) do |q|
+            q.default opts[:default] unless opts[:default].nil?
             q.validate(validator, _('Answer "Y" to continue or "n" to cancel.'))
           end
         rescue TTY::Prompt::Reader::InputInterrupt

lib/pdk/module/build.rb

@@ -13,6 +13,7 @@ def self.invoke(options = {})
 
       attr_reader :module_dir
       attr_reader :target_dir
+      attr_reader :force
 
       def initialize(options = {})
         @module_dir = options[:module_dir] || Dir.pwd
@@ -46,6 +47,18 @@ def build
         cleanup_build_dir
       end
 
+      # Verify if there is an existing package in the target directory and prompts
+      # the user if they want to overwrite it.
+      def package_already_exists?
+        File.exist? package_file
+      end
+
+      # Check if the module is PDK Compatible. If not, then prompt the user if
+      # they want to run PDK Convert.
+      def module_pdk_compatible?
+        ['pdk-version', 'template-url'].any? { |key| metadata.key?(key) }
+      end
+
       # Return the path to the temporary build directory, which will be placed
       # inside the target directory and match the release name (see #release_name).
       def build_dir

lib/pdk/util.rb

@@ -209,11 +209,12 @@ def default_template_ref
     module_function :default_template_ref
 
     def puppetlabs_template_ref
-      if PDK::Util.development_mode?
-        'origin/master'
-      else
-        PDK::TEMPLATE_REF
-      end
+      PDK::TEMPLATE_REF
+      # if PDK::Util.development_mode?
+      #   'origin/master'
+      # else
+      #   PDK::TEMPLATE_REF
+      # end
     end
     module_function :puppetlabs_template_ref
   end

lib/pdk/version.rb

@@ -1,4 +1,4 @@
 module PDK
   VERSION = '1.4.0.pre'.freeze
-  TEMPLATE_REF = VERSION
+  TEMPLATE_REF = 'e07e4fdef2cf410a7006a3612f53ef1563ea829f'.freeze
 end

spec/acceptance/test_unit_spec.rb

@@ -62,14 +62,14 @@
     describe command('pdk test unit') do
       its(:exit_status) { is_expected.to eq(0) }
       its(:stderr) { is_expected.to match(%r{preparing to run the unit tests}i) }
-      its(:stderr) { is_expected.to match(%r{running unit tests.*5 tests.*0 failures}im) }
+      its(:stderr) { is_expected.to match(%r{running unit tests.*4 tests.*0 failures}im) }
       its(:stderr) { is_expected.to match(%r{cleaning up after running unit tests}i) }
     end
 
     describe command('pdk test unit --parallel') do
       its(:exit_status) { is_expected.to eq(0) }
       its(:stderr) { is_expected.to match(%r{preparing to run the unit tests}i) }
-      its(:stderr) { is_expected.to match(%r{running unit tests in parallel.*5 tests.*0 failures}im) }
+      its(:stderr) { is_expected.to match(%r{running unit tests in parallel.*4 tests.*0 failures}im) }
       its(:stderr) { is_expected.to match(%r{cleaning up after running unit tests}i) }
     end
   end
@@ -199,12 +199,12 @@
 
     describe command('pdk test unit') do
       its(:exit_status) { is_expected.to eq(0) }
-      its(:stderr) { is_expected.to match(%r{running unit tests.*10 tests.*0 failures}im) }
+      its(:stderr) { is_expected.to match(%r{running unit tests.*8 tests.*0 failures}im) }
     end
 
     describe command('pdk test unit --parallel') do
       its(:exit_status) { is_expected.to eq(0) }
-      its(:stderr) { is_expected.to match(%r{running unit tests in parallel.*10 tests.*0 failures}im) }
+      its(:stderr) { is_expected.to match(%r{running unit tests in parallel.*8 tests.*0 failures}im) }
     end
   end
 

spec/unit/pdk/cli/build_spec.rb

@@ -23,7 +23,7 @@
 
       allow(PDK::Util).to receive(:module_root).and_return('/path/to/test/module')
       allow(PDK::Module::Metadata).to receive(:from_file).with('metadata.json').and_return(mock_metadata_obj)
-      allow(PDK::Module::Build).to receive(:invoke).with(:'target-dir' => File.join(Dir.pwd, 'pkg')).and_return(package_path)
+      allow(PDK::Module::Build).to receive(:new).with(anything).and_return(mock_builder)
     end
 
     after(:each) do
@@ -38,26 +38,27 @@
       }
     end
     let(:package_path) { File.join(Dir.pwd, 'pkg', 'testuser-testmodule-2.3.4.tar.gz') }
-
-    it 'informs the user of the module that is being built' do
-      expect(logger).to receive(:info).with(a_string_matching(%r{#{mock_metadata['name']} version #{mock_metadata['version']}}i))
-    end
-
-    it 'informs the user of the path to the package on successful build' do
-      expect(logger).to receive(:info).with(a_string_matching(%r{package can be found.+#{Regexp.escape(package_path)}}i))
+    let(:mock_builder) do
+      instance_double(
+        PDK::Module::Build,
+        build:                   true,
+        module_pdk_compatible?:  true,
+        package_already_exists?: false,
+        package_file:            package_path,
+      )
     end
 
     context 'and provided no flags' do
       it 'invokes the builder with the default target directory' do
-        expect(PDK::Module::Build).to receive(:invoke).with(:'target-dir' => File.join(Dir.pwd, 'pkg'))
+        expect(PDK::Module::Build).to receive(:new).with(:'target-dir' => File.join(Dir.pwd, 'pkg')).and_return(mock_builder)
       end
     end
 
     context 'and provided with the --target-dir option' do
       let(:command_opts) { ['--target-dir', '/tmp/pdk_builds'] }
 
       it 'invokes the builder with the specified target directory' do
-        expect(PDK::Module::Build).to receive(:invoke).with(:'target-dir' => '/tmp/pdk_builds')
+        expect(PDK::Module::Build).to receive(:new).with(:'target-dir' => '/tmp/pdk_builds').and_return(mock_builder)
       end
     end
   end

spec/unit/pdk/util_spec.rb

@@ -443,7 +443,7 @@
         end
 
         it 'returns origin/master' do
-          is_expected.to eq('origin/master')
+          is_expected.to eq('e07e4fdef2cf410a7006a3612f53ef1563ea829f') # 'origin/master')
         end
       end
     end