Reduce file system interaction in unit tests

[ericcornelissen]

Relates to #1, #7

Test Improvements

Summary

The unit tests for this project could be improved by avoiding/reducing interaction with the file system. By depending on the file system, unit tests integrate with it and also become dependent on the file system behavior. As such, the goal of this issue is to reduce file system interaction in unit tests to a minimum.

Some unit tests that currently depend on the file system could avoid that interaction through, for example, traits and stubs or mocks. Other unit tests unavoidably have to interact with the file system at some point (e.g. test_dispose and test_remove), it could be considered to remove those unit tests or somehow mark them as integration tests in another way.

[ericcornelissen]

An evaluation of the unit tests as of 2cbcd60.

Overview

The following is a complete list of unit #[cfg(test)]s that interact with the file system (based on their usage of with_test_dir):

• fs::test_open
• fs::test_is_empty
• walk::test_given
• walk::test_recurse
• rm::test_dispose
• rm::test_remove
• transform::test_disallow_filled_dirs
• transform::test_interactive

Evaluation

fs

The tests for fs::open could avoid file system interaction fairly well if the std::fs functionality used can be effectively mocked. I believe this would still be fairly meaningful as the integration tests will most certainly cover this function extensively as well.

The tests for fs::is_empty are bit less clear (I'm also not very happy with this function to begin with). If you have any suggestions feel free to commit or submit a Pull Request 🙂

walk

The tests for walk::given and walk::recurse are coupled to the file system both through the fs module and (directly) the std::fs module. While mocking both of those modules would improve the "unitness" of these tests, it seems like a lot of effort (especially for walk::recurse). A good start could be avoiding the dependency on fs.

rm

The tests for rm::dispose and rm::remove are coupled to the file system both through (resp.) the trash crate and the std::fs module. Mocking could be used not only to avoid file system interaction but also to simulate error scenarios that are non-trivial to set up with the actual file system.

transform

Tests here should probably avoid file system interaction by using a trait rather than a concrete type.

Considerations

When this ticket is being worked on I want to avoid changing the implementation simply for the sake of avoiding file system interaction in the unit tests. Any change to the implementation should be meaningful in and of itself as well.