Skip to content

Commit

Permalink
Auto merge of #7042 - ehuss:no-global-rm-rf, r=alexcrichton
Browse files Browse the repository at this point in the history
Revert test directory cleaning change.

#6900 changed it so that the entire `cit` directory was cleaned once when tests started. Previously, each `t#` directory was deleted just before each test ran. This restores the old behavior due to problems on Windows.

The problem is that the call to `rm_rf` would fail with various errors ("Not found", "directory not empty", etc.) if you run `cargo test` twice. The first panic would poison the lazy static initializer, causing all subsequent tests to fail.

There are a variety of reasons deleting a file on Windows is difficult. My hypothesis in this case is that services like the indexing service and Defender swoop in and temporarily hold handles to files. This seems to be worse on slower systems, where presumably these services take longer to process all the files created by the test suite. It may also be related to how files are "marked for deletion" but are not immediately deleted.

The solution here is to spread out the deletion over time, giving Windows more of an opportunity to release its handles. This is a poor solution, and should only help reduce the frequency, but not entirely fix it.

I believe that this cannot be solved using `DeleteFileW`. There are more details at rust-lang/rust#29497, which is a long-standing problem that there are no good Rust implementations for recursively deleting a directory.

An example of something that implements a "safe" delete is [Cygwin's unlink implementation](https://github.com/cygwin/cygwin/blob/ad101bcb0f55f0eb1a9f60187f949c3decd855e4/winsup/cygwin/syscalls.cc#L675-L1064). As you can see, it is quite complex. Of course our use case does not need to handle quite as many edge cases, but I think any implementation is going to be nontrivial, and require Windows-specific APIs not available in std.

Note: Even before #6900 I still get a lot of errors on a slow VM (particularly "directory not empty"), with Defender and Indexing off. I'm not sure why. This PR should make it more bearable, though.
  • Loading branch information
bors committed Jun 19, 2019
2 parents a3ea135 + 0082290 commit 2ee292f
Showing 1 changed file with 3 additions and 4 deletions.
7 changes: 3 additions & 4 deletions tests/testsuite/support/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,7 @@ lazy_static! {
}

path.push(CARGO_INTEGRATION_TEST_DIR);

path.rm_rf();
path.mkdir_p();

path
};

Expand Down Expand Up @@ -62,7 +59,9 @@ pub fn init_root() -> TestIdGuard {

let guard = TestIdGuard { _private: () };

root().mkdir_p();
let r = root();
r.rm_rf();
r.mkdir_p();

guard
}
Expand Down

0 comments on commit 2ee292f

Please sign in to comment.