Expand CmdResult's API
#1977
Expand CmdResult's API
#1977
Conversation
|
The checks failing here are actually an apparent bug in the tests. |
|
Manually invoking wc also confirms this |
Arcterus
left a comment
Arcterus
left a comment
There was a problem hiding this comment.
LGTM other than the typo. The other comment is more of a suggestion.
tests/common/util.rs
Outdated
| self.code.expect("Program must be run first") | ||
| } | ||
|
|
||
| /// Retunrs the program's TempDir |
tests/common/util.rs
Outdated
| /// whose bytes equal those of the passed in slice | ||
| pub fn stderr_is_bytes<T: AsRef<[u8]>>(&self, msg: T) -> Box<&CmdResult> { | ||
| assert_eq!(self.stderr.as_bytes(), msg.as_ref()); | ||
| Box::new(self) |
There was a problem hiding this comment.
Can you try to remove the Boxes? I think they must be a remnant of a change from long ago.
|
@Arcterus should I also remove the |
|
Yeah, please do. |
| pub stdout: String, | ||
| /// captured utf-8 standard error after running the Command | ||
| /// captured standard error after running the Command | ||
| pub stderr: String, |
There was a problem hiding this comment.
Shouldn't this be stored as Vec<u8>? Same for stdout.
There was a problem hiding this comment.
Shouldn't this be stored as
Vec<u8>? Same forstdout.
That's the big problem. A lot of tests depend on the fields of CmdResult.
Changing this right now would break the whole build. So, as mentioned in the original PR comment,
for now, we just add a bunch of public methods.
After we have refactored the tests to only use these functions, we can make the fields private and change the type.
The refactor shouldn't take more than 1-2 days.
There was a problem hiding this comment.
Ah, I somehow missed your steps in the original comment. I suppose that's fine then.
2 participants
This PR adds a bunch of useful functions & byte/string getters for the CmdResult struct.
It also adds methods to rw+ items in an
AtPath.Now, we need to refactor tests to use this API (together with
stdout_isfor example instead of manual asserts, andsucceedsinstead ofassert!(result.success). Once that is done, we canstderr/stdoutinCmdResulttoVec<u8>StringwithVec<u8>for stdout (stderr?) #1916