6 files changed, 94 insertions, 36 deletions

diff --git a/spec/functional/resource/execute_spec.rb b/spec/functional/resource/execute_spec.rb
index ffa4628cb2..692ccfb796 100644
--- a/spec/functional/resource/execute_spec.rb
+++ b/spec/functional/resource/execute_spec.rb
@@ -137,9 +137,16 @@ describe Chef::Resource::Execute do
     end
   end
 
+  # Ensure that CommandTimeout is raised, and is caused by resource.timeout really expiring.
+  # https://github.com/chef/chef/issues/2985
+  #
+  # resource.timeout should be short, this is what we're testing
+  # resource.command ruby sleep timer should be longer than resource.timeout to give us something to timeout
+  # Timeout::timeout should be longer than resource.timeout, but less than the resource.command ruby sleep timer,
+  #   so we fail if we finish on resource.command instead of resource.timeout, but raise CommandTimeout anyway (#2175).
   it "times out when a timeout is set on the resource" do
-    Timeout::timeout(5) do
-      resource.command %{ruby -e 'sleep 600'}
+    Timeout::timeout(30) do
+      resource.command %{ruby -e 'sleep 300'}
       resource.timeout 0.1
       expect { resource.run_action(:run) }.to raise_error(Mixlib::ShellOut::CommandTimeout)
     end
diff --git a/spec/functional/resource/file_spec.rb b/spec/functional/resource/file_spec.rb
index f1a290dea4..9e30e62111 100644
--- a/spec/functional/resource/file_spec.rb
+++ b/spec/functional/resource/file_spec.rb
@@ -86,6 +86,31 @@ describe Chef::Resource::File do
     end
   end
 
+
+  describe "when using backup" do
+    before do
+      Chef::Config[:file_backup_path] = CHEF_SPEC_BACKUP_PATH
+      resource_without_content.backup(1)
+      resource_without_content.run_action(:create)
+    end
+
+    let(:backup_glob) { File.join(CHEF_SPEC_BACKUP_PATH, test_file_dir.sub(/^([A-Za-z]:)/, ""), "#{file_base}*") }
+
+    let(:path) do
+      # Use native system path
+      ChefConfig::PathHelper.canonical_path(File.join(test_file_dir, make_tmpname(file_base)), false)
+    end
+
+    it "only stores the number of requested backups" do
+      resource_without_content.content('foo')
+      resource_without_content.run_action(:create)
+      resource_without_content.content('bar')
+      resource_without_content.run_action(:create)
+      expect(Dir.glob(backup_glob).length).to eq(1)
+    end
+
+  end
+
   # github issue 1842.
   describe "when running action :create on a relative path" do
     before do
diff --git a/spec/functional/resource/group_spec.rb b/spec/functional/resource/group_spec.rb
index 6676aa32e9..529af52d4e 100644
--- a/spec/functional/resource/group_spec.rb
+++ b/spec/functional/resource/group_spec.rb
@@ -372,6 +372,11 @@ downthestreetalwayshadagoodsmileonhisfacetheoldmanwalkingdownthestreeQQQQQQ" }
     let(:tested_action) { :manage }
 
     describe "when there is no group" do
+      before(:each) do
+        group_resource.run_action(:remove)
+        group_should_not_exist(group_name)
+      end
+
       it "raises an error on modify" do
         expect { group_resource.run_action(:modify) }.to raise_error
       end
diff --git a/spec/functional/resource/link_spec.rb b/spec/functional/resource/link_spec.rb
index d39a0c2ef6..7e903b30b4 100644
--- a/spec/functional/resource/link_spec.rb
+++ b/spec/functional/resource/link_spec.rb
@@ -348,8 +348,7 @@ describe Chef::Resource::Link do
           end
           it_behaves_like 'delete errors out'
         end
-        context 'and the link already exists and is not writeable to this user', :skip => true do
-        end
+
         it_behaves_like 'a securable resource without existing target' do
           let(:path) { target_file }
           def allowed_acl(sid, expected_perms)
@@ -360,7 +359,7 @@ describe Chef::Resource::Link do
           end
           def parent_inheritable_acls
             dummy_file_path = File.join(test_file_dir, "dummy_file")
-            dummy_file = FileUtils.touch(dummy_file_path)
+            FileUtils.touch(dummy_file_path)
             dummy_desc = get_security_descriptor(dummy_file_path)
             FileUtils.rm_rf(dummy_file_path)
             dummy_desc
@@ -416,8 +415,6 @@ describe Chef::Resource::Link do
           end
         end
       end
-      context "when the link destination is not readable to this user", :skip => true do
-      end
       context "when the link destination does not exist" do
         include_context 'create symbolic link succeeds'
         include_context 'delete is noop'
@@ -518,8 +515,6 @@ describe Chef::Resource::Link do
           end
           it_behaves_like 'delete errors out'
         end
-        context "and the link already exists and is not writeable to this user", :skip => true do
-        end
         context "and specifies security attributes" do
           before(:each) do
             resource.owner(windows? ? 'Guest' : 'nobody')
@@ -559,10 +554,10 @@ describe Chef::Resource::Link do
           end
           context 'and the link does not yet exist' do
             it 'links to the target file' do
+              skip('OS X/FreeBSD/AIX symlink? and readlink working on hard links to symlinks') if (os_x? or freebsd? or aix?)
               resource.run_action(:create)
               expect(File.exists?(target_file)).to be_truthy
               # OS X gets angry about this sort of link.  Bug in OS X, IMO.
-              pending('OS X/FreeBSD/AIX symlink? and readlink working on hard links to symlinks') if (os_x? or freebsd? or aix?)
               expect(symlink?(target_file)).to be_truthy
               expect(readlink(target_file)).to eq(canonicalize(@other_target))
             end
@@ -578,7 +573,7 @@ describe Chef::Resource::Link do
           end
           context 'and the link does not yet exist' do
             it 'links to the target file' do
-              pending('OS X/FreeBSD/AIX fails to create hardlinks to broken symlinks') if (os_x? or freebsd? or aix?)
+              skip('OS X/FreeBSD/AIX fails to create hardlinks to broken symlinks') if (os_x? or freebsd? or aix?)
               resource.run_action(:create)
               # Windows and Unix have different definitions of exists? here, and that's OK.
               if windows?
@@ -593,8 +588,7 @@ describe Chef::Resource::Link do
           end
         end
       end
-      context "when the link destination is not readable to this user", :skip => true do
-      end
+
       context "when the link destination does not exist" do
         context 'and the link does not yet exist' do
           it 'create errors out' do
diff --git a/spec/functional/resource/powershell_spec.rb b/spec/functional/resource/powershell_spec.rb
index 56a905efe7..17ae8cbd2a 100644
--- a/spec/functional/resource/powershell_spec.rb
+++ b/spec/functional/resource/powershell_spec.rb
@@ -56,14 +56,13 @@ describe Chef::Resource::WindowsScript::PowershellScript, :windows_only do
       resource.run_action(:run)
     end
 
-    it "returns the -27 for a powershell script that exits with -27", :windows_powershell_dsc_only do
-      # This is broken on Powershell < 4.0
+    it "returns the exit status 27 for a powershell script that exits with 27" do
       file = Tempfile.new(['foo', '.ps1'])
       begin
-        file.write "exit -27"
+        file.write "exit 27"
         file.close
         resource.code(". \"#{file.path}\"")
-        resource.returns(-27)
+        resource.returns(27)
         resource.run_action(:run)
       ensure
         file.close
@@ -71,6 +70,30 @@ describe Chef::Resource::WindowsScript::PowershellScript, :windows_only do
       end
     end
 
+    let (:negative_exit_status) { -27 }
+    let (:unsigned_exit_status) { (-negative_exit_status ^ 65535) + 1 }
+    it "returns the exit status -27 as a signed integer or an unsigned 16-bit 2's complement value of 65509 for a powershell script that exits with -27" do
+      # Versions of PowerShell prior to 4.0 return a 16-bit unsigned value --
+      # PowerShell 4.0 and later versions return a 32-bit signed value.
+      file = Tempfile.new(['foo', '.ps1'])
+      begin
+        file.write "exit #{negative_exit_status.to_s}"
+        file.close
+        resource.code(". \"#{file.path}\"")
+
+        # PowerShell earlier than 4.0 takes negative exit codes
+        # and returns them as the underlying unsigned 16-bit
+        # 2's complement representation. We cover multiple versions
+        # of PowerShell in this example by including both the signed
+        # exit code and its converted counterpart as permitted return values.
+        # See http://support.microsoft.com/en-us/kb/2646183/zh-cn
+        resource.returns([negative_exit_status, unsigned_exit_status])
+        expect { resource.run_action(:run) }.not_to raise_error
+      ensure
+        file.close
+        file.unlink
+      end
+    end
 
     it "returns the process exit code" do
       resource.code(arbitrary_nonzero_process_exit_code_content)
@@ -99,7 +122,19 @@ describe Chef::Resource::WindowsScript::PowershellScript, :windows_only do
     it "returns 1 if the last command was a cmdlet that failed and was preceded by a successfully executed non-cmdlet Windows binary" do
       resource.code([windows_process_exit_code_success_content, cmdlet_exit_code_not_found_content].join(';'))
       resource.returns(1)
-      resource.run_action(:run)
+      expect { resource.run_action(:run) }.not_to raise_error
+    end
+
+    it "raises an error if the script is not syntactically correct and returns is not set to 1" do
+      resource.code('if({)')
+      resource.returns(0)
+      expect { resource.run_action(:run) }.to raise_error(Mixlib::ShellOut::ShellCommandFailed)
+    end
+
+    it "returns 1 if the script provided to the code attribute is not syntactically correct" do
+      resource.code('if({)')
+      resource.returns(1)
+      expect { resource.run_action(:run) }.not_to raise_error
     end
 
     # This somewhat ambiguous case, two failures of different types,
diff --git a/spec/functional/resource/user/useradd_spec.rb b/spec/functional/resource/user/useradd_spec.rb
index 3e4e4e7604..474f6a4ecf 100644
--- a/spec/functional/resource/user/useradd_spec.rb
+++ b/spec/functional/resource/user/useradd_spec.rb
@@ -65,8 +65,12 @@ describe Chef::Provider::User::Useradd, metadata do
     end
   end
 
-  def supports_quote_in_username?
-    OHAI_SYSTEM["platform_family"] == "debian"
+  def self.quote_in_username_unsupported?
+    if OHAI_SYSTEM["platform_family"] == "debian"
+      false
+    else
+      "Only debian family systems support quotes in username"
+    end
   end
 
   def password_should_be_set
@@ -108,7 +112,7 @@ describe Chef::Provider::User::Useradd, metadata do
         break if status.exitstatus != 8
 
         sleep 1
-        max_retries = max_retries -1
+        max_retries = max_retries - 1
       rescue UserNotFound
         break
       end
@@ -162,15 +166,10 @@ describe Chef::Provider::User::Useradd, metadata do
     end
   end
 
-  let(:skip) { false }
-
   describe "action :create" do
 
     context "when the user does not exist beforehand" do
       before do
-        if reason = skip
-          pending(reason)
-        end
         user_resource.run_action(:create)
         expect(user_resource).to be_updated_by_last_action
       end
@@ -186,14 +185,7 @@ describe Chef::Provider::User::Useradd, metadata do
       #  tabulation: '\t', etc.). Note that using a slash ('/') may break the
       #  default algorithm for the definition of the user's home directory.
 
-      context "and the username contains a single quote" do
-        let(:skip) do
-          if supports_quote_in_username?
-            false
-          else
-            "Platform #{OHAI_SYSTEM["platform"]} not expected to support username w/ quote"
-          end
-        end
+      context "and the username contains a single quote", skip: quote_in_username_unsupported? do
 
         let(:username) { "t'bilisi" }
 
@@ -342,7 +334,7 @@ describe Chef::Provider::User::Useradd, metadata do
 
       before do
         if reason = skip
-          pending(reason)
+          skip(reason)
         end
         existing_user.run_action(:create)
         expect(existing_user).to be_updated_by_last_action
@@ -535,7 +527,7 @@ describe Chef::Provider::User::Useradd, metadata do
 
     def aix_user_lock_status
       lock_info = shell_out!("lsuser -a account_locked #{username}")
-      status = /\S+\s+account_locked=(\S+)/.match(lock_info.stdout)[1]
+      /\S+\s+account_locked=(\S+)/.match(lock_info.stdout)[1]
     end
 
     def user_account_should_be_locked