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

WIP: Make cargo bin helpers work with empty env #54

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

killercup
Copy link
Collaborator

Currently based on #50 and very much WIP.

epage and others added 4 commits October 12, 2017 18:40
`clap` couldn't be used for command line arguments.  I didn't want to
add it as a dependency for clients, which meant it had to be optional.
The problem is that `main_binary` and `cargo_binary` run with optional
features enabled.

Note: you cannot use `main_binary` or `cargo_binary`
- With `Environment::empty` because the executable can't be found
- In skeptic tests (like the README) because the working dir is changed
We use cargo run behind the scenes to run the main binary but that
should only be an implementation detail.

Fixes #45
@killercup killercup requested a review from epage October 13, 2017 08:42
Copy link
Collaborator Author

@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.

This is just a first draft.

cc @Freyskeyd: This solves #51 without the need for Freyskeyd/environment#4

@@ -8,6 +8,20 @@ use std::path::PathBuf;
use std::process::{Command, Stdio};
use std::vec::Vec;

fn find_cargo() -> String {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We should should need to panic in this fn at all

@@ -25,11 +39,13 @@ impl default::Default for Assert {
///
/// Defaults to asserting _successful_ execution.
fn default() -> Self {
let cargo_path = find_cargo();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This fn can currently panic. We should deal with the result instead and maybe cache the error in the Assert struct so we can return it only when running execute or unwrap

Copy link
Collaborator

Choose a reason for hiding this comment

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

The main value of caching the result would be for when calling .execute() rather than .unwrap(), right?

Also, should the result of find_cargo be shoved off into a lazy_static!?

@@ -7,6 +8,20 @@ use std::path::PathBuf;
use std::process::{Command, Stdio};
use std::vec::Vec;

fn find_cargo() -> String {
let which_cargo = Command::new("which").arg("cargo")
Copy link
Collaborator

Choose a reason for hiding this comment

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

At this point, we should probably add a Windows CI

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.

2 participants