assert_fs icon indicating copy to clipboard operation
assert_fs copied to clipboard

Call out in docs that you can operate on things besides the built-in temp support

Open epage opened this issue 6 years ago • 3 comments

In case someone wants custom temp dir functionality or some other unknown situation.

From @volks73 at https://github.com/assert-rs/assert_fs/issues/46

I would use assert_fs more if I could upgrade from v0.9. The assert_fs crate wraps the tempfile crate, but does not expose all of the functionality. For example, I need to be able to define a prefix for a temporary directory. The default .tmp prefix is an invalid name for a Cargo package. Windows is also not too keen on file and folder names with leading periods. This means I need to use the tempfile::Builder type to add a custom prefix. There is no Fromtempfile::TempDir trait implementation for assert_fs::TempDir, so I cannot use the tempfile::Builder type to customize a temporary directory and then "cast" it to the assert_fs::TempDir type to gain the assertion-ness and conveniences from the assert_fs crate, nor is there a similar Builder interface available. Thus, I am stuck using v0.9 of assert_fs, which does not wrap tempfile and instead just extends it.

This can be worked around today with ChildPath but the docs never talk about it!

epage avatar Mar 25 '19 22:03 epage

So, the ChildPath workaround would be something like this:

#[test]
fn default_works() {
    let temp_package = common::create_test_package();
    let package = ChildPath::new(temp_package.path());
    // ...
}

which would require updating/modifying every test?

I understand this is a workaround, but it would be nicer if I could just change the create_test_package implementation:

#[allow(dead_code)]
pub fn create_test_package() -> assert_fs::TempDir {
    let temp_dir = tempfile::Builder::new().prefix("cargo_wix_test_").tempdir().unwrap();
    let cargo_init_status = Command::new("cargo")
        .arg("init")
        .arg("--bin")
        .arg("--quiet")
        .arg("--vcs")
        .arg("none")
        .arg(temp_dir.path())
        .status()
        .expect("Creation of test Cargo package");
    assert!(cargo_init_status.success());
    temp_dir.into()
    //
    // or
    // assert_fs::TempDir::from(temp_dir)
    //
    // or do the "upcast" earlier with:
    //
    // let temp_dir = assert_fs::TempDir::from(tempfile::Builder::new().prefix("cargo_wix_test_"));
}

Justification for needing the prefix and "special" temporary directory name is given in #46.

volks73 avatar Mar 26 '19 21:03 volks73

I know it has been awhile, but a brief update on this since it is still open.

I have upgraded to v0.11 and changed the create_test_package function to use the --name <NAME> option for the cargo init command to get around the limitations of naming cargo projects/packages with periods as discussion in #46 . The following is the current implementation of the create_test_package function:

#[allow(dead_code)]
pub fn create_test_package() -> TempDir {
    let temp_dir = TempDir::new().unwrap();
    let cargo_init_status = Command::new("cargo")
        .arg("init")
        .arg("--bin")
        .arg("--quiet")
        .arg("--vcs")
        .arg("none")
        .arg("--name")
        .arg(PACKAGE_NAME)
        .arg(temp_dir.path())
        .status()
        .expect("Creation of test Cargo package");
    assert!(cargo_init_status.success());
    temp_dir
}

The PACKAGE_NAME is a constant that can only contain letters or numbers because of naming limitations in the WiX Toolset compiler and linker (candle and light). This avoided having to change all of the tests with the above mentioned workaround using the ChildPath type. Although, I did initially try this and it worked.

Side note, on Windows 10, the temporary directory does have a leading period, but this no longer appears to be a problem. I think there were some recent updates to Windows Explorer that better handle leading periods in file and folder names and any problems I was experiencing with this were specific to an older version of Windows I was runing.

I still think it would be nice to have from conversion functionality from the tempfile::Builder so that the options available within the Builder are available to the assert_fs::TempDir without having to re-implement the Builder functionality. Should I create a separate issue to add this functionality? Would you like/accept a PR with this feature added? I might have some time to take a look into implementing.

volks73 avatar Sep 03 '19 17:09 volks73

I still think it would be nice to have from conversion functionality from the tempfile::Builder so that the options available within the Builder are available to the assert_fs::TempDir without having to re-implement the Builder functionality. Should I create a separate issue to add this functionality? Would you like/accept a PR with this feature added? I might have some time to take a look into implementing.

Could you create a separate issue for that? I'll then copy over relevant parts from #46

I'm still hesitant about exposing tempfile in assert_fss public API.

epage avatar Sep 03 '19 17:09 epage