Skip to content

pass features to doctest binary #1284

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

Merged
merged 2 commits into from
Feb 14, 2015
Merged

pass features to doctest binary #1284

merged 2 commits into from
Feb 14, 2015

Conversation

apasel422
Copy link
Contributor

This is a WIP for #1277. I would like to add a test for the new behavior in tests/test_cargo_test.rs, but I'm having a hard time getting the output to match.

Some feedback would be great to see if this is even the right approach.

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see CONTRIBUTING.md for more information.

@apasel422
Copy link
Contributor Author

This is definitely not correct as is, because it doesn't handle default features.

@alexcrichton
Copy link
Member

Could you add some tests for this as well? I think that this does handle default features but it would be good to have a test showing so. You may also want to store a sorted Vec<String> to match against the exact output.

@apasel422
Copy link
Contributor Author

Sure. I'm just trying to figure out how to use the test stuff correctly. What I have right now is failing (see the second commit):

---- test_cargo_test::doctest_feature stdout ----
    thread 'test_cargo_test::doctest_feature' panicked at '
Expected: execs
    but: differences:
  9 - |running 1 test|
    + |running 0 tests|

 10 - |test bar_0 ... ok|
    + ||

 11 - ||
    + |test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured|

 12 - |test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured|
    + ||

@apasel422
Copy link
Contributor Author

This doesn't appear to be the solution, or, at least, there is a separate bug in rustdoc:

Doc-tests foo
     Running `rustdoc --test /home/andrew/foo/src/lib.rs --crate-name foo -L /home/andrew/foo/target -L /home/andrew/foo/target/deps --cfg feature="bar" --extern foo=/home/andrew/foo/target/libfoo-7cf71ac9a7d9fc8e.rlib`

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured

@alexcrichton
Copy link
Member

If this is an upstream rustdoc bug we may want to fix it there before adding support in Cargo first.

@apasel422
Copy link
Contributor Author

I think I've narrowed the problem down to the way cargo is building the library and then invoking rustdoc with it, just gathering some more evidence.

@apasel422
Copy link
Contributor Author

See rust-lang/rust#22131.

@apasel422
Copy link
Contributor Author

I should be able to add a test for this after Cargo updates to Rust master.

@alexcrichton
Copy link
Member

If you edit src/rustversion.txt to have today's date it should do the trick (update to master)

@apasel422
Copy link
Contributor Author

Updated with a test.

@alexcrichton
Copy link
Member

@bors: r+ b522d0f

Thanks!

@bors
Copy link
Contributor

bors commented Feb 11, 2015

⌛ Testing commit b522d0f with merge 200ae94...

@bors
Copy link
Contributor

bors commented Feb 11, 2015

💔 Test failed - cargo-mac-64

@apasel422
Copy link
Contributor Author

Not sure what's going on with the test failure.

EDIT: It looks like the linker isn't being told to look in builder/target. It's weird because almost identical code in tests/test_cargo_compile_custom_build.rs seems to work just fine.

@alexcrichton
Copy link
Member

I believe this is a bug in the compiler: rust-lang/rust#22197

@apasel422
Copy link
Contributor Author

Rebased on #1300.

@alexcrichton
Copy link
Member

@bors: r+ 256faff

@bors
Copy link
Contributor

bors commented Feb 14, 2015

⌛ Testing commit 256faff with merge 3f74d7e...

bors added a commit that referenced this pull request Feb 14, 2015
This is a WIP for #1277. I would like to add a test for the new behavior in `tests/test_cargo_test.rs`, but I'm having a hard time getting the output to match.

Some feedback would be great to see if this is even the right approach.
@bors
Copy link
Contributor

bors commented Feb 14, 2015

☀️ Test successful - cargo-linux-32, cargo-linux-64, cargo-mac-32, cargo-mac-64, cargo-win-32, cargo-win-64

@bors bors merged commit 256faff into rust-lang:master Feb 14, 2015
@apasel422 apasel422 deleted the issue-1277 branch February 14, 2015 01:54
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.

4 participants