Finishing specs for create and edit. During edit functional testing I discovered some large refactors needed.

9 files changed, 117 insertions, 93 deletions

diff --git a/lib/chef/knife/data_bag_common.rb b/lib/chef/knife/data_bag_common.rb
index 4d4f270139..93f8c98624 100644
--- a/lib/chef/knife/data_bag_common.rb
+++ b/lib/chef/knife/data_bag_common.rb
@@ -17,32 +17,34 @@
 #
 
 require 'mixlib/cli'
-# TODO these were in a `deps` call before - okay that they aren't anymore?
 require 'chef/config'
-#require 'chef/data_bag'
 require 'chef/encrypted_data_bag_item'
 
 class Chef
   class Knife
-    module DataBagSecretOptions
+    module DataBagCommon
       include Mixlib::CLI
 
-      option :secret,
-             :short => "-s SECRET",
-             :long  => "--secret ",
-             :description => "The secret key to use to encrypt data bag item values",
-             :proc => Proc.new { |s| Chef::Config[:knife][:secret] = s }
-
-      option :secret_file,
-             :long => "--secret-file SECRET_FILE",
-             :description => "A file containing the secret key to use to encrypt data bag item values",
-             :proc => Proc.new { |sf| Chef::Config[:knife][:secret_file] = sf }
-
-      option :encrypt,
-             :long => "--encrypt",
-             :description => "Only use the secret configured in knife.rb when this is true",
-             :boolean => true,
-             :default => false
+      # TODO when https://github.com/opscode/mixlib-cli/pull/13 is fixed, we can make this a base class
+      # instead of a module
+      def self.included(base)
+        base.option :secret,
+               :short => "-s SECRET",
+               :long  => "--secret ",
+               :description => "The secret key to use to encrypt data bag item values",
+               :proc => Proc.new { |s| Chef::Config[:knife][:secret] = s }
+
+        base.option :secret_file,
+               :long => "--secret-file SECRET_FILE",
+               :description => "A file containing the secret key to use to encrypt data bag item values",
+               :proc => Proc.new { |sf| Chef::Config[:knife][:secret_file] = sf }
+
+        base.option :encrypt,
+               :long => "--encrypt",
+               :description => "Only use the secret configured in knife.rb when this is true",
+               :boolean => true,
+               :default => false
+      end
 
       ##
       # Determine if the user has specified an appropriate secret for encrypting data bag items.
diff --git a/lib/chef/knife/data_bag_create.rb b/lib/chef/knife/data_bag_create.rb
index 048d34f543..40921d7d09 100644
--- a/lib/chef/knife/data_bag_create.rb
+++ b/lib/chef/knife/data_bag_create.rb
@@ -22,9 +22,9 @@ require 'chef/knife'
 class Chef
   class Knife
     class DataBagCreate < Knife
-      include DataBagSecretOptions
+      include DataBagCommon
 
-      # TODO duplicating deps here and in the DataBagSecretOptions module
+      # TODO duplicating deps here and in the DataBagCommon module
       deps do
         require 'chef/data_bag'
         require 'chef/encrypted_data_bag_item'
diff --git a/lib/chef/knife/data_bag_edit.rb b/lib/chef/knife/data_bag_edit.rb
index 13d51daee0..718aeedc29 100644
--- a/lib/chef/knife/data_bag_edit.rb
+++ b/lib/chef/knife/data_bag_edit.rb
@@ -22,7 +22,7 @@ require 'chef/knife'
 class Chef
   class Knife
     class DataBagEdit < Knife
-      include DataBagSecretOptions
+      include DataBagCommon
 
       deps do
         require 'chef/data_bag_item'
diff --git a/lib/chef/knife/data_bag_from_file.rb b/lib/chef/knife/data_bag_from_file.rb
index 4e61f1b53f..daaede706d 100644
--- a/lib/chef/knife/data_bag_from_file.rb
+++ b/lib/chef/knife/data_bag_from_file.rb
@@ -23,7 +23,7 @@ require 'chef/util/path_helper'
 class Chef
   class Knife
     class DataBagFromFile < Knife
-      include DataBagSecretOptions
+      include DataBagCommon
 
       deps do
         require 'chef/data_bag'
diff --git a/lib/chef/knife/data_bag_show.rb b/lib/chef/knife/data_bag_show.rb
index 0392d417cc..72f76734e7 100644
--- a/lib/chef/knife/data_bag_show.rb
+++ b/lib/chef/knife/data_bag_show.rb
@@ -22,7 +22,7 @@ require 'chef/knife'
 class Chef
   class Knife
     class DataBagShow < Knife
-      include DataBagSecretOptions
+      include DataBagCommon
 
       deps do
         require 'chef/data_bag'
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index ed0a8f89f6..50289451af 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -185,3 +185,26 @@ module WEBrick
     end
   end
 end
+
+# Lifted from http://stackoverflow.com/questions/1480537/how-can-i-validate-exits-and-aborts-in-rspec
+RSpec::Matchers.define :exit_with_code do |exp_code|
+  actual = nil
+  match do |block|
+    begin
+      block.call
+    rescue SystemExit => e
+      actual = e.status
+    end
+    actual and actual == exp_code
+  end
+  failure_message_for_should do |block|
+    "expected block to call exit(#{exp_code}) but exit" +
+        (actual.nil? ? " not called" : "(#{actual}) was called")
+  end
+  failure_message_for_should_not do |block|
+    "expected block not to call exit(#{exp_code})"
+  end
+  description do
+    "expect block to call exit(#{exp_code})"
+  end
+end
diff --git a/spec/unit/knife/data_bag_common_spec.rb b/spec/unit/knife/data_bag_common_spec.rb
index 67c63a8239..ebba9033ba 100644
--- a/spec/unit/knife/data_bag_common_spec.rb
+++ b/spec/unit/knife/data_bag_common_spec.rb
@@ -22,13 +22,13 @@ require 'chef/config'
 require 'tempfile'
 
 class ExampleDataBag < Chef::Knife
-  include Chef::Knife::DataBagSecretOptions
+  include Chef::Knife::DataBagCommon
 
   #banner "you must provide a banner"
   #category "data bag"
 end
 
-describe Chef::Knife::DataBagSecretOptions do
+describe Chef::Knife::DataBagCommon do
   let(:example_db) do
     k = ExampleDataBag.new
     allow(k.ui).to receive(:stdout).and_return(stdout)
@@ -42,10 +42,13 @@ describe Chef::Knife::DataBagSecretOptions do
     sfile = Tempfile.new("encrypted_data_bag_secret")
     sfile.puts(secret)
     sfile.flush
+    sfile
   end
 
   after do
     Chef::Config.reset
+    secret_file.close
+    secret_file.unlink
   end
 
   describe "#validate_secrets" do
diff --git a/spec/unit/knife/data_bag_create_spec.rb b/spec/unit/knife/data_bag_create_spec.rb
index d99575fa82..ae34b68a13 100644
--- a/spec/unit/knife/data_bag_create_spec.rb
+++ b/spec/unit/knife/data_bag_create_spec.rb
@@ -20,19 +20,6 @@
 require 'spec_helper'
 require 'tempfile'
 
-module ChefSpecs
-  class ChefRest
-    attr_reader :args_received
-    def initialize
-      @args_received = []
-    end
-
-    def post_rest(*args)
-      @args_received << args
-    end
-  end
-end
-
 describe Chef::Knife::DataBagCreate do
   let(:knife) do
     k = Chef::Knife::DataBagCreate.new
@@ -41,7 +28,7 @@ describe Chef::Knife::DataBagCreate do
     k
   end
 
-  let(:rest) { ChefSpecs::ChefRest.new }
+  let(:rest) { double("ChefSpecs::ChefRest") }
   let(:stdout) { StringIO.new }
 
   let(:bag_name) { "sudoing_admins" }
@@ -61,8 +48,8 @@ describe Chef::Knife::DataBagCreate do
 
   it "tries to create a data bag with an invalid name when given one argument" do
     knife.name_args = ['invalid&char']
-    expect(knife).to receive(:exit).with(1)
-    knife.run
+    expect(Chef::DataBag).to receive(:validate_name!).with(knife.name_args[0]).and_raise(Chef::Exceptions::InvalidDataBagName)
+    expect {knife.run}.to exit_with_code(1)
   end
 
   context "when given one argument" do
@@ -85,10 +72,6 @@ describe Chef::Knife::DataBagCreate do
       item
     end
 
-    before do
-      knife.name_args = [bag_name, item_name]
-    end
-
     it "creates a data bag item" do
       expect(knife).to receive(:create_object).and_yield(raw_hash)
       expect(knife).to receive(:encryption_secret_provided?).and_return(false)
diff --git a/spec/unit/knife/data_bag_edit_spec.rb b/spec/unit/knife/data_bag_edit_spec.rb
index ba931c1883..6a7c8b33b2 100644
--- a/spec/unit/knife/data_bag_edit_spec.rb
+++ b/spec/unit/knife/data_bag_edit_spec.rb
@@ -21,73 +21,86 @@ require 'tempfile'
 
 describe Chef::Knife::DataBagEdit do
   before do
-    @plain_data = {"login_name" => "alphaomega", "id" => "item_name"}
-    @edited_data = {
-      "login_name" => "rho", "id" => "item_name",
-      "new_key" => "new_value" }
+    Chef::Config[:node_name] = "webmonkey.example.com"
+    knife.name_args = [bag_name, item_name]
+    allow(knife).to receive(:config).and_return(config)
+  end
 
-    Chef::Config[:node_name]  = "webmonkey.example.com"
+  let(:knife) do
+    k = Chef::Knife::DataBagEdit.new
+    allow(k).to receive(:rest).and_return(rest)
+    allow(k).to receive(:stdout).and_return(stdout)
+    k
+  end
 
-    @knife = Chef::Knife::DataBagEdit.new
-    @rest = double('chef-rest-mock')
-    @knife.stub(:rest).and_return(@rest)
+  let(:plain_hash) { {"login_name" => "alphaomega", "id" => "item_name"} }
+  let(:plain_db) {Chef::DataBagItem.from_hash(plain_hash)}
+  let(:edited_hash) { {"login_name" => "rho", "id" => "item_name", "new_key" => "new_value"} }
+  let(:edited_db) {Chef::DataBagItem.from_hash(edited_hash)}
 
-    @stdout = StringIO.new
-    @knife.stub(:stdout).and_return(@stdout)
-    @log = Chef::Log
-    @knife.name_args = ['bag_name', 'item_name']
-  end
+  let(:rest) { double("ChefSpecs::ChefRest") }
+  let(:stdout) { StringIO.new }
+
+  let(:bag_name) { "sudoing_admins" }
+  let(:item_name) { "ME" }
+
+  let(:secret) { "abc123SECRET" }
+
+  let(:raw_hash)  {{ "login_name" => "alphaomega", "id" => item_name }}
+
+  let(:config) { {} }
 
   it "requires data bag and item arguments" do
-    @knife.name_args = []
-    lambda { @knife.run }.should raise_error(SystemExit)
-    @stdout.string.should match(/^You must supply the data bag and an item to edit/)
+    knife.name_args = []
+    expect(stdout).to receive(:puts).twice.with(anything)
+    expect {knife.run}.to exit_with_code(1)
   end
 
   it "saves edits on a data bag item" do
-    Chef::DataBagItem.stub(:load).with('bag_name', 'item_name').and_return(@plain_data)
-    @knife.should_receive(:edit_data).with(@plain_data).and_return(@edited_data)
-    @rest.should_receive(:put_rest).with("data/bag_name/item_name", @edited_data).ordered
-    @knife.run
+    expect(Chef::DataBagItem).to receive(:load).with(bag_name, item_name).and_return(plain_db)
+    expect(knife).to receive(:encrypted?) { false }
+    expect(knife).to receive(:edit_data).with(plain_db).and_return(edited_db.raw_data)
+    expect(rest).to receive(:put_rest).with("data/#{bag_name}/#{item_name}", edited_db.raw_data).ordered
+    knife.run
   end
 
   describe "encrypted data bag items" do
+    let(:enc_plain_hash) { Chef::EncryptedDataBagItem.encrypt_data_bag_item(plain_hash, secret) }
+    let(:data_bag_with_encoded_hash) { Chef::DataBagItem.from_hash(enc_plain_hash) }
+    let(:enc_edited_hash) { Chef::EncryptedDataBagItem.encrypt_data_bag_item(edited_hash, secret) }
+
     before(:each) do
-      @secret = "abc123SECRET"
-      @enc_data = Chef::EncryptedDataBagItem.encrypt_data_bag_item(@plain_data,
-                                                                   @secret)
-      @enc_edited_data = Chef::EncryptedDataBagItem.encrypt_data_bag_item(@edited_data,
-                                                                          @secret)
-      Chef::DataBagItem.stub(:load).with('bag_name', 'item_name').and_return(@enc_data)
-
-      # Random IV is used each time the data bag item is encrypted, so values
-      # will not be equal if we encrypt same value twice.
-      Chef::EncryptedDataBagItem.should_receive(:encrypt_data_bag_item).and_return(@enc_edited_data)
-
-      @secret_file = Tempfile.new("encrypted_data_bag_secret_file_test")
-      @secret_file.puts(@secret)
-      @secret_file.flush
+      allow(knife).to receive(:encrypted?) { true }
+      allow(knife).to receive(:encryption_secret_provided?) { true }
+      allow(knife).to receive(:read_secret).and_return(secret)
     end
 
-    after do
-      @secret_file.close
-      @secret_file.unlink
+    it "decrypts an encrypted data bag, edits it and rencrypts it" do
+      expect(Chef::DataBagItem).to receive(:load).with(bag_name, item_name).and_return(data_bag_with_encoded_hash)
+      expect(knife).to receive(:edit_data).with(plain_hash).and_return(edited_hash)
+      expect(Chef::EncryptedDataBagItem).to receive(:encrypt_data_bag_item).with(edited_hash, secret).and_return(enc_edited_hash)
+      expect(rest).to receive(:put_rest).with("data/#{bag_name}/#{item_name}", enc_edited_hash).ordered
+
+      knife.run
     end
 
-    it "decrypts and encrypts via --secret and --encrypted" do
-      @knife.stub(:config).and_return({:secret => @secret, :encrypted => true})
-      @knife.should_receive(:edit_data).with(@plain_data).and_return(@edited_data)
-      @rest.should_receive(:put_rest).with("data/bag_name/item_name", @enc_edited_data).ordered
+    it "edits an unencrypted data bag and encrypts it" do
+      expect(knife).to receive(:encrypted?) { false }
+      expect(Chef::DataBagItem).to receive(:load).with(bag_name, item_name).and_return(plain_db)
+      expect(knife).to receive(:edit_data).with(plain_db).and_return(edited_hash)
+      expect(Chef::EncryptedDataBagItem).to receive(:encrypt_data_bag_item).with(edited_hash, secret).and_return(enc_edited_hash)
+      expect(rest).to receive(:put_rest).with("data/#{bag_name}/#{item_name}", enc_edited_hash).ordered
 
-      @knife.run
+      knife.run
     end
 
-    it "decrypts and encrypts via --secret_file and --encrypted" do
-      @knife.stub(:config).and_return({:secret_file => @secret_file.path, :encrypted => true})
-      @knife.should_receive(:edit_data).with(@plain_data).and_return(@edited_data)
-      @rest.should_receive(:put_rest).with("data/bag_name/item_name", @enc_edited_data).ordered
+    it "fails to edit an encrypted data bag if the secret is missing" do
+      allow(knife).to receive(:encryption_secret_provided?) { false }
+      expect(Chef::DataBagItem).to receive(:load).with(bag_name, item_name).and_return(data_bag_with_encoded_hash)
 
-      @knife.run
+      expect(knife.ui).to receive(:fatal).with("You cannot edit an encrypted data bag without providing the secret.")
+      expect {knife.run}.to exit_with_code(1)
     end
+
   end
 end