Repository: taiki-e/cargo-hack (language: rust)

Issue:
request: Add a `--skip` subcommand
## Motivation

`--feature-powerset` was added recently and it has been really helpful.
Unfortunately, if there is a feature that doesn't need to be tested in the
combinations it's not possible to skip. Each additional feature that is tested
in the powerset doubles the run-time so it can be costly as the number of
features grows.

## Solution

Add a `--skip` subcommand that can be paired with `--each-feature` or
`--feature-powerset`. The latter subcommands would filter out any features
passed in `--skip` before running their own logic.

I wanted to open the issue before sending in a PR incase @taiki-e had any
thoughts? If this is something you are okay with adding, I can send in a quick
change to add it. It should be fairly straightforward.



Discussion / hints:
Seems reasonable to me!

Fix patch:
diff --git a/src/cli.rs b/src/cli.rs
index 9b329534..d76b3d60 100644
--- a/src/cli.rs
+++ b/src/cli.rs
@@ -79,6 +79,8 @@ pub(crate) struct Args {
     pub(crate) each_feature: bool,
     /// --feature-powerset
     pub(crate) feature_powerset: bool,
+    /// --skip
+    pub(crate) skip: Vec<String>,
     /// --no-dev-deps
     pub(crate) no_dev_deps: bool,
     /// --remove-dev-deps
@@ -156,6 +158,7 @@ pub(crate) fn args(coloring: &mut Option<Coloring>) -> Result<Option<Args>> {
     let mut package = Vec::new();
     let mut exclude = Vec::new();
     let mut features = Vec::new();
+    let mut skip = Vec::new();
 
     let mut workspace = None;
     let mut no_dev_deps = false;
@@ -251,6 +254,7 @@ pub(crate) fn args(coloring: &mut Option<Coloring>) -> Result<Option<Args>> {
             parse_arg2!(package, false, "-p", "--package <SPEC>");
             parse_arg2!(exclude, false, "--exclude", "--exclude <SPEC>");
             parse_arg2!(features, true, "--features", "--features <FEATURES>");
+            parse_arg2!(skip, true, "--skip", "skip <SKIP>");
 
             match arg.as_str() {
                 "--workspace" | "--all" => {
@@ -327,6 +331,11 @@ pub(crate) fn args(coloring: &mut Option<Coloring>) -> Result<Option<Args>> {
     if !exclude.is_empty() && workspace.is_none() {
         bail!("--exclude can only be used together with --workspace");
     }
+
+    if !skip.is_empty() && (!each_feature && !feature_powerset) {
+        bail!("--skip can only be used with either --each-feature or --feature-powerset");
+    }
+
     if let Some(subcommand) = &subcommand {
         if subcommand == "test" || subcommand == "bench" {
             if remove_dev_deps {
@@ -400,6 +409,7 @@ For more information try --help
         workspace: workspace.is_some(),
         each_feature,
         feature_powerset,
+        skip,
         no_dev_deps,
         remove_dev_deps,
         ignore_private,
diff --git a/src/main.rs b/src/main.rs
index 32d3ba0f..1c9d539a 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -176,15 +176,20 @@ fn features(args: &Args, package: &Package, line: &ProcessBuilder) -> Result<()>
 }
 
 fn each_feature(args: &Args, package: &Package, line: &ProcessBuilder) -> Result<()> {
-    package.features.iter().filter(|(k, _)| *k != "default").try_for_each(|(feature, _)| {
-        let mut line = line.clone();
-        line.append_features(&[feature]);
-        exec_cargo(args, package, &line)
-    })
+    package
+        .features
+        .iter()
+        .filter(|(k, _)| (*k != "default" && !args.skip.contains(k)))
+        .try_for_each(|(feature, _)| {
+            let mut line = line.clone();
+            line.append_features(&[feature]);
+            exec_cargo(args, package, &line)
+        })
 }
 
 fn feature_powerset(args: &Args, package: &Package, line: &ProcessBuilder) -> Result<()> {
-    let features: Vec<&String> = package.features.keys().filter(|k| *k != "default").collect();
+    let features: Vec<&String> =
+        package.features.keys().filter(|k| (*k != "default" && !args.skip.contains(k))).collect();
     let powerset = powerset(&features);
 
     // The first element of a powerset is `[]` so it should be skipped.


Test patch:
diff --git a/tests/test.rs b/tests/test.rs
index 082babe9..2c44d96d 100644
--- a/tests/test.rs
+++ b/tests/test.rs
@@ -534,6 +534,69 @@ fn test_feature_powerset() {
         );
 }
 
+#[test]
+fn test_skip_failure() {
+    let output = cargo_hack()
+        .args(&["hack", "check", "--skip", "a"])
+        .current_dir(test_dir("tests/fixtures/real"))
+        .output()
+        .unwrap();
+
+    output.assert_failure().assert_stderr_contains(
+        "--skip can only be used with either --each-feature or --feature-powerset",
+    );
+}
+
+#[test]
+fn test_each_feature_skip_success() {
+    let output = cargo_hack()
+        .args(&["hack", "check", "--each-feature", "--skip", "a"])
+        .current_dir(test_dir("tests/fixtures/real"))
+        .output()
+        .unwrap();
+
+    output
+        .assert_success()
+        .assert_stderr_contains("running `cargo check` on real")
+        .assert_stderr_contains("running `cargo check --no-default-features` on real")
+        .assert_stderr_contains("running `cargo check --no-default-features --features b` on real")
+        .assert_stderr_contains("running `cargo check --no-default-features --features c` on real")
+        .assert_stderr_not_contains(
+            "running `cargo check --no-default-features --features a` on real",
+        );
+}
+
+#[test]
+fn test_powerset_skip_success() {
+    let output = cargo_hack()
+        .args(&["hack", "check", "--feature-powerset", "--skip", "a"])
+        .current_dir(test_dir("tests/fixtures/real"))
+        .output()
+        .unwrap();
+
+    output
+        .assert_success()
+        .assert_stderr_contains("running `cargo check` on real")
+        .assert_stderr_contains("running `cargo check --no-default-features` on real")
+        .assert_stderr_contains("running `cargo check --no-default-features --features b` on real")
+        .assert_stderr_contains("running `cargo check --no-default-features --features c` on real")
+        .assert_stderr_contains(
+            "running `cargo check --no-default-features --features b,c` on real",
+        )
+        .assert_stderr_not_contains(
+            "running `cargo check --no-default-features --features a` on real",
+        )
+        .assert_stderr_not_contains(
+            "running `cargo check --no-default-features --features a,b` on real",
+        )
+        .assert_stderr_not_contains(
+            "running `cargo check --no-default-features --features a,c` on real",
+        )
+        .assert_stderr_not_contains(
+            "running `cargo check --no-default-features --features a,b,c` on real",
+        );
+}
+
 #[test]
 fn test_each_feature2() {
     let output = cargo_hack()