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

Add conversion from tempfile::TempDir to assert_fs::TempDir #54

Closed
volks73 opened this issue Sep 3, 2019 · 4 comments
Closed

Add conversion from tempfile::TempDir to assert_fs::TempDir #54

volks73 opened this issue Sep 3, 2019 · 4 comments
Labels
question Uncertainty is involved

Comments

@volks73
Copy link

volks73 commented Sep 3, 2019

As mentioned and requested in #48, sometimes it might be necessary or needed to customize the temporary directory generated during tests. For example, the tempfile crate provides the tempfile::Builder type for customizing the temporary directory. Similar customizations are not possible with the assert_fs::TempDir.

As a way to avoid re-implementation of the tempfile::Builder customizations, a From implementation could be added to the assert_fs::TempDir type. Two separate From implementations could be added: (1) tempfile::TempDir and (2) tempfile::Builder. This would allow for the following:

let ex_temp_dir_1 = assert_fs::TempDir::from(tempfile::Builder::new().prefix("hello-world-").build());
// and/or
let ex_temp_dir_2 = assert_fs::TempDir::from(tempfile::Builder::new().prefix("hello-world-again-"));

This also means the complementary Into implementations would also exist.

I do recognize the majority use-case is to just create a "default" temporary directory, but this would add some functionality and flexibility for more uncommon test cases without having to re-implement anything.

@epage
Copy link
Contributor

epage commented Sep 3, 2019

I made a non-configurable TempDir with the assumption that this would be a root folder people would be operating under and the name or other characteristics wouldn't matter.

In #46, @volks73 use case was to call cargo init with the project name inferred from the directory. The main workarounds being

  • Create a custom subdirectory
  • Pass --name

On the other hand, if we expose TempDir, that puts tempfile in our public API and we'd need to bump major when they do. So far they are on v3.

So I'm a bit concerned about how much value this adds in consequence to the cost.

@epage epage added the question Uncertainty is involved label Sep 3, 2019
@volks73
Copy link
Author

volks73 commented Sep 3, 2019

Yes, the more I think about this, it is probably an extreme edge case and the other workarounds are perfectly reasonable, but more for my own edification, can you explain a little bit more about the following consequence:

On the other hand, if we expose TempDir, that puts tempfile in our public API and we'd need to bump major when they do. So far they are on v3.

Why is it bad to have tempfile in the public API and isn't tempfile::TempDir also "read-only"? The tempfile crate is used internally, so it already has to be included as dependency. Adding the From implementations does not require any changes to the current API or usage, it would be a small, backwards-compatible modification. I think I am missing the justification for creating a wrapper type around tempfile::TempDir in v0.11 instead of extending it like in v0.9.

Again, this is just for my own benefit as I try to learn and apply various crate and API conventions for my own packages and projects. Thank you!

@epage
Copy link
Contributor

epage commented Sep 4, 2019

Why is it bad to have tempfile in the public API and isn't tempfile::TempDir also "read-only"? The tempfile crate is used internally, so it already has to be included as dependency. Adding the From implementations does not require any changes to the current API or usage, it would be a small, backwards-compatible modification. I think I am missing the justification for creating a wrapper type around tempfile::TempDir in v0.11 instead of extending it like in v0.9.

Currently, tempfile is an implementation detail and we can upgrade to a future tempfile v4 without breaking users.

However, if we create a From<tempfile::TempDir> and we upgrade to a future tempfile v4, then that From will not work with existing clients using tempfile v3, breaking them. This means From implementations expose the related crate in your public API.

@epage
Copy link
Contributor

epage commented Nov 29, 2019

I think at this point I'm going to err on the side of #48 and close this.

@epage epage closed this as completed Nov 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Uncertainty is involved
Projects
None yet
Development

No branches or pull requests

2 participants