Skip to content
This repository has been archived by the owner on Dec 29, 2021. It is now read-only.

WIP: Holistic CLI test harness experiments #98

Closed
wants to merge 4 commits into from

Conversation

epage
Copy link
Collaborator

@epage epage commented Mar 27, 2018

This includes

  • Experiments with extension traits, opening the door for duct, see Use duct internally? #32
  • API for quickly setting up a file system for testing, mostly inspired by my work on cobalt ("I'd like to test X but that'll be too much work to set up a fixture. Maybe its good enough...").
  • Comparisons with cli_test_dir, another holistic harness that is more rigid in its API

See commit messages for more details

Work to do

  • Replace Asserts ability to run commands with traits
  • Make it easy to setup a tempdir
  • Add assertion predicates
  • Determine crate(s) structure
  • Tests and documentation

@azriel91
Copy link
Contributor

btw, tempdir is now merged into tempfile 3.0.0 and was published 2 days ago.

This is an experiment in trying to use extension traits rather than
wrapping `process::Command`.  This both makes it more extensible (can
interop with other crates) and able to be adapted to other "Command"
crates like `duct`.

`cli_test_dir` has something like `CommandStdInExt` called `CommandExt`.  Differences include:
- Scoped name since traits generally are pulled out of any namespace
  they are in.
- Preserves the command and `stdin` to for richer error reporting.
This is an experiment in what kind of tempdir operations a holistic CLI
testing framework might provide, following on the previous experiments
with extension traits. The exact structure in this crate or across
crates is TBD.

This crate extends `TempDir` with the following
- In TempDir or a child path, run a command.
- On child path, touch a file.
- On child path, write a binary blob or str to file.
- Copy to a TempDir or a child path some files.

Some other potential operations include
- `write_yml(serde)`
- `write_json(serde)`
- `write_toml(serde)`

In contrast, `cli_test_dir` can:
- Run a single pre-defined program within the tempdir
- Write binary files to tempdir
- Offer a absolute path to a child file within the crate source (so its
  safe to pass to the program running in the tempdir).
@killercup
Copy link
Collaborator

fyi @nikomatsakis just told me he used assert_cli and manually added a tempdir wrapper because that was missing


impl CommandCargoExt for process::Command {
fn main_binary() -> Self {
let mut cmd = process::Command::new("carg");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

carg -> cargo?
Same in line 51

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Obviously very little testing has been done which might be clear considering this is here :). This is a rough sketch to get high level feedback before actually getting the details right.

@epage
Copy link
Collaborator Author

epage commented May 8, 2018

My current thought on crate organization

  • assert_cmd for writing asserts on std::process::Command
  • assert_fs for tempfile / path assertions
  • assert_cli that re-exports both of the above

Couple questions from that

  • I assume some people would appreciate process::Command helpers that this PR provides but aren't interested in assertions. Should we advertise that we provide these or create yet another crate for them?
    • passing in a Vec<u8> for stdin
    • Turning an Command / Output into a Result
  • Currently, the tempfile parts have a distinct struct for playing with files in the tempdir. The cmd parts then support using that to run a command in that directory. This might run afoul of trait rules if we split this into multiple crates.
    • Will it?
    • If it will, should we just switch to attaching these extension traits to Path instead?

Copy link
Collaborator

@killercup killercup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool! Sorry it took me so long to around to this, @epage.

I've long been in favor of splitting assert_cli up into smaller pieces. What we should also offer in the long run, though, is a way to easily extend this, e.g., to make shortcut methods to set a bunch of env vars.

/// ```
fn main_binary() -> Self;

/// Create a `Command` Run a specific binary of the current crate.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/Run/to run

/// `std::process::Output` represented as a `Result`.
pub type OutputResult = Result<process::Output, OutputError>;

/// Extends `std::process::Output` with methods to to convert it to an `OutputResult`.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/to to/to

///
/// By default, stdin, stdout and stderr are inherited from the parent.
///
/// *(mirrors `std::process::Command::spawn`**
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/**/)*


/// Extend `TempDir` to run commands in it.
pub trait TempDirCommandExt {
/// Constructs a new Command for launching the program at path program, with the following
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/path program/path

/// temp.command("pwd").output().unwrap();
/// temp.close().unwrap();
/// ```
fn command<S>(&self, program: S) -> process::Command
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'd probably call this exec?

@epage
Copy link
Collaborator Author

epage commented May 29, 2018

@epage epage closed this May 29, 2018
@epage epage deleted the trait branch May 29, 2018 12:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants