Error and/or Documentation Error and/or Relative Path Handling Error
I was using a relative path for the fn call directly per doc example for Then::body_from_file
But environment variable not found error popped up instead:
Cannot create absolute path from string 'x': "environment variable not found"
I tried to look whether the doc said much about environment variables but nothing to evaluate.
However Looking into I can find the call to crate::util::get_test_resource_file_path() when path.is_absolute is false:
match env::var("CARGO_MANIFEST_DIR") {Problem is this env may not be visible to cargo sub-commands as I found the hard way with tarpaulin:
env RUN_MODE=development cargo +nightly tarpaulin --run-types Tests,Doctests,Benchmarks,Examples,Lib,Bins -v
- Works okay with cargo test but tarpaulin sub-command seems to omit that environment variable.
- Omitting the env seems to be the case with cargo sub-command.
- Cargo documentation only lists one env passed down without explicitly ruling out others.
Doc wise Mock::return_body_from_file says explicitly either relative/absolute:
resource_file_path - The file path to the file with the response body content. The file path can either be absolute or relative to the project root directory.
As for relative path definition, I would expect this to be the OsStr<> from the Current working directory (CWD) which may be set to be elsewhere than the directory holding the cargo manifest.
For backwards compatibility the document probably should say that the environment variable is used for relative path or behavior could be changed some way to rely on CWD that would break things relying on it to be the manifest path
Plus CWD typically is not UTF-8 guaranteed path if Into is relied (not absolutely sure whether CARGO_MANIFEST_DIR is guaranteed to be either for lossless conversion Into either if it's allowed to be non UTF-8 🔢 ?)
I personally use this pattern and document the var_os OssStr returning environment variable use for my app:
let mut path_locator = match env::var_os("APP_CONFIG_PATH") {
Some(val) => PathBuf::from(val),
None => env::current_dir().unwrap()
};If I feel I need to enforce UTF-8 paths like Rust ecosystem like cargo does I use camino Utf8Path to see if I can upgrade from Path and match appropriate error.
Happy to submit a PR to fix doc and automatically check the truth whatever is decided when/if there is a decision what to do with this.
Code Blocks Involved
Then::body_from_file(httpmock/0.5.8/source/src/lib.rs)
pub fn body_from_file<S: Into<String>>(self, body: S) -> Self {
self.mock.set(self.mock.take().return_body_from_file(body));Mock::return_body_from_file(httpmock/0.5.8/source/src/api/mock.rs):around 1317
pub fn return_body_from_file<S: Into<String>>(mut self, resource_file_path: S) -> Self {
let resource_file_path = resource_file_path.into();
let path = Path::new(&resource_file_path);
let absolute_path = match path.is_absolute() {
true => path.to_path_buf(),
false => get_test_resource_file_path(&resource_file_path).expect(&format!(
"Cannot create absolute path from string '{}'",
//--snip--Dilemma
- Documentation advertises/promises both the relative and absolute path use
- Relative path is implicitly derived from documentation as current working directory (CWD)
- If relative path is used CARGO_MANIFEST_DIR is used as base path which may or may not be lossless Into
- Cargo may or may not pass this env down to sub-command
- std::String is always UTF-8
- std::ffi::OsString implements Into String type but is lossy and breaks relative path guarantee
- OsString and Path is supposed to work everywhere safely abstracting it
- Environmental variable etc. can be non-UTF8 and requires appropriate handling boilerplate before it hits Then::body_from_file
- Lossy conversion is due to POSIX filenames allowing anything except \0
- Requires conversion to String for the whole path (if I read the code right🙆♀️)
- String/std::path::Path conversion has been always a pain
- Many OS allows wide array of bytes in OsStr
Solution 1: Allow From &std::path::Path without using CARGO env
Pros:
- Straightforward pattern is to pass std::path::Path directly e.g. with config::from
- Allows relative path handling at ease over "somebody else handles it" abstraction
- Library already uses std::Path internally despite Into String
- Library using this can handle easily cross-platform TEST_DATA path which may contain "" or "/" path separators etc.
- The world is not perfect problem is easier - less friction to use the library esp re: relative path
- Documentation is the source of truth, though implicit Type impl loses (String is only UTF-8)
- Doesn't rely on cargo env
Cons:
- Rust configured paths are UTF-8
- Lossy Display/Debug
- Either confusing implementation if String is still allowed
- Libs using may/will break if on same method - needs new method
- Current working directory derived from var_os needs to be used instead of CARGO env
Example is using config::File::from
impl<'a> From<&'a Path> for File<source::file::FileSourceFile> {
fn from(path: &'a Path) -> Self {
File {
source: source::file::FileSourceFile::new(path.to_path_buf()),
//--snip--Where I can easily construct and pass the full Path natively and use relative path if I want
let mut path_locator = match env::var_os("CONFIG_PATH") {
Some(val) => PathBuf::from(val),
None => env::current_dir().unwrap()
};
path_locator.push("subdir");
// And then finally merge the file I want with full POSIX compatible Path
s.merge(File::from(path_locator.join("default")).required(false))?;Solution 2: Enforce and be explicit about UTF-8 and handle Error thru camino
Pros:
- Proper taint check and error
- Lossless Display/Debug
- Less friction for anyone who uses the library as it's checked for validity
- Does not need new method as it was always UTF-8
- Document implicit Type impl wins to some degree (String is only UTF-8 and we are just enforcing that)
Cons:
- Relative path use is a pattern sadly good or bad.
- Friction for anyone who uses the library
- Library using this have to handle cross platform paths for TEST_DATA path which may contain "" or "/" path separators etc.
- Adds camino crate that handles sanity on Utf8Paths
- Documentation which advertised flexibility between relative/absolute loses
Solution 3: New fn pass by Buffer/Stream/Channel oneshot etc.
Pros:
- Many async libs just wait-read the whole buffer internally and then process/pass the whole of it
- Would allow timing and replaying a server behaviour in timeline basis for writing into kernel buffer out
- Implementation could say replay sequence from libpcap file the near timing packets were either sent/recvd
- Could differentiate between Content-Length, gzip stream, chunked modes
- WebSocket etc. streaming
- Can serve as building block for HTTP/2 or gRPC/Protobuffers
- More fun than simple singular delay :)
Cons:
- Library using this have to handle cross platform paths for TEST_DATA path which may contain "" or "/" path separators etc.
- Library was never intended for buffer/stream handling?
- Complexity
Solution 4: Status quo
Pros:
- Supposed to have less burden for library
Cons:
- Friction to use the library
- Library using this have to handle cross platform paths for TEST_DATA path which may contain "" or "/" path separators etc.
- Misleading error (library is not using environment variable, the full path is passed to it)
- Still using lossy Display/Debug as it is using Path internally
Solution is probably somekind combination?
I usually do both integration / units along my /// docs or even on README.md that gets included in test run and tarpaulin --run-types DocTests takes those into account
#[cfg(doctest)]
doc_comment::doctest!("../README.md");I will check with tarpaulin too to expose the environment var but it would be nice to get some clarity as it caused a bit confusion initially for me and not sure if there is anyone doing things without cargo env :)
React
Typescript
Django
Laravel
Facebook
Microsoft
Google
Alibaba
D3
Tencent