Wasi test suite cleanups (#7553)

* rename preview1_remove_directory_trailing_slashes to preview1_remove_directory

and get rid its testing of the inconsistient behavior of removing a
directory using a trailing slash.

Then, unmark it as should_panic everywhere, so we have test coverage of
the behaviors that are consistient

prtest:full

* delete preview1_path_rename_file_trailing_slashes

this only tests behavior which is not consistient across platforms. the
rest of the behavior of path_rename is covered well by the path_rename
test and path_rename_dir_trailing_slashes

* debugging: see what CI tells me the preview1_interesting_paths chokes on in windows

* more debugging information

* interesting paths: windows allows a trailing nul apparently

* preview1_remove_directory: every os says notdir, apparently!

* no more should_panic tests!

crates/test-programs/src/bin/preview1_interesting_paths.rs

@@ -35,12 +35,12 @@ unsafe fn test_interesting_paths(dir_fd: wasi::Fd, arg: &str) {
     );
     wasi::fd_close(file_fd).expect("closing a file");
 
-    // Now open it with a trailing NUL.
+    // Now open it with a trailing NUL. Windows will allow this.
     assert_errno!(
         wasi::path_open(dir_fd, 0, "dir/nested/file\0", 0, 0, 0, 0)
             .expect_err("opening a file with a trailing NUL"),
-        wasi::ERRNO_INVAL,
-        wasi::ERRNO_ILSEQ
+        unix => wasi::ERRNO_INVAL,
+        windows => wasi::ERRNO_NOENT
     );
 
     // Now open it with a trailing slash.

crates/test-programs/src/bin/preview1_path_rename_file_trailing_slashes.rs

@@ -1,53 +0,0 @@
-use std::{env, process};
-use test_programs::preview1::{assert_errno, create_file, open_scratch_directory};
-
-unsafe fn test_path_rename_trailing_slashes(dir_fd: wasi::Fd) {
-    // Test renaming a file with a trailing slash in the name.
-    create_file(dir_fd, "source");
-
-    wasi::path_rename(dir_fd, "source", dir_fd, "target")
-        .expect("no trailing slashes rename works");
-    wasi::path_rename(dir_fd, "target", dir_fd, "source").expect("rename it back to source");
-
-    assert_errno!(
-        wasi::path_rename(dir_fd, "source/", dir_fd, "target")
-            .expect_err("renaming a file with a trailing slash in the source name should fail"),
-        wasi::ERRNO_NOTDIR
-    );
-    assert_errno!(
-        wasi::path_rename(dir_fd, "source", dir_fd, "target/").expect_err(
-            "renaming a file with a trailing slash in the destination name should fail"
-        ),
-        wasi::ERRNO_NOTDIR
-    );
-    assert_errno!(
-        wasi::path_rename(dir_fd, "source/", dir_fd, "target/").expect_err(
-            "renaming a file with a trailing slash in the source and destination names should fail"
-        ),
-        wasi::ERRNO_NOTDIR
-    );
-    wasi::path_unlink_file(dir_fd, "source").expect("removing a file");
-}
-
-fn main() {
-    let mut args = env::args();
-    let prog = args.next().unwrap();
-    let arg = if let Some(arg) = args.next() {
-        arg
-    } else {
-        eprintln!("usage: {} <scratch directory>", prog);
-        process::exit(1);
-    };
-
-    // Open scratch directory
-    let dir_fd = match open_scratch_directory(&arg) {
-        Ok(dir_fd) => dir_fd,
-        Err(err) => {
-            eprintln!("{}", err);
-            process::exit(1)
-        }
-    };
-
-    // Run the tests.
-    unsafe { test_path_rename_trailing_slashes(dir_fd) }
-}

crates/test-programs/src/bin/preview1_remove_directory_trailing_slashes.rs → crates/test-programs/src/bin/preview1_remove_directory.rs

@@ -1,7 +1,7 @@
 use std::{env, process};
 use test_programs::preview1::{assert_errno, create_file, open_scratch_directory};
 
-unsafe fn test_remove_directory_trailing_slashes(dir_fd: wasi::Fd) {
+unsafe fn test_remove_directory(dir_fd: wasi::Fd) {
     // Create a directory in the scratch directory.
     wasi::path_create_directory(dir_fd, "dir").expect("creating a directory");
 
@@ -9,11 +9,9 @@ unsafe fn test_remove_directory_trailing_slashes(dir_fd: wasi::Fd) {
     wasi::path_remove_directory(dir_fd, "dir")
         .expect("remove_directory on a directory should succeed");
 
-    wasi::path_create_directory(dir_fd, "dir").expect("creating a directory");
-
-    // Test that removing it with a trailing slash succeeds.
-    wasi::path_remove_directory(dir_fd, "dir/")
-        .expect("remove_directory with a trailing slash on a directory should succeed");
+    // There isn't consistient behavior across operating systems of whether removing with a
+    // directory where the path has a trailing slash succeeds or fails, so we won't test
+    // that behavior.
 
     // Create a temporary file.
     create_file(dir_fd, "file");
@@ -29,8 +27,7 @@ unsafe fn test_remove_directory_trailing_slashes(dir_fd: wasi::Fd) {
     assert_errno!(
         wasi::path_remove_directory(dir_fd, "file/")
             .expect_err("remove_directory with a trailing slash on a file should fail"),
-        unix => wasi::ERRNO_NOTDIR,
-        windows => wasi::ERRNO_NOENT
+        wasi::ERRNO_NOTDIR
     );
 
     wasi::path_unlink_file(dir_fd, "file").expect("removing a file");
@@ -56,5 +53,5 @@ fn main() {
     };
 
     // Run the tests.
-    unsafe { test_remove_directory_trailing_slashes(dir_fd) }
+    unsafe { test_remove_directory(dir_fd) }
 }

crates/wasi-common/tests/all/async_.rs

@@ -75,8 +75,7 @@ async fn run(path: &str, inherit_stdio: bool) -> Result<()> {
 }
 
 // Below here is mechanical: there should be one test for every binary in
-// wasi-tests. The only differences should be should_panic annotations for
-// tests which fail.
+// wasi-tests.
 #[test_log::test(tokio::test(flavor = "multi_thread"))]
 async fn preview1_big_random_buf() {
     run(PREVIEW1_BIG_RANDOM_BUF, true).await.unwrap()
@@ -146,7 +145,6 @@ async fn preview1_file_unbuffered_write() {
     run(PREVIEW1_FILE_UNBUFFERED_WRITE, true).await.unwrap()
 }
 #[test_log::test(tokio::test(flavor = "multi_thread"))]
-#[cfg_attr(windows, should_panic)]
 async fn preview1_interesting_paths() {
     run(PREVIEW1_INTERESTING_PATHS, true).await.unwrap()
 }
@@ -201,13 +199,6 @@ async fn preview1_path_rename_dir_trailing_slashes() {
         .unwrap()
 }
 #[test_log::test(tokio::test(flavor = "multi_thread"))]
-#[should_panic]
-async fn preview1_path_rename_file_trailing_slashes() {
-    run(PREVIEW1_PATH_RENAME_FILE_TRAILING_SLASHES, false)
-        .await
-        .unwrap()
-}
-#[test_log::test(tokio::test(flavor = "multi_thread"))]
 async fn preview1_path_rename() {
     run(PREVIEW1_PATH_RENAME, true).await.unwrap()
 }
@@ -230,11 +221,8 @@ async fn preview1_readlink() {
     run(PREVIEW1_READLINK, true).await.unwrap()
 }
 #[test_log::test(tokio::test(flavor = "multi_thread"))]
-#[should_panic]
-async fn preview1_remove_directory_trailing_slashes() {
-    run(PREVIEW1_REMOVE_DIRECTORY_TRAILING_SLASHES, false)
-        .await
-        .unwrap()
+async fn preview1_remove_directory() {
+    run(PREVIEW1_REMOVE_DIRECTORY, true).await.unwrap()
 }
 #[test_log::test(tokio::test(flavor = "multi_thread"))]
 async fn preview1_remove_nonempty_directory() {

crates/wasi-common/tests/all/sync.rs

@@ -67,8 +67,7 @@ fn run(path: &str, inherit_stdio: bool) -> Result<()> {
 }
 
 // Below here is mechanical: there should be one test for every binary in
-// wasi-tests. The only differences should be should_panic annotations for
-// tests which fail.
+// wasi-tests.
 #[test_log::test]
 fn preview1_big_random_buf() {
     run(PREVIEW1_BIG_RANDOM_BUF, true).unwrap()
@@ -138,7 +137,6 @@ fn preview1_file_unbuffered_write() {
     run(PREVIEW1_FILE_UNBUFFERED_WRITE, true).unwrap()
 }
 #[test_log::test]
-#[cfg_attr(windows, should_panic)]
 fn preview1_interesting_paths() {
     run(PREVIEW1_INTERESTING_PATHS, true).unwrap()
 }
@@ -191,11 +189,6 @@ fn preview1_path_rename_dir_trailing_slashes() {
     run(PREVIEW1_PATH_RENAME_DIR_TRAILING_SLASHES, true).unwrap()
 }
 #[test_log::test]
-#[should_panic]
-fn preview1_path_rename_file_trailing_slashes() {
-    run(PREVIEW1_PATH_RENAME_FILE_TRAILING_SLASHES, false).unwrap()
-}
-#[test_log::test]
 fn preview1_path_rename() {
     run(PREVIEW1_PATH_RENAME, true).unwrap()
 }
@@ -216,9 +209,8 @@ fn preview1_readlink() {
     run(PREVIEW1_READLINK, true).unwrap()
 }
 #[test_log::test]
-#[should_panic]
-fn preview1_remove_directory_trailing_slashes() {
-    run(PREVIEW1_REMOVE_DIRECTORY_TRAILING_SLASHES, false).unwrap()
+fn preview1_remove_directory() {
+    run(PREVIEW1_REMOVE_DIRECTORY, true).unwrap()
 }
 #[test_log::test]
 fn preview1_remove_nonempty_directory() {

crates/wasi/tests/all/async_.rs

@@ -26,8 +26,7 @@ foreach_preview1!(assert_test_exists);
 foreach_preview2!(assert_test_exists);
 
 // Below here is mechanical: there should be one test for every binary in
-// wasi-tests. The only differences should be should_panic annotations for
-// tests which fail.
+// wasi-tests.
 #[test_log::test(tokio::test(flavor = "multi_thread"))]
 async fn preview1_big_random_buf() {
     run(PREVIEW1_BIG_RANDOM_BUF_COMPONENT, false).await.unwrap()
@@ -111,9 +110,8 @@ async fn preview1_file_unbuffered_write() {
         .unwrap()
 }
 #[test_log::test(tokio::test(flavor = "multi_thread"))]
-#[cfg_attr(windows, should_panic)]
 async fn preview1_interesting_paths() {
-    run(PREVIEW1_INTERESTING_PATHS_COMPONENT, false)
+    run(PREVIEW1_INTERESTING_PATHS_COMPONENT, true)
         .await
         .unwrap()
 }
@@ -184,13 +182,6 @@ async fn preview1_path_rename_dir_trailing_slashes() {
         .unwrap()
 }
 #[test_log::test(tokio::test(flavor = "multi_thread"))]
-#[should_panic]
-async fn preview1_path_rename_file_trailing_slashes() {
-    run(PREVIEW1_PATH_RENAME_FILE_TRAILING_SLASHES_COMPONENT, false)
-        .await
-        .unwrap()
-}
-#[test_log::test(tokio::test(flavor = "multi_thread"))]
 async fn preview1_path_rename() {
     run(PREVIEW1_PATH_RENAME_COMPONENT, false).await.unwrap()
 }
@@ -218,9 +209,8 @@ async fn preview1_readlink() {
     run(PREVIEW1_READLINK_COMPONENT, false).await.unwrap()
 }
 #[test_log::test(tokio::test(flavor = "multi_thread"))]
-#[should_panic]
-async fn preview1_remove_directory_trailing_slashes() {
-    run(PREVIEW1_REMOVE_DIRECTORY_TRAILING_SLASHES_COMPONENT, false)
+async fn preview1_remove_directory() {
+    run(PREVIEW1_REMOVE_DIRECTORY_COMPONENT, false)
         .await
         .unwrap()
 }

crates/wasi/tests/all/preview1.rs

@@ -24,8 +24,7 @@ async fn run(path: &str, inherit_stdio: bool) -> Result<()> {
 foreach_preview1!(assert_test_exists);
 
 // Below here is mechanical: there should be one test for every binary in
-// wasi-tests. The only differences should be should_panic annotations for
-// tests which fail.
+// wasi-tests.
 #[test_log::test(tokio::test(flavor = "multi_thread"))]
 async fn preview1_big_random_buf() {
     run(PREVIEW1_BIG_RANDOM_BUF, false).await.unwrap()
@@ -95,9 +94,8 @@ async fn preview1_file_unbuffered_write() {
     run(PREVIEW1_FILE_UNBUFFERED_WRITE, false).await.unwrap()
 }
 #[test_log::test(tokio::test(flavor = "multi_thread"))]
-#[cfg_attr(windows, should_panic)]
 async fn preview1_interesting_paths() {
-    run(PREVIEW1_INTERESTING_PATHS, false).await.unwrap()
+    run(PREVIEW1_INTERESTING_PATHS, true).await.unwrap()
 }
 #[test_log::test(tokio::test(flavor = "multi_thread"))]
 async fn preview1_regular_file_isatty() {
@@ -152,13 +150,6 @@ async fn preview1_path_rename_dir_trailing_slashes() {
         .unwrap()
 }
 #[test_log::test(tokio::test(flavor = "multi_thread"))]
-#[should_panic]
-async fn preview1_path_rename_file_trailing_slashes() {
-    run(PREVIEW1_PATH_RENAME_FILE_TRAILING_SLASHES, false)
-        .await
-        .unwrap()
-}
-#[test_log::test(tokio::test(flavor = "multi_thread"))]
 async fn preview1_path_rename() {
     run(PREVIEW1_PATH_RENAME, false).await.unwrap()
 }
@@ -182,11 +173,8 @@ async fn preview1_readlink() {
     run(PREVIEW1_READLINK, false).await.unwrap()
 }
 #[test_log::test(tokio::test(flavor = "multi_thread"))]
-#[should_panic]
-async fn preview1_remove_directory_trailing_slashes() {
-    run(PREVIEW1_REMOVE_DIRECTORY_TRAILING_SLASHES, false)
-        .await
-        .unwrap()
+async fn preview1_remove_directory() {
+    run(PREVIEW1_REMOVE_DIRECTORY, false).await.unwrap()
 }
 #[test_log::test(tokio::test(flavor = "multi_thread"))]
 async fn preview1_remove_nonempty_directory() {

crates/wasi/tests/all/sync.rs

@@ -25,8 +25,7 @@ foreach_preview1!(assert_test_exists);
 foreach_preview2!(assert_test_exists);
 
 // Below here is mechanical: there should be one test for every binary in
-// wasi-tests. The only differences should be should_panic annotations for
-// tests which fail.
+// wasi-tests.
 #[test_log::test]
 fn preview1_big_random_buf() {
     run(PREVIEW1_BIG_RANDOM_BUF_COMPONENT, false).unwrap()
@@ -96,7 +95,6 @@ fn preview1_file_unbuffered_write() {
     run(PREVIEW1_FILE_UNBUFFERED_WRITE_COMPONENT, false).unwrap()
 }
 #[test_log::test]
-#[cfg_attr(windows, should_panic)]
 fn preview1_interesting_paths() {
     run(PREVIEW1_INTERESTING_PATHS_COMPONENT, false).unwrap()
 }
@@ -149,11 +147,6 @@ fn preview1_path_rename_dir_trailing_slashes() {
     run(PREVIEW1_PATH_RENAME_DIR_TRAILING_SLASHES_COMPONENT, false).unwrap()
 }
 #[test_log::test]
-#[should_panic]
-fn preview1_path_rename_file_trailing_slashes() {
-    run(PREVIEW1_PATH_RENAME_FILE_TRAILING_SLASHES_COMPONENT, false).unwrap()
-}
-#[test_log::test]
 fn preview1_path_rename() {
     run(PREVIEW1_PATH_RENAME_COMPONENT, false).unwrap()
 }
@@ -175,9 +168,8 @@ fn preview1_readlink() {
     run(PREVIEW1_READLINK_COMPONENT, false).unwrap()
 }
 #[test_log::test]
-#[should_panic]
-fn preview1_remove_directory_trailing_slashes() {
-    run(PREVIEW1_REMOVE_DIRECTORY_TRAILING_SLASHES_COMPONENT, false).unwrap()
+fn preview1_remove_directory() {
+    run(PREVIEW1_REMOVE_DIRECTORY_COMPONENT, false).unwrap()
 }
 #[test_log::test]
 fn preview1_remove_nonempty_directory() {