Don't mask directory deletion errors

[jaymzh]

closes #4988

[jaymzh]

CC @chef/client-core

[lamont-granquist]

yeah, that makes sense 👍

[thommay]

seems good 👍

[btm]

Can you add a regression unit test to make sure we don't revert back and fix the existing copy-pasta on line 230 of the unit test while you're in there?

  describe "#run_action(:create)" do
    describe "when the directory exists" do
      it "deletes the directory" do
        directory.run_action(:delete)
        expect(File.exist?(tmp_dir)).to be false
      end

[jaymzh]

The typo is easy - but I have no idea how to write a test to simulate the OS failing the delete... :(

[btm]

being a unit test I was just thinking something like

it "does not use rm_rf which silently consumes errors" do
  expect(FileUtils).not_to receive(:rm_rf)
end

[jaymzh]

Oh. I can do that. sure :)

[jaymzh]

Tests fixed/added.

[btm]

👍 on green

[smurawski]

The appveyor build failure is not related - I looks like the appveyor build images just got a newer version of powershell which requires a different key usage type for the cert for password decryption.

Appveyor added PowerShell 5 to the base worker image on June 5. appveyor/ci#778

[jaymzh]

green on travis. av is unrelated. Merging.