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

required-features implicit binaries for test does not check for dep syntax #7853

Closed
ehuss opened this issue Feb 1, 2020 · 1 comment · Fixed by #8020
Closed

required-features implicit binaries for test does not check for dep syntax #7853

ehuss opened this issue Feb 1, 2020 · 1 comment · Fixed by #8020
Labels
A-features Area: features — conditional compilation A-required-features Area: required-features setting C-bug Category: bug

Comments

@ehuss
Copy link
Contributor

ehuss commented Feb 1, 2020

Problem
A project with a binary with required-features = ["dep_name/feature_name"] will only build the binary when the given dependency's feature is enabled. This feature syntax is not checked when the binary is built as an implicit dependency of an integration test, and thus the binary will not get built during cargo test --features dep_name/feature_name when I think it should.

Steps
The following patch updates one of Cargo's tests to exercise this case.

diff --git a/tests/testsuite/required_features.rs b/tests/testsuite/required_features.rs
index 6afd917ee..7d7b547c3 100644
--- a/tests/testsuite/required_features.rs
+++ b/tests/testsuite/required_features.rs
@@ -4,6 +4,7 @@ use cargo_test_support::install::{
     assert_has_installed_exe, assert_has_not_installed_exe, cargo_home,
 };
 use cargo_test_support::is_nightly;
+use cargo_test_support::paths::CargoPathExt;
 use cargo_test_support::project;

 #[cargo_test]
@@ -910,7 +911,17 @@ fn dep_feature_in_cmd_line() {
         )
         .file("src/main.rs", "fn main() {}")
         .file("examples/foo.rs", "fn main() {}")
-        .file("tests/foo.rs", "#[test]\nfn test() {}")
+        .file(
+            "tests/foo.rs",
+            r#"
+            #[test]
+            fn bin_is_built() {
+                let s = format!("target/debug/foo{}", std::env::consts::EXE_SUFFIX);
+                let p = std::path::Path::new(&s);
+                assert!(p.exists(), "foo does not exist");
+            }
+            "#,
+        )
         .file(
             "benches/foo.rs",
             r#"
@@ -936,7 +947,9 @@ fn dep_feature_in_cmd_line() {
         .file("bar/src/lib.rs", "")
         .build();

-    p.cargo("build").run();
+    // This is a no-op.
+    p.cargo("build").with_stderr("[FINISHED] dev [..]").run();
+    assert!(!p.bin("foo").is_file());

     // bin
     p.cargo("build --bin=foo")
@@ -967,19 +980,23 @@ Consider enabling them by passing, e.g., `--features=\"bar/a\"`
     assert!(p.bin("examples/foo").is_file());

     // test
+    // This is a no-op, since no tests are enabled.
     p.cargo("test")
         .with_stderr("[FINISHED] test [unoptimized + debuginfo] target(s) in [..]")
         .with_stdout("")
         .run();

+    // Delete the target directory so this can check if the main.rs gets built.
+    p.build_dir().rm_rf();
     p.cargo("test --test=foo --features bar/a")
         .with_stderr(
             "\
+[COMPILING] bar v0.0.1 ([CWD])
 [COMPILING] foo v0.0.1 ([CWD])
 [FINISHED] test [unoptimized + debuginfo] target(s) in [..]
 [RUNNING] target/debug/deps/foo-[..][EXE]",
         )
-        .with_stdout_contains("test test ... ok")
+        .with_stdout_contains("test bin_is_built ... ok")
         .run();

     // bench

Possible Solution(s)
The implicit binary check is done here. This does not check for the extended feature syntax, which is normally done here. Ideally the code for checking required-features should be shared.

Notes

cargo 1.42.0-nightly (f6449ba 2020-01-21)

@ehuss ehuss added C-bug Category: bug A-features Area: features — conditional compilation A-required-features Area: required-features setting labels Feb 1, 2020
@xiongmao86
Copy link
Contributor

If no one is working on this, I'll try to take a stab.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-features Area: features — conditional compilation A-required-features Area: required-features setting C-bug Category: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants