From 0a8a7b467d8564bc2087a930c2f186ab480160ce Mon Sep 17 00:00:00 2001
From: Tim Sharpe <tim@sharpe.id.au>
Date: Tue, 4 Jul 2017 18:11:08 +1000
Subject: [PATCH 1/5] (SDK-305) Initial implementation of an answer file

---
 lib/pdk.rb                   |   1 +
 lib/pdk/answer_file.rb       | 112 +++++++++++++++++++
 spec/pdk/answer_file_spec.rb | 206 +++++++++++++++++++++++++++++++++++
 3 files changed, 319 insertions(+)
 create mode 100644 lib/pdk/answer_file.rb
 create mode 100644 spec/pdk/answer_file_spec.rb

diff --git a/lib/pdk.rb b/lib/pdk.rb
index 76fcdc811..77e30ab79 100644
--- a/lib/pdk.rb
+++ b/lib/pdk.rb
@@ -1,3 +1,4 @@
+require 'pdk/answer_file'
 require 'pdk/generate'
 require 'pdk/i18n'
 require 'pdk/logger'
diff --git a/lib/pdk/answer_file.rb b/lib/pdk/answer_file.rb
new file mode 100644
index 000000000..8e9cbc90b
--- /dev/null
+++ b/lib/pdk/answer_file.rb
@@ -0,0 +1,112 @@
+require 'json'
+
+module PDK
+  # Singleton accessor to the current answer file being used by the PDK.
+  #
+  # @return [PDK::AnswerFile] The AnswerFile instance currently being used by
+  #   the PDK.
+  def self.answers
+    @answer_file ||= PDK::AnswerFile.new
+  end
+
+  class AnswerFile
+    attr_reader :answers
+    attr_reader :answer_file_path
+
+    # Initialises the AnswerFile object, which stores the responses to certain
+    # interactive questions.
+    #
+    # @param answer_file_path [String, nil] The path on disk to the file where
+    #   the answers will be stored and read from. If not specified (or `nil`),
+    #   the default path will be used (see #default_answer_file_path).
+    #
+    # @raise (see #read_from_disk)
+    def initialize(answer_file_path = nil)
+      @answer_file_path = answer_file_path || default_answer_file_path
+      @answers = read_from_disk
+    end
+
+    # Retrieve the stored answer to a question.
+    #
+    # @param question [String] The question name/identifying string.
+    #
+    # @return [Object] The answer to the question, or `nil` if no answer found.
+    def [](question)
+      answers[question]
+    end
+
+    # Update the stored answers in memory and then save them to disk.
+    #
+    # @param new_answers [Hash{String => Object}] The new questions and answers
+    #   to be merged into the existing answers.
+    #
+    # @raise [PDK::CLI::FatalError] if the new answers are not provided as
+    #   a Hash.
+    # @raise (see #save_to_disk)
+    def update!(new_answers = {})
+      unless new_answers.is_a?(Hash)
+        raise PDK::CLI::FatalError, _('Answer file can only be updated with a Hash')
+      end
+
+      answers.merge!(new_answers)
+
+      save_to_disk
+    end
+
+    private
+
+    # Determine the default path to the answer file.
+    #
+    # @return [String] The path on disk to the default answer file.
+    def default_answer_file_path
+      File.join(PDK::Util.cachedir, 'answers.json')
+    end
+
+    # Read existing answers into memory from the answer file on disk.
+    #
+    # @raise [PDK::CLI::FatalError] If the answer file exists but can not be
+    #   read.
+    #
+    # @return [Hash{String => Object}] The existing questions and answers.
+    def read_from_disk
+      return {} if !File.file?(answer_file_path) || File.zero?(answer_file_path)
+
+      unless File.readable?(answer_file_path)
+        raise PDK::CLI::FatalError, _("Unable to open '%{file}' for reading") % {
+          file: answer_file_path,
+        }
+      end
+
+      answers = JSON.parse(File.read(answer_file_path))
+      if answers.is_a?(Hash)
+        answers
+      else
+        PDK.logger.warn _("Answer file '%{path}' did not contain a valid set of answers, recreating it") % {
+          path: answer_file_path,
+        }
+        {}
+      end
+    rescue JSON::JSONError
+      PDK.logger.warn _("Answer file '%{path}' did not contain valid JSON, recreating it") % {
+        path: answer_file_path,
+      }
+      {}
+    end
+
+    # Save the in memory answer set to the answer file on disk.
+    #
+    # @raise [PDK::CLI::FatalError] if the answer file can not be written to.
+    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
+    rescue SystemCallError, IOError => e
+      raise PDK::CLI::FatalError, _("Unable to write '%{file}': %{msg}") % {
+        file: answer_file_path,
+        msg:  e.message,
+      }
+    end
+  end
+end
diff --git a/spec/pdk/answer_file_spec.rb b/spec/pdk/answer_file_spec.rb
new file mode 100644
index 000000000..a46d2c946
--- /dev/null
+++ b/spec/pdk/answer_file_spec.rb
@@ -0,0 +1,206 @@
+require 'spec_helper'
+require 'stringio'
+
+shared_context 'a valid answer file' do
+  before(:each) do
+    allow(File).to receive(:file?).with(default_path).and_return(true)
+    allow(File).to receive(:zero?).with(default_path).and_return(false)
+    allow(File).to receive(:readable?).with(default_path).and_return(true)
+    allow(File).to receive(:read).with(default_path).and_return('{"question": "answer"}')
+  end
+
+  subject(:answer_file) { described_class.new }
+end
+
+describe PDK::AnswerFile do
+  let(:default_path) { File.join(PDK::Util.cachedir, 'answers.json') }
+
+  describe '#initialize' do
+    context 'when not provided a path to an answer file' do
+      it 'uses the default path' do
+        expect(described_class.new).to have_attributes(answer_file_path: default_path)
+      end
+    end
+
+    context 'when provided a path to an answer file' do
+      let(:path) { '/path/to/answers.json' }
+
+      it 'uses the provided path' do
+        expect(described_class.new(path)).to have_attributes(answer_file_path: path)
+      end
+    end
+  end
+
+  describe '#read_from_disk' do
+    context 'when the answer file does not exist' do
+      before(:each) do
+        allow(File).to receive(:file?).with(default_path).and_return(false)
+      end
+
+      it 'creates an empty set of answers' do
+        expect(described_class.new).to have_attributes(answers: {})
+      end
+    end
+
+    context 'when the answer file exists' do
+      before(:each) do
+        allow(File).to receive(:file?).with(default_path).and_return(true)
+      end
+
+      context 'and contains no data' do
+        before(:each) do
+          allow(File).to receive(:zero?).with(default_path).and_return(true)
+        end
+
+        it 'creates an empty set of answers' do
+          expect(described_class.new).to have_attributes(answers: {})
+        end
+      end
+
+      context 'and is unreadable' do
+        before(:each) do
+          allow(File).to receive(:zero?).with(default_path).and_return(false)
+          allow(File).to receive(:readable?).with(default_path).and_return(false)
+        end
+
+        it 'raises a FatalError' do
+          expect { described_class.new }.to raise_error(PDK::CLI::FatalError, %r{unable to open .+ for reading}i)
+        end
+      end
+
+      context 'and is readable' do
+        let(:file_contents) {}
+
+        before(:each) do
+          allow(File).to receive(:zero?).with(default_path).and_return(false)
+          allow(File).to receive(:readable?).with(default_path).and_return(true)
+          allow(File).to receive(:read).with(default_path).and_return(file_contents)
+        end
+
+        context 'but contains invalid JSON' do
+          let(:file_contents) { 'this is not JSON' }
+          let(:warning_message) { a_string_matching(%r{answer file .+ did not contain valid JSON}i) }
+
+          it 'warns the user that the file could not be parsed' do
+            expect(logger).to receive(:warn).with(warning_message)
+
+            described_class.new
+          end
+
+          it 'creates a new empty set of answers' do
+            allow(logger).to receive(:warn).with(warning_message)
+
+            expect(described_class.new).to have_attributes(answers: {})
+          end
+        end
+
+        context 'but contains valid JSON that is not a Hash of answers' do
+          let(:file_contents) { '["this", "is", "an", "array"]' }
+          let(:warning_message) { a_string_matching(%r{answer file .+ did not contain a valid set of answers}i) }
+
+          it 'warns the user that the answers are invalid' do
+            expect(logger).to receive(:warn).with(warning_message)
+
+            described_class.new
+          end
+
+          it 'creates a new empty set of answers' do
+            allow(logger).to receive(:warn).with(warning_message)
+
+            expect(described_class.new).to have_attributes(answers: {})
+          end
+        end
+
+        context 'and contains a valid JSON Hash of answers' do
+          let(:file_contents) { '{"question": "answer"}' }
+
+          it 'populates the answer set with the values from the file' do
+            expect(described_class.new).to have_attributes(answers: { 'question' => 'answer' })
+          end
+        end
+      end
+    end
+  end
+
+  describe '#[]' do
+    include_context 'a valid answer file'
+
+    it 'returns the answer to the question if stored' do
+      expect(answer_file['question']).to eq('answer')
+    end
+
+    it 'returns nil to the question if not stored' do
+      expect(answer_file['unknown question']).to be_nil
+    end
+  end
+
+  describe '#update!' do
+    include_context 'a valid answer file'
+
+    before(:each) do
+      allow(answer_file).to receive(:save_to_disk)
+    end
+
+    it 'takes a hash of new answers and merges it into the existing answer set' do
+      answer_file.update!('another question' => 'different answer')
+
+      expect(answer_file).to have_attributes(answers: { 'question' => 'answer', 'another question' => 'different answer' })
+    end
+
+    it 'raises a FatalError if not passed a Hash' do
+      expect {
+        answer_file.update!('an answer without a question')
+      }.to raise_error(PDK::CLI::FatalError, %r{answer file can only be updated with a hash}i)
+    end
+  end
+
+  describe '#save_to_disk' do
+    include_context 'a valid answer file'
+
+    context 'when the file can be written to' do
+      let(:fake_file) { StringIO.new }
+
+      before(:each) do
+        allow(File).to receive(:open).with(default_path, 'w').and_yield(fake_file)
+      end
+
+      it 'writes the answer set to disk' do
+        answer_file.update!
+
+        fake_file.rewind
+        expect(fake_file.read).not_to be_empty
+      end
+
+      it 'writes out the answers as valid JSON' do
+        answer_file.update!
+
+        fake_file.rewind
+        expect(JSON.parse(fake_file.read)).to eq('question' => 'answer')
+      end
+    end
+
+    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')
+      end
+
+      it 'raises a FatalError' do
+        message = %r{\Aunable to write '#{Regexp.escape(default_path)}': .*some error message}i
+        expect { answer_file.update! }.to raise_error(PDK::CLI::FatalError, message)
+      end
+    end
+
+    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')
+      end
+
+      it 'raises a FatalError' do
+        message = %r{\Aunable to write '#{Regexp.escape(default_path)}': .*some other error}i
+        expect { answer_file.update! }.to raise_error(PDK::CLI::FatalError, message)
+      end
+    end
+  end
+end

From fbfeaaf4045bf7333b039365020612d1dfff0e82 Mon Sep 17 00:00:00 2001
From: Tim Sharpe <tim@sharpe.id.au>
Date: Wed, 5 Jul 2017 15:43:21 +1000
Subject: [PATCH 2/5] (maint) Add hidden option to set answer file path for
 testing

---
 lib/pdk/answer_file.rb                     | 8 ++++++++
 lib/pdk/cli.rb                             | 4 ++++
 spec/acceptance/support/in_a_new_module.rb | 9 ++++++++-
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/lib/pdk/answer_file.rb b/lib/pdk/answer_file.rb
index 8e9cbc90b..92d9bec8e 100644
--- a/lib/pdk/answer_file.rb
+++ b/lib/pdk/answer_file.rb
@@ -9,6 +9,14 @@ def self.answers
     @answer_file ||= PDK::AnswerFile.new
   end
 
+  # Specify the path to a custom answer file that the PDK should use.
+  #
+  # @param path [String] A path on disk to the file where the PDK should store
+  #   answers to interactive questions.
+  def self.answer_file=(value)
+    @answer_file = PDK::AnswerFile.new(value)
+  end
+
   class AnswerFile
     attr_reader :answers
     attr_reader :answer_file_path
diff --git a/lib/pdk/cli.rb b/lib/pdk/cli.rb
index ab00ab4bd..03493fc3b 100644
--- a/lib/pdk/cli.rb
+++ b/lib/pdk/cli.rb
@@ -66,6 +66,10 @@ def self.template_url_option(dsl)
     flag :d, :debug, _('Enable debug output.') do |_, _|
       PDK.logger.enable_debug_output
     end
+
+    option nil, 'answer-file', _('Path to an answer file'), argument: :required, hidden: true do |value|
+      PDK.answer_file = value
+    end
   end
 
   require 'pdk/cli/new'
diff --git a/spec/acceptance/support/in_a_new_module.rb b/spec/acceptance/support/in_a_new_module.rb
index d6e3ddd3a..037a80fe9 100644
--- a/spec/acceptance/support/in_a_new_module.rb
+++ b/spec/acceptance/support/in_a_new_module.rb
@@ -2,7 +2,13 @@
 
 shared_context 'in a new module' do |name|
   before(:all) do
-    output, status = Open3.capture2e('pdk', 'new', 'module', name, '--skip-interview', '--template-url', "file://#{RSpec.configuration.template_dir}")
+    argv = [
+      'pdk', 'new', 'module', name,
+      '--skip-interview',
+      '--template-url', "file:///#{RSpec.configuration.template_dir}",
+      '--answer-file', File.join(Dir.pwd, "#{name}_answers.json")
+    ]
+    output, status = Open3.capture2e(*argv)
 
     raise "Failed to create test module:\n#{output}" unless status.success?
 
@@ -12,5 +18,6 @@
   after(:all) do
     Dir.chdir('..')
     FileUtils.rm_rf(name)
+    FileUtils.rm("#{name}_answers.json")
   end
 end

From 4b876261d97e28c52319756c59ad8db45ec99328 Mon Sep 17 00:00:00 2001
From: Tim Sharpe <tim@sharpe.id.au>
Date: Wed, 5 Jul 2017 15:43:54 +1000
Subject: [PATCH 3/5] (SDK-305) Integrate answer file into module generator

---
 lib/pdk/generators/module.rb   |  40 ++-
 spec/generators/module_spec.rb | 545 ++++++++++++++++++++++++++++-----
 2 files changed, 502 insertions(+), 83 deletions(-)

diff --git a/lib/pdk/generators/module.rb b/lib/pdk/generators/module.rb
index 568aa1166..6d717ef67 100644
--- a/lib/pdk/generators/module.rb
+++ b/lib/pdk/generators/module.rb
@@ -30,7 +30,7 @@ def self.invoke(opts = {})
 
         prepare_module_directory(temp_target_dir)
 
-        template_url = opts.fetch(:'template-url', DEFAULT_TEMPLATE)
+        template_url = opts.fetch(:'template-url', PDK.answers['template-url'] || DEFAULT_TEMPLATE)
 
         PDK::Module::TemplateDir.new(template_url) do |templates|
           templates.render do |file_path, file_content|
@@ -48,16 +48,28 @@ def self.invoke(opts = {})
           end
         end
 
+        PDK.answers.update!('template-url' => template_url)
+
         FileUtils.mv(temp_target_dir, target_dir)
       end
 
-      def self.prepare_metadata(opts)
-        username = Etc.getlogin.gsub(%r{[^0-9a-z]}i, '')
-        username = 'username' if username == ''
-        if Etc.getlogin != username
-          PDK.logger.warn(_('Your username is not a valid Forge username, proceeding with the username %{username}' % { username: username }))
+      def self.username_from_login
+        login = Etc.getlogin
+        login_clean = login.gsub(%r{[^0-9a-z]}i, '')
+        login_clean = 'username' if login_clean.empty?
+
+        if login_clean != login
+          PDK.logger.warn _('You username is not a valid Forge username, proceeding with the username %{username}') % {
+            username: login_clean,
+          }
         end
 
+        login_clean
+      end
+
+      def self.prepare_metadata(opts)
+        username = PDK.answers['forge-username'] || username_from_login
+
         defaults = {
           'name'         => "#{username}-#{opts[:name]}",
           'version'      => '0.1.0',
@@ -65,11 +77,13 @@ def self.prepare_metadata(opts)
             { 'name' => 'puppetlabs-stdlib', 'version_requirement' => '>= 4.13.1 < 5.0.0' },
           ],
         }
+        defaults['author'] = PDK.answers['author'] unless PDK.answers['author'].nil?
+        defaults['license'] = PDK.answers['license'] unless PDK.answers['license'].nil?
         defaults['license'] = opts[:license] if opts.key? :license
 
         metadata = PDK::Module::Metadata.new(defaults)
 
-        module_interview(metadata, opts) unless opts[:'skip-interview'] # @todo Build way to get info by answers file
+        module_interview(metadata, opts) unless opts[:'skip-interview']
 
         metadata.update!('pdk-version' => PDK::Util::Version.version_string)
 
@@ -98,7 +112,7 @@ def self.module_interview(metadata, opts = {})
             required:         true,
             validate_pattern: %r{\A[a-z0-9]+\Z}i,
             validate_message: _('Forge usernames can only contain lowercase letters and numbers'),
-            default:          metadata.data['author'],
+            default:          PDK.answers['forge-username'] || metadata.data['author'],
           },
           {
             name:             'version',
@@ -171,7 +185,9 @@ def self.module_interview(metadata, opts = {})
           exit 0
         end
 
+        forge_username = answers['name']
         answers['name'] = "#{answers['name']}-#{opts[:name]}"
+        answers['license'] = opts[:license] if opts.key?(:license)
         metadata.update!(answers)
 
         puts '-' * 40
@@ -181,10 +197,16 @@ def self.module_interview(metadata, opts = {})
         puts '-' * 40
         puts
 
-        unless prompt.yes?(_('About to generate this module; continue?')) # rubocop:disable Style/GuardClause
+        unless prompt.yes?(_('About to generate this module; continue?'))
           puts _('Aborting...')
           exit 0
         end
+
+        PDK.answers.update!(
+          'forge-username' => forge_username,
+          'author'         => answers['author'],
+          'license'        => answers['license'],
+        )
       end
     end
   end
diff --git a/spec/generators/module_spec.rb b/spec/generators/module_spec.rb
index ddf72a888..169306e20 100644
--- a/spec/generators/module_spec.rb
+++ b/spec/generators/module_spec.rb
@@ -1,108 +1,471 @@
 require 'spec_helper'
+require 'tempfile'
+require 'stringio'
 
-describe PDK::Generate::Module do
-  context 'when gathering module information via the user interview' do
-    let(:metadata) do
-      PDK::Module::Metadata.new.update!(
-        'version' => '0.1.0',
-        'dependencies' => [
-          { 'name' => 'puppetlabs-stdlib', 'version_requirement' => '>= 1.0.0' },
-        ],
-      )
-    end
-    let(:prompt) { TTY::TestPrompt.new }
-
-    it 'populates the Metadata object based on user input' do
-      expect(TTY::Prompt).to receive(:new).and_return(prompt)
-      prompt.input << [
-        "foo\n",
-        "2.2.0\n",
-        "William Hopper\n",
-        "Apache-2.0\n",
-        "A simple module to do some stuff.\n",
-        "github.com/whopper/bar\n",
-        "forge.puppet.com/whopper/bar\n",
-        "tickets.foo.com/whopper/bar\n",
-        "yes\n",
-      ].join
-      prompt.input.rewind
+shared_context 'blank answer file' do
+  let(:temp_answer_file) { Tempfile.new('pdk-test-answers') }
 
-      described_class.module_interview(metadata, name: 'bar')
-
-      expect(metadata.data).to eq(
-        'name' => 'foo-bar',
-        'version'       => '2.2.0',
-        'author'        => 'William Hopper',
-        'license'       => 'Apache-2.0',
-        'summary'       => 'A simple module to do some stuff.',
-        'source'        => 'github.com/whopper/bar',
-        'project_page'  => 'forge.puppet.com/whopper/bar',
-        'issues_url'    => 'tickets.foo.com/whopper/bar',
-        'dependencies'  => [{ 'name' => 'puppetlabs-stdlib', 'version_requirement' => '>= 1.0.0' }],
-        'data_provider' => nil,
-        'operatingsystem_support' => [
-          { 'operatingsystem' => 'Debian', 'operatingsystemrelease' => ['8'] },
-          { 'operatingsystem' => 'RedHat', 'operatingsystemrelease' => ['7.0'] },
-          { 'operatingsystem' => 'Ubuntu', 'operatingsystemrelease' => ['16.04'] },
-          { 'operatingsystem' => 'Windows', 'operatingsystemrelease' => ['2016'] },
-        ],
-      )
-    end
+  before(:each) do
+    PDK.answer_file = temp_answer_file.path
   end
 
-  context 'when running under a user whose name is not a valid forge name' do
-    before(:each) do
-      allow(Etc).to receive(:getlogin).and_return('user.name')
-    end
+  after(:each) do
+    temp_answer_file.close
+    temp_answer_file.unlink
+  end
+end
+
+shared_context 'allow summary to be printed to stdout' do
+  before(:each) do
+    allow($stdout).to receive(:puts).with(a_string_matching(%r{\A-+\Z}))
+    allow($stdout).to receive(:puts).with('SUMMARY')
+    allow($stdout).to receive(:puts).with(a_string_matching(%r{\A\{.+\}\Z}m))
+    allow($stdout).to receive(:puts).with(no_args)
+  end
+end
+
+shared_context 'mock template dir' do
+  let(:test_template_dir) { instance_double(PDK::Module::TemplateDir, render: true, metadata: {}) }
+  let(:test_template_file) { StringIO.new }
 
-    let(:defaults) do
+  before(:each) do
+    allow(PDK::Module::TemplateDir).to receive(:new).with(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)
+    allow(Pathname).to receive(:new).with(temp_target_dir).and_return(dir_double)
+  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)
+  end
+end
+
+describe PDK::Generate::Module do
+  describe '.invoke' do
+    let(:target_dir) { File.expand_path('/path/to/target/module') }
+    let(:invoke_opts) do
       {
-        :name => 'foo',
+        :target_dir       => target_dir,
+        :name             => 'foo',
         :'skip-interview' => true,
-        :target_dir => 'foo',
       }
     end
 
-    it 'still works' do
-      expect { described_class.prepare_metadata(defaults) }.not_to raise_error
+    context 'when the target module directory already exists' do
+      before(:each) do
+        allow(File).to receive(:exist?).with(target_dir).and_return(true)
+      end
+
+      it 'raises a FatalError' do
+        expect {
+          described_class.invoke(target_dir: target_dir)
+        }.to raise_error(PDK::CLI::FatalError, %r{destination directory '.+' already exists}i)
+      end
     end
 
-    describe 'the generated metadata' do
-      subject(:the_metadata) { described_class.prepare_metadata(defaults).data }
+    context 'when the target module directory does not exist' do
+      include_context 'blank answer file'
+      include_context 'mock template dir'
+      include_context 'mock metadata.json'
+
+      let(:temp_target_dir) { '/path/to/temp/dir' }
+
+      before(:each) do
+        allow(File).to receive(:exist?).with(target_dir).and_return(false)
+        allow(PDK::Util).to receive(:make_tmpdir_name).with(anything).and_return(temp_target_dir)
+        allow(FileUtils).to receive(:mv).with(temp_target_dir, target_dir)
+        allow(PDK::Util::Version).to receive(:version_string).and_return('0.0.0')
+        allow(described_class).to receive(:prepare_module_directory).with(temp_target_dir)
+      end
+
+      it 'generates the new module in a temporary directory' do
+        expect(described_class).to receive(:prepare_module_directory).with(temp_target_dir)
+        described_class.invoke(invoke_opts)
+      end
+
+      context 'when the module template contains template files' do
+        before(:each) do
+          allow(test_template_dir).to receive(:render).and_yield('test_file_path', 'test_file_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')
+        end
+      end
+
+      context 'when the template dir generates metadata about itself' do
+        let(:template_metadata) do
+          {
+            'template-url' => 'test_template_url',
+            'template-ref' => 'test_template_ref',
+          }
+        end
+
+        before(:each) do
+          allow(test_template_dir).to receive(:metadata).and_return(template_metadata)
+        end
+
+        it 'includes details about the template in the generated metadata.json' do
+          described_class.invoke(invoke_opts)
+
+          metadata_json.rewind
+          expect(JSON.parse(metadata_json.read)).to include(template_metadata)
+        end
+      end
+
+      it 'moves the temporary directory to the target directory when done' do
+        expect(FileUtils).to receive(:mv).with(temp_target_dir, target_dir)
+        described_class.invoke(invoke_opts)
+      end
+
+      context 'when a template-url is supplied on the command line' do
+        it 'uses that template to generate the module' do
+          expect(PDK::Module::TemplateDir).to receive(:new).with('cli-template').and_yield(test_template_dir)
+          described_class.invoke(invoke_opts.merge(:'template-url' => 'cli-template'))
+        end
+
+        it 'takes precedence over the template-url answer' do
+          PDK.answers.update!('template-url' => 'answer-template')
+          expect(PDK::Module::TemplateDir).to receive(:new).with('cli-template').and_yield(test_template_dir)
+          described_class.invoke(invoke_opts.merge(:'template-url' => 'cli-template'))
+        end
+
+        it 'saves the template-url to the answer file' do
+          expect(PDK.answers).to receive(:update!).with('template-url' => 'cli-template')
+
+          described_class.invoke(invoke_opts.merge(:'template-url' => 'cli-template'))
+        end
+      end
+
+      context 'when a template-url is not supplied on the command line' do
+        context 'and a template-url answer exists' do
+          it 'uses the template-url from the answer file to generate the module' do
+            PDK.answers.update!('template-url' => 'answer-template')
+            expect(PDK::Module::TemplateDir).to receive(:new).with('answer-template').and_yield(test_template_dir)
+
+            described_class.invoke(invoke_opts)
+          end
+        end
+
+        context 'and no template-url answer exists' do
+          it 'uses the default template to generate the module' do
+            expect(PDK::Module::TemplateDir).to receive(:new).with(PDK::Generate::Module::DEFAULT_TEMPLATE).and_yield(test_template_dir)
 
-      it do
-        expect(the_metadata['name']).to eq 'username-foo'
+            described_class.invoke(invoke_opts)
+          end
+        end
       end
     end
+  end
 
-    context 'when running under an entirely non-alphanumeric username' do
-      before(:each) do
-        allow(Etc).to receive(:getlogin).and_return('Αρίσταρχος ό Σάμιος')
+  describe '.module_interview' do
+    include_context 'blank answer file'
+
+    subject(:interview_metadata) do
+      metadata = PDK::Module::Metadata.new
+      metadata.update!(default_metadata)
+      described_class.module_interview(metadata, options)
+      metadata.data
+    end
+
+    subject(:answers) do
+      allow($stdout).to receive(:puts).with(a_string_matching(%r{quick questions}))
+      interview_metadata
+      PDK.answers
+    end
+
+    let(:module_name) { 'bar' }
+    let(:default_metadata) { {} }
+    let(:options) { { name: module_name } }
+
+    before(:each) do
+      prompt = TTY::TestPrompt.new
+      allow(TTY::Prompt).to receive(:new).and_return(prompt)
+      prompt.input << responses.join("\n") + "\n"
+      prompt.input.rewind
+    end
+
+    context 'when provided answers to all the questions' do
+      include_context 'allow summary to be printed to stdout'
+
+      let(:responses) do
+        [
+          'foo',
+          '2.2.0',
+          'William Hopper',
+          'Apache-2.0',
+          'A simple module to do some stuff.',
+          'github.com/whopper/bar',
+          'forge.puppet.com/whopper/bar',
+          'tickets.foo.com/whopper/bar',
+          'yes',
+        ]
       end
 
-      let(:defaults) do
+      it 'populates the Metadata object based on user input' do
+        allow($stdout).to receive(:puts).with(a_string_matching(%r{8 quick questions}m))
+
+        expect(interview_metadata).to include(
+          'name'         => 'foo-bar',
+          'version'      => '2.2.0',
+          'author'       => 'William Hopper',
+          'license'      => 'Apache-2.0',
+          'summary'      => 'A simple module to do some stuff.',
+          'source'       => 'github.com/whopper/bar',
+          'project_page' => 'forge.puppet.com/whopper/bar',
+          'issues_url'   => 'tickets.foo.com/whopper/bar',
+        )
+      end
+
+      it 'saves the forge username to the answer file' do
+        expect(answers['forge-username']).to eq('foo')
+      end
+
+      it 'saves the module author to the answer file' do
+        expect(answers['author']).to eq('William Hopper')
+      end
+
+      it 'saves the license to the answer file' do
+        expect(answers['license']).to eq('Apache-2.0')
+      end
+    end
+
+    context 'when the user chooses the default values for everything' do
+      include_context 'allow summary to be printed to stdout'
+
+      let(:default_metadata) do
         {
-          :name => 'foo',
-          :'skip-interview' => true,
-          :target_dir => 'foo',
+          'author'  => 'defaultauthor',
+          'version' => '0.0.1',
+          'summary' => 'default summary',
+          'source'  => 'default source',
+          'license' => 'default license',
+        }
+      end
+
+      let(:responses) do
+        [
+          '',
+          '',
+          '',
+          '',
+          '',
+          '',
+          '',
+          '',
+        ]
+      end
+
+      it 'populates the interview question defaults with existing metadata values' do
+        allow($stdout).to receive(:puts).with(a_string_matching(%r{8 quick questions}))
+
+        expect(interview_metadata).to include(
+          'name'    => 'defaultauthor-bar',
+          'version' => '0.0.1',
+          'author'  => 'defaultauthor',
+          'license' => 'default license',
+          'summary' => 'default summary',
+          'source'  => 'default source',
+        )
+      end
+
+      it 'saves the forge username to the answer file' do
+        expect(answers['forge-username']).to eq('defaultauthor')
+      end
+
+      it 'saves the module author to the answer file' do
+        expect(answers['author']).to eq('defaultauthor')
+      end
+
+      it 'saves the license to the answer file' do
+        expect(answers['license']).to eq('default license')
+      end
+    end
+
+    context 'when the user provides the license as a command line option' do
+      include_context 'allow summary to be printed to stdout'
+
+      let(:options) { { name: module_name, license: 'MIT' } }
+      let(:responses) do
+        [
+          'foo',
+          '2.2.0',
+          'William Hopper',
+          'A simple module to do some stuff.',
+          'github.com/whopper/bar',
+          'forge.puppet.com/whopper/bar',
+          'tickets.foo.com/whopper/bar',
+          'yes',
+        ]
+      end
+
+      it 'populates the Metadata object based on user input' do
+        allow($stdout).to receive(:puts).with(a_string_matching(%r{7 quick questions}m))
+
+        expect(interview_metadata).to include(
+          'name'         => 'foo-bar',
+          'version'      => '2.2.0',
+          'author'       => 'William Hopper',
+          'license'      => 'MIT',
+          'summary'      => 'A simple module to do some stuff.',
+          'source'       => 'github.com/whopper/bar',
+          'project_page' => 'forge.puppet.com/whopper/bar',
+          'issues_url'   => 'tickets.foo.com/whopper/bar',
+        )
+      end
+
+      it 'saves the forge username to the answer file' do
+        expect(answers['forge-username']).to eq('foo')
+      end
+
+      it 'saves the module author to the answer file' do
+        expect(answers['author']).to eq('William Hopper')
+      end
+
+      it 'saves the license to the answer file' do
+        expect(answers['license']).to eq('MIT')
+      end
+    end
+
+    context 'when the user cancels the interview' do
+      let(:responses) do
+        [
+          "foo\003", # \003 being the equivalent to the user hitting Ctrl-C
+        ]
+      end
+
+      it 'exits cleanly' do
+        allow(logger).to receive(:info).with(a_string_matching(%r{interview cancelled}i))
+        allow($stdout).to receive(:puts).with(a_string_matching(%r{8 quick questions}m))
+
+        expect { interview_metadata }.to raise_error(SystemExit) { |error|
+          expect(error.status).to eq(0)
         }
       end
+    end
+
+    context 'when the user does not confirm the metadata' do
+      include_context 'allow summary to be printed to stdout'
+
+      let(:responses) do
+        [
+          'foo',
+          '2.2.0',
+          'William Hopper',
+          'Apache-2.0',
+          'A simple module to do some stuff.',
+          'github.com/whopper/bar',
+          'forge.puppet.com/whopper/bar',
+          'tickets.foo.com/whopper/bar',
+          'no',
+        ]
+      end
+
+      it 'exits cleanly' do
+        allow(logger).to receive(:info).with(a_string_matching(%r{module not generated}i))
+        allow($stdout).to receive(:puts).with(a_string_matching(%r{8 quick questions}m))
+
+        expect { interview_metadata }.to raise_error(SystemExit) { |error|
+          expect(error.status).to eq(0)
+        }
+      end
+    end
+  end
+
+  describe '.prepare_metadata' do
+    include_context 'blank answer file'
+
+    subject(:metadata) { described_class.prepare_metadata(options) }
+
+    before(:each) do
+      allow(described_class).to receive(:username_from_login).and_return('testlogin')
+    end
+
+    let(:options) { { name: 'baz' } }
+
+    context 'when provided :skip-interview => true' do
+      let(:options) { { :name => 'baz', :'skip-interview' => true } }
+
+      it 'does not perform the module interview' do
+        expect(described_class).not_to receive(:module_interview)
+
+        metadata
+      end
+    end
+
+    context 'when there are no saved answers' do
+      before(:each) do
+        allow(described_class).to receive(:module_interview).with(any_args)
+      end
+
+      it 'guesses the forge username from the system login' do
+        expect(metadata.data).to include('name' => 'testlogin-baz')
+      end
 
-      it 'still works' do
-        expect { described_class.prepare_metadata(defaults) }.not_to raise_error
+      it 'sets the version number to a 0.x release' do
+        expect(metadata.data).to include('version' => a_string_starting_with('0.'))
       end
 
-      describe 'the generated metadata' do
-        subject(:the_metadata) { described_class.prepare_metadata(defaults).data }
+      it 'includes puppetlabs-stdlib as a dependency' do
+        expect(metadata.data).to include(
+          'dependencies' => [
+            {
+              'name'                => 'puppetlabs-stdlib',
+              'version_requirement' => a_string_matching(%r{>=?\s+\d+\.\d+\.\d+\s<=?\s\d+\.\d+\.\d+}),
+            },
+          ],
+        )
+      end
+
+      it 'includes the PDK version number' do
+        expect(metadata.data).to include('pdk-version' => PDK::Util::Version.version_string)
+      end
+    end
+
+    context 'when an answer file exists with answers' do
+      before(:each) do
+        allow(described_class).to receive(:module_interview).with(any_args)
 
-        it do
-          expect(the_metadata['name']).to eq 'username-foo'
+        PDK.answers.update!(
+          'forge-username' => 'testuser123',
+          'license'        => 'MIT',
+          'author'         => 'Test User',
+        )
+      end
+
+      it 'uses the saved forge-username answer' do
+        expect(metadata.data).to include('name' => 'testuser123-baz')
+      end
+
+      it 'uses the saved author answer' do
+        expect(metadata.data).to include('author' => 'Test User')
+      end
+
+      it 'uses the saved license answer' do
+        expect(metadata.data).to include('license' => 'MIT')
+      end
+
+      context 'and the user specifies a license as a command line option' do
+        let(:options) { { name: 'baz', license: 'Apache-2.0' } }
+
+        it 'prefers the license specified on the command line over the saved license answer' do
+          expect(metadata.data).to include('license' => 'Apache-2.0')
         end
       end
     end
   end
 
-  context '.prepare_module_directory' do
+  describe '.prepare_module_directory' do
     let(:path) { 'test123' }
 
     it 'creates a skeleton directory structure' do
@@ -112,4 +475,38 @@
       described_class.prepare_module_directory(path)
     end
   end
+
+  describe '.username_from_login' do
+    subject { described_class.username_from_login }
+
+    before(:each) do
+      allow(Etc).to receive(:getlogin).and_return(login)
+    end
+
+    context 'when the login is entirely alphanumeric' do
+      let(:login) { 'testUser123' }
+
+      it 'returns the unaltered login' do
+        is_expected.to eq(login)
+      end
+    end
+
+    context 'when the login contains some non-alphanumeric characters' do
+      let(:login) { 'test_user' }
+
+      it 'warns the user and returns the login with the characters removed' do
+        expect(logger).to receive(:warn).with(a_string_matching(%r{not a valid forge username}i))
+        is_expected.to eq('testuser')
+      end
+    end
+
+    context 'when the login contains only non-alphanumeric characters' do
+      let(:login) { 'Αρίσταρχος ό Σάμιος' }
+
+      it 'warns the user and returns the string "username"' do
+        expect(logger).to receive(:warn).with(a_string_matching(%r{not a valid forge username}i))
+        is_expected.to eq('username')
+      end
+    end
+  end
 end

From 456f350a2ef5cc8aa6efdb28375deaad34efd966 Mon Sep 17 00:00:00 2001
From: Tim Sharpe <tim@sharpe.id.au>
Date: Thu, 6 Jul 2017 18:54:08 +1000
Subject: [PATCH 4/5] (maint) Print interview question number & help through
 TTY::Prompt outputter

---
 lib/pdk/cli/util/interview.rb | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/pdk/cli/util/interview.rb b/lib/pdk/cli/util/interview.rb
index 975bb9168..a376ebbc6 100644
--- a/lib/pdk/cli/util/interview.rb
+++ b/lib/pdk/cli/util/interview.rb
@@ -27,9 +27,9 @@ def run
           num_questions = @questions.count
           @questions.each do |question_name, question|
             @name = question_name
-            puts pastel.bold(_('[Q %{current_number}/%{questions_total}]') % { current_number: i, questions_total: num_questions })
-            puts pastel.bold(question[:question])
-            puts question[:help]
+            @prompt.print pastel.bold(_('[Q %{current_number}/%{questions_total}]') % { current_number: i, questions_total: num_questions })
+            @prompt.print pastel.bold(question[:question])
+            @prompt.print question[:help]
             ask(_('-->')) do |q|
               q.required(question.fetch(:required, false))
 
@@ -40,7 +40,7 @@ def run
               q.default(question[:default]) if question.key?(:default)
             end
             i += 1
-            puts ''
+            @prompt.print ''
           end
           @answers
         rescue TTY::Prompt::Reader::InputInterrupt

From 093f94725c445d008984f94f6308d76605eabdbb Mon Sep 17 00:00:00 2001
From: Tim Sharpe <tim@sharpe.id.au>
Date: Thu, 6 Jul 2017 18:54:45 +1000
Subject: [PATCH 5/5] (maint) Use PDK::Logger instead of .puts in
 PDK::Generate::Module

---
 lib/pdk/generators/module.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/pdk/generators/module.rb b/lib/pdk/generators/module.rb
index 6d717ef67..bbdf53256 100644
--- a/lib/pdk/generators/module.rb
+++ b/lib/pdk/generators/module.rb
@@ -181,7 +181,7 @@ def self.module_interview(metadata, opts = {})
         answers = interview.run
 
         if answers.nil?
-          puts _('Interview cancelled, aborting...')
+          PDK.logger.info _('Interview cancelled, not generating the module.')
           exit 0
         end
 
@@ -198,7 +198,7 @@ def self.module_interview(metadata, opts = {})
         puts
 
         unless prompt.yes?(_('About to generate this module; continue?'))
-          puts _('Aborting...')
+          PDK.logger.info _('Module not generated.')
           exit 0
         end