Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: test_create_cache_if_dir_not_exit nerver passes once executed #4636

Merged
merged 1 commit into from
Apr 6, 2020

Conversation

keroxp
Copy link
Contributor

@keroxp keroxp commented Apr 5, 2020

This test doesn't remove created directory after test. It will fail on next run.

   Compiling deno v0.39.0 (/Users/keroxp/src/deno/cli)
   Compiling test_plugin v0.0.1 (/Users/keroxp/src/deno/test_plugin)
    Finished test [unoptimized + debuginfo] target(s) in 28.69s
     Running target/debug/deps/deno-dadb83997376169a

running 1 test
test disk_cache::tests::test_create_cache_if_dir_not_exits ... FAILED

failures:

---- disk_cache::tests::test_create_cache_if_dir_not_exits stdout ----
thread 'disk_cache::tests::test_create_cache_if_dir_not_exits' panicked at 'assertion failed: `(left == right)`
  left: `true`,
 right: `false`', cli/disk_cache.rs:154:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    disk_cache::tests::test_create_cache_if_dir_not_exits

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 220 filtered out

Copy link
Contributor

@Drunpy Drunpy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! I've missed that due to have run the tests in different machines at the time.
Thanks.

Comment on lines +149 to +152
let temp_dir = TempDir::new().unwrap();
let mut cache_location = temp_dir.path().to_owned();
assert!(fs::remove_dir(&cache_location).is_ok());
cache_location.push("foo");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you use TempDir::new() then there's no need to remove the directory before the test

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did for checking ::new creates directories recursively. The reason to create temp dir is to get safe path that can contain sub directory.

Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - thanks @kt3k

@ry ry merged commit b9e5e4c into denoland:master Apr 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants