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

feat: add getters to mirror those available on std::process::Command #213

Merged
merged 1 commit into from
Aug 9, 2024

Conversation

mcky
Copy link
Contributor

@mcky mcky commented Aug 8, 2024

As discussed in #212, this adds the following getters that dispatch to the underlying std::process::Command

pub fn get_program(&self) -> &ffi::OsStr {}
pub fn get_args(&self) -> process::CommandArgs<'_> {}
pub fn get_envs<'a>(&'a self) -> process::CommandEnvs<'a> {}
pub fn get_current_dir(&self) -> Option<&path::Path> {}

This is my first rust PR, so apologies if anything is against convention.

@epage I'm not sure which additional getters you meant by

plus ones specialized for the extra content in assert_cmd::Command.

as unless I'm missing something, there aren't any setters that aren't also in std

  • Most of this is liberally copied from std, but I noticed many of the existing doc blocks were also either the same or had significant overlap. I'm happy to trim them down or just write new ones
  • I haven't added any new unit tests as I couldn't see any similar ones / the rest of the suite seems to be more integration test style
  • I left in the doctests and didn't add no_run like many of the other examples (and have verified that they pass correctly)
  • Similarly to std, I grouped the getters at the end. I can change them to be colocated if preferred

src/cmd.rs Show resolved Hide resolved
src/cmd.rs Fixed Show fixed Hide fixed
src/cmd.rs Fixed Show fixed Hide fixed
src/cmd.rs Fixed Show fixed Hide fixed
@coveralls
Copy link

coveralls commented Aug 8, 2024

Pull Request Test Coverage Report for Build 10309787777

Details

  • 0 of 8 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.7%) to 44.581%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/cmd.rs 0 8 0.0%
Totals Coverage Status
Change from base Build 10190508249: -0.7%
Covered Lines: 218
Relevant Lines: 489

💛 - Coveralls

@epage
Copy link
Contributor

epage commented Aug 8, 2024

This is good to go once clippy is resolved. Feel free to squash the fix.

@epage epage enabled auto-merge August 8, 2024 18:21
adds `get_program`, `get_args`, `get_envs` and `get_current_dir` which
all wrap their `std` counterparts
auto-merge was automatically disabled August 8, 2024 21:27

Head branch was pushed to by a user without write access

@mcky mcky force-pushed the feat-add-command-getters branch from feca312 to b037f96 Compare August 8, 2024 21:27
@epage epage merged commit 6539f0a into assert-rs:master Aug 9, 2024
15 checks passed
@mcky mcky deleted the feat-add-command-getters branch August 9, 2024 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants