Fix remove_file_fails_if_node_is_a_directory on MacOS

[2-complex]

This test

os::remove_file_fails_if_node_is_a_directory

fails because MacOS produces a different error from other platforms when attempting to remove_file a directory. MacOS returns PermissionDenied, where other OS's return Other.

Modifying fake-filessystem to produce PermissionDenied when running on MacOS, and changing the test to reflect.

[TheSven73]

When you look at something for too long, things can become too complicated, happens to me a lot :)

I think that to fix this, you only need 2 very simple changes:

In registry.rs::remove_file:

    Err(_) => Err(if cfg!(macos) { PermissionDenied } else { Other }),

In remove_file_fails_if_node_is_a_directory:

    let expected_error = if cfg!(macos) { PermissionDenied } else { Other };
    assert_eq!(result.unwrap_err().kind(), expected_error);

[2-complex]

When you look at something for too long, things can become too complicated, happens to me a lot :)

Right? Sometimes it's an illusion of an addled psyche, and sometimes it's real. It's hard to tell.

let expected_error = if cfg!(macos) { PermissionDenied } else { Other };
assert_eq!(result.unwrap_err().kind(), expected_error);

I like the temp variable for readability. Putting that in.

Err(_) => Err(if cfg!(macos) { PermissionDenied } else { Other }),

You mean just replace registry.rs:150 with that line? That doesn't quite work. It breaks fake::remove_file_fails_if_file_does_not_exist, because it converts the NotFound error into PermissionDenied, too

[TheSven73]

Holy smoke :)

We're only trying to catch the case on MacOS where someone is trying to delete a directory using remove_file, right? How about this:

    pub fn remove_file(&mut self, path: &Path) -> Result<()> {
        if cfg!(target_os = "macos") && self.is_dir(path) {
            // on MacOS, attempting to delete a directory results
            // in a "permission denied" error.
            return Err(create_error(ErrorKind::PermissionDenied));
        }
        match self.get_file(path) {
            Ok(_) => self.remove(path).and(Ok(())),
            Err(e) => Err(e),
        }
    }

[2-complex]

That code fixes the test, and I like how it reflects the intent more literally. It's a tiny bit redundant, because both is_dir(path) and get_file(path) call get(path). You cool with that?

[TheSven73]

Thank you !! I'm cool with it for now, as long as supporting macos (or windows) doesn't become any more complex. In that case we'll need to refactor. But we'll cross that bridge when we get to it.