Merge pull request #9726 from chef/lcg/chef-16-git

Refactor scm, git and subversion resources & fix longstanding git issues

8 files changed, 338 insertions, 196 deletions

diff --git a/spec/functional/resource/git_spec.rb b/spec/functional/resource/git_spec.rb
index 677cfe4bf0..0355b59233 100644
--- a/spec/functional/resource/git_spec.rb
+++ b/spec/functional/resource/git_spec.rb
@@ -17,30 +17,18 @@
 #
 
 require "spec_helper"
-require "chef/mixin/shell_out"
 require "tmpdir"
-require "shellwords"
 
 # Deploy relies heavily on symlinks, so it doesn't work on windows.
 describe Chef::Resource::Git, requires_git: true do
-  include Chef::Mixin::ShellOut
-  let(:file_cache_path) { Dir.mktmpdir }
+  include RecipeDSLHelper
+
   # Some versions of git complains when the deploy directory is
   # already created. Here we intentionally don't create the deploy
   # directory beforehand.
   let(:base_dir_path) { Dir.mktmpdir }
   let(:deploy_directory) { File.join(base_dir_path, make_tmpname("git_base")) }
 
-  let(:node) do
-    Chef::Node.new.tap do |n|
-      n.name "rspec-test"
-      n.consume_external_attrs(@ohai.data, {})
-    end
-  end
-
-  let(:event_dispatch) { Chef::EventDispatch::Dispatcher.new }
-  let(:run_context) { Chef::RunContext.new(node, {}, event_dispatch) }
-
   # These tests use git's bundle feature, which is a way to export an entire
   # git repo (or subset of commits) as a single file.
   #
@@ -64,33 +52,36 @@ describe Chef::Resource::Git, requires_git: true do
   let(:rev_testing) { "972d153654503bccec29f630c5dd369854a561e8" }
   let(:rev_head) { "d294fbfd05aa7709ad9a9b8ef6343b17d355bf5f" }
 
-  let(:git_user_config) do
-    <<~E
-      [user]
-        name = frodoTbaggins
-        email = frodo@shire.org
-    E
-  end
-
   before(:each) do
-    Chef::Log.level = :warn # silence git command live streams
-    @old_file_cache_path = Chef::Config[:file_cache_path]
-    shell_out!("git clone \"#{git_bundle_repo}\" example", cwd: origin_repo_dir)
-    File.open("#{origin_repo}/.git/config", "a+") { |f| f.print(git_user_config) }
-    Chef::Config[:file_cache_path] = file_cache_path
+    shell_out!("git", "clone", git_bundle_repo, "example", cwd: origin_repo_dir)
+    File.open("#{origin_repo}/.git/config", "a+") do |f|
+      f.print <<~EOF
+        [user]
+          name = frodoTbaggins
+          email = frodo@shire.org
+      EOF
+    end
   end
 
   after(:each) do
-    Chef::Config[:file_cache_path] = @old_file_cache_path
     FileUtils.remove_entry_secure deploy_directory if File.exist?(deploy_directory)
     FileUtils.remove_entry_secure base_dir_path
-    FileUtils.remove_entry_secure file_cache_path
     FileUtils.remove_entry_secure origin_repo_dir
   end
 
-  before(:all) do
-    @ohai = Ohai::System.new
-    @ohai.all_plugins(%w{platform os})
+  def expect_revision_to_be(revision, version)
+    rev_ver = shell_out!("git", "rev-parse", revision, cwd: deploy_directory).stdout.strip
+    expect(rev_ver).to eq(version)
+  end
+
+  def expect_branch_upstream_to_be(branch, upstream)
+    branch_upstream = shell_out("git", "rev-parse", "--abbrev-ref", "#{branch}@{upstream}", cwd: deploy_directory).stdout.strip
+    expect(branch_upstream).to eq(upstream)
+  end
+
+  def expect_branch_to_be(branch)
+    head_branch = shell_out!("git name-rev --name-only HEAD", cwd: deploy_directory).stdout.strip
+    expect(head_branch).to eq(branch)
   end
 
   context "working with pathes with special characters" do
@@ -102,156 +93,221 @@ describe Chef::Resource::Git, requires_git: true do
     end
 
     it "clones a repository with a space in the path" do
-      Chef::Resource::Git.new(deploy_directory, run_context).tap do |r|
-        r.repository "#{path_with_spaces}/example-repo.gitbundle"
-      end.run_action(:sync)
+      repo = "#{path_with_spaces}/example-repo.gitbundle"
+      git(deploy_directory) do
+        repository repo
+      end.should_be_updated
+      expect_revision_to_be("HEAD", rev_head)
     end
   end
 
   context "when deploying from an annotated tag" do
-    let(:basic_git_resource) do
-      Chef::Resource::Git.new(deploy_directory, run_context).tap do |r|
-        r.repository origin_repo
-        r.revision "v1.0.0"
-      end
-    end
-
-    # We create a copy of the basic_git_resource so that we can run
-    # the resource again and verify that it doesn't update.
-    let(:copy_git_resource) do
-      Chef::Resource::Git.new(deploy_directory, run_context).tap do |r|
-        r.repository origin_repo
-        r.revision "v1.0.0"
-      end
-    end
-
     it "checks out the revision pointed to by the tag commit, not the tag commit itself" do
-      basic_git_resource.run_action(:sync)
-      head_rev = shell_out!("git rev-parse HEAD", cwd: deploy_directory, returns: [0]).stdout.strip
-      expect(head_rev).to eq(v1_commit)
+      git deploy_directory do
+        repository origin_repo
+        revision "v1.0.0"
+      end.should_be_updated
+      expect_revision_to_be("HEAD", v1_commit)
+      expect_branch_to_be("tags/v1.0.0^0") # detatched
       # also verify the tag commit itself is what we expect as an extra sanity check
-      rev = shell_out!("git rev-parse v1.0.0", cwd: deploy_directory, returns: [0]).stdout.strip
-      expect(rev).to eq(v1_tag)
+      expect_revision_to_be("v1.0.0", v1_tag)
     end
 
     it "doesn't update if up-to-date" do
-      # this used to fail because we didn't resolve the annotated tag
-      # properly to the pointed to commit.
-      basic_git_resource.run_action(:sync)
-      head_rev = shell_out!("git rev-parse HEAD", cwd: deploy_directory, returns: [0]).stdout.strip
-      expect(head_rev).to eq(v1_commit)
-
-      copy_git_resource.run_action(:sync)
-      expect(copy_git_resource).not_to be_updated
+      git deploy_directory do
+        repository origin_repo
+        revision "v1.0.0"
+      end.should_be_updated
+      git deploy_directory do
+        repository origin_repo
+        revision "v1.0.0"
+        expect_branch_to_be("tags/v1.0.0^0") # detatched
+      end.should_not_be_updated
     end
   end
 
   context "when deploying from a SHA revision" do
-    let(:basic_git_resource) do
-      Chef::Resource::Git.new(deploy_directory, run_context).tap do |r|
-        r.repository git_bundle_repo
-      end
-    end
-
-    # We create a copy of the basic_git_resource so that we can run
-    # the resource again and verify that it doesn't update.
-    let(:copy_git_resource) do
-      Chef::Resource::Git.new(deploy_directory, run_context).tap do |r|
-        r.repository origin_repo
-      end
+    it "checks out the expected revision ed18" do
+      git deploy_directory do
+        repository git_bundle_repo
+        revision rev_foo
+      end.should_be_updated
+      expect_revision_to_be("HEAD", rev_foo)
+      expect_branch_to_be("master~1") # detatched
     end
 
-    it "checks out the expected revision ed18" do
-      basic_git_resource.revision rev_foo
-      basic_git_resource.run_action(:sync)
-      head_rev = shell_out!("git rev-parse HEAD", cwd: deploy_directory, returns: [0]).stdout.strip
-      expect(head_rev).to eq(rev_foo)
+    it "checks out the expected revision ed18 to a local branch" do
+      git deploy_directory do
+        repository git_bundle_repo
+        revision rev_foo
+        checkout_branch "deploy"
+      end.should_be_updated
+      expect_revision_to_be("HEAD", rev_foo)
+      expect_branch_to_be("deploy") # detatched
     end
 
     it "doesn't update if up-to-date" do
-      basic_git_resource.revision rev_foo
-      basic_git_resource.run_action(:sync)
-      head_rev = shell_out!("git rev-parse HEAD", cwd: deploy_directory, returns: [0]).stdout.strip
-      expect(head_rev).to eq(rev_foo)
-
-      copy_git_resource.revision rev_foo
-      copy_git_resource.run_action(:sync)
-      expect(copy_git_resource).not_to be_updated
+      git deploy_directory do
+        repository git_bundle_repo
+        revision rev_foo
+      end.should_be_updated
+      expect_revision_to_be("HEAD", rev_foo)
+
+      git deploy_directory do
+        repository origin_repo
+        revision rev_foo
+      end.should_not_be_updated
+      expect_branch_to_be("master~1") # detatched
     end
 
     it "checks out the expected revision 972d" do
-      basic_git_resource.revision rev_testing
-      basic_git_resource.run_action(:sync)
-      head_rev = shell_out!("git rev-parse HEAD", cwd: deploy_directory, returns: [0]).stdout.strip
-      expect(head_rev).to eq(rev_testing)
+      git deploy_directory do
+        repository git_bundle_repo
+        revision rev_testing
+      end.should_be_updated
+      expect_revision_to_be("HEAD", rev_testing)
+      expect_branch_to_be("master~2") # detatched
+    end
+
+    it "checks out the expected revision 972d to a local branch" do
+      git deploy_directory do
+        repository git_bundle_repo
+        revision rev_testing
+        checkout_branch "deploy"
+      end.should_be_updated
+      expect_revision_to_be("HEAD", rev_testing)
+      expect_branch_to_be("deploy")
     end
   end
 
   context "when deploying from a revision named 'HEAD'" do
-    let(:basic_git_resource) do
-      Chef::Resource::Git.new(deploy_directory, run_context).tap do |r|
-        r.repository origin_repo
-        r.revision "HEAD"
-      end
+    it "checks out the expected revision" do
+      git deploy_directory do
+        repository origin_repo
+        revision "HEAD"
+      end.should_be_updated
+      expect_revision_to_be("HEAD", rev_head)
+      expect_branch_to_be("master")
     end
 
-    it "checks out the expected revision" do
-      basic_git_resource.run_action(:sync)
-      head_rev = shell_out!("git rev-parse HEAD", cwd: deploy_directory, returns: [0]).stdout.strip
-      expect(head_rev).to eq(rev_head)
+    it "checks out the expected revision, and is idempotent" do
+      git deploy_directory do
+        repository origin_repo
+        revision "HEAD"
+      end.should_be_updated
+      git deploy_directory do
+        repository origin_repo
+        revision "HEAD"
+      end.should_not_be_updated
+      expect_revision_to_be("HEAD", rev_head)
+      expect_branch_to_be("master")
+    end
+
+    it "checks out the expected revision to a local branch" do
+      git deploy_directory do
+        repository origin_repo
+        revision "HEAD"
+        checkout_branch "deploy"
+      end.should_be_updated
+      expect_revision_to_be("HEAD", rev_head)
+      expect_branch_to_be("deploy")
     end
   end
 
   context "when deploying from the default revision" do
-    let(:basic_git_resource) do
-      Chef::Resource::Git.new(deploy_directory, run_context).tap do |r|
-        r.repository origin_repo
-        # use default
-      end
+    it "checks out HEAD as the default revision" do
+      git deploy_directory do
+        repository origin_repo
+      end.should_be_updated
+      expect_branch_upstream_to_be("master", "origin/master")
+      expect_revision_to_be("HEAD", rev_head)
+      expect_branch_to_be("master")
     end
 
-    it "checks out HEAD as the default revision" do
-      basic_git_resource.run_action(:sync)
-      head_rev = shell_out!("git rev-parse HEAD", cwd: deploy_directory, returns: [0]).stdout.strip
-      expect(head_rev).to eq(rev_head)
+    it "checks out HEAD as the default revision, and is idempotent" do
+      git deploy_directory do
+        repository origin_repo
+      end.should_be_updated
+      git deploy_directory do
+        repository origin_repo
+      end.should_not_be_updated
+      expect_revision_to_be("HEAD", rev_head)
+      expect_branch_to_be("master")
+    end
+
+    it "checks out HEAD as the default revision to a local branch" do
+      git deploy_directory do
+        repository origin_repo
+        checkout_branch "deploy"
+      end.should_be_updated
+      expect_revision_to_be("HEAD", rev_head)
+      expect_branch_to_be("deploy")
     end
   end
 
   context "when dealing with a repo with a degenerate tag named 'HEAD'" do
     before do
-      shell_out!("git tag -m\"degenerate tag\" HEAD ed181b3419b6f489bedab282348162a110d6d3a1",
-        cwd: origin_repo)
+      shell_out!("git", "tag", "-m\"degenerate tag\"", "HEAD", "ed181b3419b6f489bedab282348162a110d6d3a1", cwd: origin_repo)
     end
 
-    let(:basic_git_resource) do
-      Chef::Resource::Git.new(deploy_directory, run_context).tap do |r|
-        r.repository origin_repo
-        r.revision "HEAD"
-      end
+    it "checks out the (master) HEAD revision and ignores the tag" do
+      git deploy_directory do
+        repository origin_repo
+        revision "HEAD"
+      end.should_be_updated
+      expect_revision_to_be("HEAD", rev_head)
+      expect_branch_to_be("master")
     end
 
-    let(:git_resource_default_rev) do
-      Chef::Resource::Git.new(deploy_directory, run_context).tap do |r|
-        r.repository origin_repo
-        # use default of revision
-      end
+    it "checks out the (master) HEAD revision and ignores the tag, and is idempotent" do
+      git deploy_directory do
+        repository origin_repo
+        revision "HEAD"
+      end.should_be_updated
+      git deploy_directory do
+        repository origin_repo
+        revision "HEAD"
+      end.should_not_be_updated
+      expect_revision_to_be("HEAD", rev_head)
+      expect_branch_to_be("master")
     end
 
-    it "checks out the (master) HEAD revision and ignores the tag" do
-      basic_git_resource.run_action(:sync)
-      head_rev = shell_out!("git rev-parse HEAD",
-        cwd: deploy_directory,
-        returns: [0]).stdout.strip
-      expect(head_rev).to eq(rev_head)
+    it "checks out the (master) HEAD revision and ignores the tag to a local branch" do
+      git deploy_directory do
+        repository origin_repo
+        revision "HEAD"
+        checkout_branch "deploy"
+      end.should_be_updated
+      expect_revision_to_be("HEAD", rev_head)
+      expect_branch_to_be("deploy")
     end
 
     it "checks out the (master) HEAD revision when no revision is specified (ignores tag)" do
-      git_resource_default_rev.run_action(:sync)
-      head_rev = shell_out!("git rev-parse HEAD",
-        cwd: deploy_directory,
-        returns: [0]).stdout.strip
-      expect(head_rev).to eq(rev_head)
+      git deploy_directory do
+        repository origin_repo
+      end.should_be_updated
+      expect_revision_to_be("HEAD", rev_head)
+      expect_branch_to_be("master")
+    end
+
+    it "checks out the (master) HEAD revision when no revision is specified (ignores tag), and is idempotent" do
+      git deploy_directory do
+        repository origin_repo
+      end.should_be_updated
+      git deploy_directory do
+        repository origin_repo
+      end.should_not_be_updated
+      expect_revision_to_be("HEAD", rev_head)
+      expect_branch_to_be("master")
     end
 
+    it "checks out the (master) HEAD revision when no revision is specified (ignores tag) to a local branch" do
+      git deploy_directory do
+        repository origin_repo
+        checkout_branch "deploy"
+      end.should_be_updated
+      expect_revision_to_be("HEAD", rev_head)
+      expect_branch_to_be("deploy")
+    end
   end
 end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 127af0470f..e2e967ba9c 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -74,6 +74,8 @@ require "spec/support/local_gems.rb" if File.exists?(File.join(File.dirname(__FI
 require "spec/support/platform_helpers"
 require "spec/support/shared/unit/mock_shellout"
 
+require "spec/support/recipe_dsl_helper"
+
 # Autoloads support files
 # Excludes support/platforms by default
 # Do not change the gsub.
diff --git a/spec/support/recipe_dsl_helper.rb b/spec/support/recipe_dsl_helper.rb
new file mode 100644
index 0000000000..2542345ed4
--- /dev/null
+++ b/spec/support/recipe_dsl_helper.rb
@@ -0,0 +1,83 @@
+#
+# This is a helper for functional tests to embed the recipe DSL directly into the rspec example blocks using
+# unified mode.
+#
+# If you wind up wanting to stub/expect on internal details of the resource/provider you are not testing the
+# public API and are trying to write a unit test, which this is not designed for.
+#
+# If you want to start writing full recipes and testing them, doing notifies/subscribes/etc then you are writing
+# an integration test, and not a functional single-resource test, which this is not designed for.
+#
+# Examples:
+#
+# it "creates a file" do
+#   FileUtils.rm_f("/tmp/foo.xyz")
+#   file "/tmp/foo.xyz" do           # please use proper tmpdir though
+#     content "whatever"
+#   end.should_be_updated
+#   expect(IO.read("/tmp/foo.xyz").to eql("content")
+# end
+#
+# it "is idempotent" do
+#   FileUtils.rm_f("/tmp/foo.xyz")
+#   file "/tmp/foo.xyz" do           # please use proper tmpdir though
+#     content "whatever"
+#   end.should_be_updated
+#   file "/tmp/foo.xyz" do           # please use proper tmpdir though
+#     content "whatever"
+#   end.should_not_be_updated
+#   expect(IO.read("/tmp/foo.xyz").to eql("content")
+# end
+#
+# it "has a failure" do
+#   FileUtils.rm_f("/tmp/foo.xyz")
+#   expect { file "/tmp/lksjdflksjdf/foo.xyz" do
+#     content "whatever"
+#   end }.to raise_error(Chef::Exception::EnclosingDirectoryDoesNotExist)
+# end
+#
+module RecipeDSLHelper
+  include Chef::DSL::Recipe
+  def event_dispatch
+    @event_dispatch ||= Chef::EventDispatch::Dispatcher.new
+  end
+
+  def node
+    @node ||= Chef::Node.new.tap do |n|
+      # clone the global ohai data to keep tests fast but reasonably isolated
+      n.consume_external_attrs(OHAI_SYSTEM.data.dup, {})
+    end
+  end
+
+  def run_context
+    @run_context ||= Chef::RunContext.new(node, {}, event_dispatch).tap do |rc|
+      rc.resource_collection.unified_mode = true
+      Chef::Runner.new(rc)
+    end
+  end
+
+  def cookbook_name
+    "rspec"
+  end
+
+  def recipe_name
+    "default"
+  end
+
+  def declare_resource(type, name, created_at: nil, run_context: self.run_context, &resource_attrs_block)
+    created_at = caller[0]
+    rspec_context = self
+    # we slightly abuse the "enclosing_provider" method_missing magic to send methods to the rspec example block so that
+    # rspec `let` methods work as arguments to resource properties
+    resource = super(type, name, created_at: created_at, run_context: run_context, enclosing_provider: rspec_context, &resource_attrs_block)
+    # we also inject these methods to make terse expression of checking the updated status (so it is more readiable and
+    # therefore should get used more -- even though it is "should" vs. "expect")
+    resource.define_singleton_method(:should_be_updated) do
+      rspec_context.expect(self).to be_updated
+    end
+    resource.define_singleton_method(:should_not_be_updated) do
+      rspec_context.expect(self).not_to be_updated
+    end
+    resource
+  end
+end
diff --git a/spec/unit/provider/git_spec.rb b/spec/unit/provider/git_spec.rb
index b98745b3ca..2f253358f1 100644
--- a/spec/unit/provider/git_spec.rb
+++ b/spec/unit/provider/git_spec.rb
@@ -400,7 +400,8 @@ describe Chef::Provider::Git do
     @provider.clone
   end
 
-  it "runs a checkout command with default options" do
+  it "runs a checkout command when the local branch is set" do
+    @resource.checkout_branch "deploy"
     expect(@provider).to receive(:shell_out!).with("git branch -f deploy d35af14d41ae22b19da05d7d03a0bafc321b244c", cwd: "/my/deploy/dir",
                                                                                                                     log_tag: "git[web2.0 app]").ordered
     expect(@provider).to receive(:shell_out!).with("git checkout deploy", cwd: "/my/deploy/dir",
@@ -607,7 +608,7 @@ describe Chef::Provider::Git do
 
     it "does not raise an error if user exists" do
       allow(@provider).to receive(:get_homedir).with(@resource.user).and_return("/home/test")
-      expect { @provider.run_action(:sync) }.not_to raise_error(ArgumentError)
+      expect { @provider.run_action(:sync) }.not_to raise_error
     end
   end
 
@@ -622,8 +623,10 @@ describe Chef::Provider::Git do
     end
 
     it "does not raise an error if user exists" do
+      allow(@provider).to receive(:action_sync) # stub the entire action
+      allow(::File).to receive(:directory?).with("/my/deploy").and_return(true)
       allow(@provider).to receive(:get_homedir).with(@resource.user).and_return("/home/test")
-      expect { @provider.run_action(:sync) }.not_to raise_error(Chef::Exceptions::User)
+      expect { @provider.run_action(:sync) }.not_to raise_error
     end
   end
 
diff --git a/spec/unit/provider/subversion_spec.rb b/spec/unit/provider/subversion_spec.rb
index f0393f6b40..f3d8404841 100644
--- a/spec/unit/provider/subversion_spec.rb
+++ b/spec/unit/provider/subversion_spec.rb
@@ -46,7 +46,8 @@ describe Chef::Provider::Subversion do
   it "converts resource properties to options for shell_out" do
     expect(@provider.run_options).to eq({})
     @resource.user "deployninja"
-    expect(@provider.run_options).to eq({ user: "deployninja" })
+    expect(@provider).to receive(:get_homedir).and_return("/home/deployninja")
+    expect(@provider.run_options).to eq({ user: "deployninja", environment: { "HOME" => "/home/deployninja" } })
   end
 
   context "determining the revision of the currently deployed code" do
@@ -221,7 +222,8 @@ describe Chef::Provider::Subversion do
     @resource.user "whois"
     @resource.group "thisis"
     expected_cmd = "svn checkout -q   -r12345 http://svn.example.org/trunk/ /my/deploy/dir"
-    expect(@provider).to receive(:shell_out!).with(expected_cmd, { user: "whois", group: "thisis" })
+    expect(@provider).to receive(:get_homedir).and_return("/home/whois")
+    expect(@provider).to receive(:shell_out!).with(expected_cmd, { user: "whois", group: "thisis", environment: { "HOME" => "/home/whois" } })
     @provider.run_action(:checkout)
     expect(@resource).to be_updated
   end
diff --git a/spec/unit/resource/git_spec.rb b/spec/unit/resource/scm/git_spec.rb
index 740af48e5f..706b7c370b 100644
--- a/spec/unit/resource/git_spec.rb
+++ b/spec/unit/resource/scm/git_spec.rb
@@ -17,6 +17,7 @@
 #
 
 require "spec_helper"
+require_relative "scm"
 
 describe Chef::Resource::Git do
 
@@ -29,8 +30,55 @@ describe Chef::Resource::Git do
 
   let(:resource) { Chef::Resource::Git.new("fakey_fakerton") }
 
-  it "is a subclass of Chef::Resource::Scm" do
-    expect(resource).to be_a_kind_of(Chef::Resource::Scm)
+  it_behaves_like "an SCM resource"
+
+  it "takes the depth as an integer for shallow clones" do
+    resource.depth 5
+    expect(resource.depth).to eq(5)
+    expect { resource.depth "five" }.to raise_error(ArgumentError)
+  end
+
+  it "defaults to nil depth for a full clone" do
+    expect(resource.depth).to be_nil
+  end
+
+  it "takes a boolean for #enable_submodules" do
+    resource.enable_submodules true
+    expect(resource.enable_submodules).to be_truthy
+    expect { resource.enable_submodules "lolz" }.to raise_error(ArgumentError)
+  end
+
+  it "defaults to not enabling submodules" do
+    expect(resource.enable_submodules).to be_falsey
+  end
+
+  it "takes a boolean for #enable_checkout" do
+    resource.enable_checkout true
+    expect(resource.enable_checkout).to be_truthy
+    expect { resource.enable_checkout "lolz" }.to raise_error(ArgumentError)
+  end
+
+  it "defaults to enabling checkout" do
+    expect(resource.enable_checkout).to be_truthy
+  end
+
+  it "takes a string for the remote" do
+    resource.remote "opscode"
+    expect(resource.remote).to eql("opscode")
+    expect { resource.remote 1337 }.to raise_error(ArgumentError)
+  end
+
+  it "defaults to ``origin'' for the remote" do
+    expect(resource.remote).to eq("origin")
+  end
+
+  it "takes a string for the ssh wrapper" do
+    resource.ssh_wrapper "with_ssh_fu"
+    expect(resource.ssh_wrapper).to eql("with_ssh_fu")
+  end
+
+  it "defaults to nil for the ssh wrapper" do
+    expect(resource.ssh_wrapper).to be_nil
   end
 
   it "uses aliases revision as branch" do
diff --git a/spec/unit/resource/scm_spec.rb b/spec/unit/resource/scm/scm.rb
index 6f282c1350..28c3f73136 100644
--- a/spec/unit/resource/scm_spec.rb
+++ b/spec/unit/resource/scm/scm.rb
@@ -19,9 +19,7 @@
 
 require "spec_helper"
 
-describe Chef::Resource::Scm do
-  let(:resource) { Chef::Resource::Scm.new("fakey_fakerton") }
-
+shared_examples_for "an SCM resource" do
   it "the destination property is the name_property" do
     expect(resource.destination).to eql("fakey_fakerton")
   end
@@ -78,55 +76,6 @@ describe Chef::Resource::Scm do
     expect(resource.group).to eq(23)
   end
 
-  it "takes the depth as an integer for shallow clones" do
-    resource.depth 5
-    expect(resource.depth).to eq(5)
-    expect { resource.depth "five" }.to raise_error(ArgumentError)
-  end
-
-  it "defaults to nil depth for a full clone" do
-    expect(resource.depth).to be_nil
-  end
-
-  it "takes a boolean for #enable_submodules" do
-    resource.enable_submodules true
-    expect(resource.enable_submodules).to be_truthy
-    expect { resource.enable_submodules "lolz" }.to raise_error(ArgumentError)
-  end
-
-  it "defaults to not enabling submodules" do
-    expect(resource.enable_submodules).to be_falsey
-  end
-
-  it "takes a boolean for #enable_checkout" do
-    resource.enable_checkout true
-    expect(resource.enable_checkout).to be_truthy
-    expect { resource.enable_checkout "lolz" }.to raise_error(ArgumentError)
-  end
-
-  it "defaults to enabling checkout" do
-    expect(resource.enable_checkout).to be_truthy
-  end
-
-  it "takes a string for the remote" do
-    resource.remote "opscode"
-    expect(resource.remote).to eql("opscode")
-    expect { resource.remote 1337 }.to raise_error(ArgumentError)
-  end
-
-  it "defaults to ``origin'' for the remote" do
-    expect(resource.remote).to eq("origin")
-  end
-
-  it "takes a string for the ssh wrapper" do
-    resource.ssh_wrapper "with_ssh_fu"
-    expect(resource.ssh_wrapper).to eql("with_ssh_fu")
-  end
-
-  it "defaults to nil for the ssh wrapper" do
-    expect(resource.ssh_wrapper).to be_nil
-  end
-
   it "defaults to nil for the environment" do
     expect(resource.environment).to be_nil
   end
diff --git a/spec/unit/resource/subversion_spec.rb b/spec/unit/resource/scm/subversion_spec.rb
index 48cc7fea42..394ed6be9f 100644
--- a/spec/unit/resource/subversion_spec.rb
+++ b/spec/unit/resource/scm/subversion_spec.rb
@@ -17,6 +17,7 @@
 #
 
 require "spec_helper"
+require_relative "scm"
 
 describe Chef::Resource::Subversion do
   static_provider_resolution(
@@ -28,9 +29,7 @@ describe Chef::Resource::Subversion do
 
   let(:resource) { Chef::Resource::Subversion.new("fakey_fakerton") }
 
-  it "is a subclass of Resource::Scm" do
-    expect(resource).to be_a_kind_of(Chef::Resource::Scm)
-  end
+  it_behaves_like "an SCM resource"
 
   it "the destination property is the name_property" do
     expect(resource.destination).to eql("fakey_fakerton")