altugbakan / rs-tftpd

TFTP Server Daemon implemented in Rust
https://crates.io/crates/tftpd
MIT License
51 stars 14 forks source link

window::tests::fills_and_removes_from_window failure #3

Closed vilheikk closed 1 year ago

vilheikk commented 1 year ago

When running the tests with cargo test (in Fedora 37), window::tests::fills_and_removes_from_window fails.

RUST_BACKTRACE=1 cargo test
    Finished test [unoptimized + debuginfo] target(s) in 0.00s
     Running unittests src/lib.rs (target/debug/deps/tftpd-bc4c52d8dbe9f884)

running 28 tests
test config::tests::parses_full_config ... ok
test config::tests::parses_some_config ... ok
test config::tests::returns_error_on_invalid_directory ... ok
test config::tests::returns_error_on_invalid_port ... ok
test config::tests::returns_error_on_invalid_ip ... ok
test convert::tests::converts_to_string ... ok
test convert::tests::converts_to_string_with_index ... ok
test convert::tests::converts_to_u16 ... ok
test convert::tests::returns_error_on_short_array ... ok
test packet::tests::parses_ack ... ok
test packet::tests::parses_data ... ok
test packet::tests::parses_error ... ok
test packet::tests::parses_error_without_message ... ok
test packet::tests::parses_read_request ... ok
test packet::tests::parses_read_request_with_options ... ok
test packet::tests::parses_write_request ... ok
test packet::tests::parses_write_request_with_options ... ok
test packet::tests::serializes_ack ... ok
test packet::tests::serializes_data ... ok
test packet::tests::serializes_error ... ok
test packet::tests::serializes_oack ... ok
test server::tests::parses_default_options ... ok
test server::tests::parses_read_options ... ok
test server::tests::parses_write_options ... ok
test server::tests::validates_file_path ... ok
test socket::tests::test_recv ... ok
test window::tests::adds_to_and_empties_window ... ok
test window::tests::fills_and_removes_from_window ... FAILED

failures:

---- window::tests::fills_and_removes_from_window stdout ----
thread 'window::tests::fills_and_removes_from_window' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 17, kind: AlreadyExists, message: "File exists" }', src/window.rs:194:38
stack backtrace:
   0: rust_begin_unwind
             at /rustc/90743e7298aca107ddaa0c202a4d3604e29bfeb6/library/std/src/panicking.rs:575:5
   1: core::panicking::panic_fmt
             at /rustc/90743e7298aca107ddaa0c202a4d3604e29bfeb6/library/core/src/panicking.rs:65:14
   2: core::result::unwrap_failed
             at /rustc/90743e7298aca107ddaa0c202a4d3604e29bfeb6/library/core/src/result.rs:1791:5
   3: core::result::Result<T,E>::unwrap
             at /rustc/90743e7298aca107ddaa0c202a4d3604e29bfeb6/library/core/src/result.rs:1113:23
   4: tftpd::window::tests::initialize
             at ./src/window.rs:194:13
   5: tftpd::window::tests::fills_and_removes_from_window
             at ./src/window.rs:132:24
   6: tftpd::window::tests::fills_and_removes_from_window::{{closure}}
             at ./src/window.rs:129:5
   7: core::ops::function::FnOnce::call_once
             at /rustc/90743e7298aca107ddaa0c202a4d3604e29bfeb6/library/core/src/ops/function.rs:251:5
   8: core::ops::function::FnOnce::call_once
             at /rustc/90743e7298aca107ddaa0c202a4d3604e29bfeb6/library/core/src/ops/function.rs:251:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

failures:
    window::tests::fills_and_removes_from_window

test result: FAILED. 27 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s

error: test failed, to rerun pass `--lib`

I was able to fix this by ignoring the error

diff --git a/src/window.rs b/src/window.rs
index 7aad4f1..2a40ade 100644
--- a/src/window.rs
+++ b/src/window.rs
@@ -191,7 +191,8 @@ mod tests {
     fn initialize(file_name: &str) -> File {
         let file_name = DIR_NAME.to_string() + "/" + file_name;
         if !Path::new(DIR_NAME).is_dir() {
-            fs::create_dir(DIR_NAME).unwrap();
+            if fs::create_dir(DIR_NAME).is_err() {
+            }
         }

         if File::open(&file_name).is_ok() {

But I don't know if it's the correct fix.

altugbakan commented 1 year ago

Hey! Thanks for checking this project out.

I know about that issue and the leading cause is the tests run in parallel and they all try to create the same folder at the same time. Generally, this does not happen, but it is a flaky behavior and it should be fixed.

Your fix is correct for the short term, but in the long term, I think the tests should be mocked using a mock filesystem, which I hopefully will work on soon enough. You can open a PR with your fix if you'd like, and I would be happy to merge that into main.