Build and test examples when invoking cargo test

[Ryman]

This patch currently only adds new behaviour of testing and doesn't move building of the examples to cargo build as there may be people relying on that behaviour?

How should this be opt-in / Improvement to consider:
During cargo test we're only really using the test flag from the [[example]] config to determine if we should add it to the compilation unit. But, if we don't move the building of the examples to cargo build then perhaps we can use a build flag to determine if the examples get built during cargo test. This would allow only building and not testing examples. If we do that, should example.test be false (opt-in) by default then? (can therefore only activate it in explicit examples?)

[rust-highfive]

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. Due to 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 the contribution instructions for more information.

[alexcrichton]

Thanks for the PR! This tweaks the current behavior where cargo test will compile all examples twice, right? I wonder if we should perhaps have examples with the default of test = false?

[Ryman]

If we set test = false to mean "don't do the test build", I think we still need another flag to let users disable or enable building the examples (current behaviour allows users to disable builds by setting test = false).

Without introducing a new flag, or repurposing a flag, we'll have to choose between test = false means build nothing, while true means test&build, or test = false means only do the building of the example which reduces current behavior (you can currently disable the build if you want).

[bors]

☔ The latest upstream changes (presumably #2670) made this pull request unmergeable. Please resolve the merge conflicts.

[alexcrichton]

Whoa I had no idea that test = false was interpreted as "don't build this example"...

In some sense may unit tests don't belong in unit tests? In theory example unit tests are just unit tests :)

[Ryman]

In some sense may unit tests don't belong in unit tests? In theory example unit tests are just unit tests :)

I'm failing to parse this line 100%, my best guess is: "tests don't belong in examples, examples are a form of unit test" so that's the interpretation I'm replying to:

I think in many cases tests won't make any sense for examples, which is why opt-in is best, I think we both agree there.

For reasons why it can be useful to allow some form of executed tests:

• It's a bit like the difference between no_run or not on doctests, there are cases where the types of things do line up but the logic is flawed in subtle ways that explode quickly at runtime. Having an example not perform as advertised could easily put a user off exploring deeper with a library.
• It can also be a nice signal to users reading the example of 'this is the expected runtime output' without having to have potentially-outdated docstrings to walk them through it. This is the similar to the common practice of adding asserts in docstrings for signalling to readers.

Test inclusion can be emulated using the integration-test harness by abusing the include! macro (to include the example wholesale) but I think having an official way to do this stuff would make it more likely that people actually use it.

cc #2631

[alexcrichton]

Gah sorry, you had the right interpretation, I just messed up that comment quite a bit! I guess what I mean is that if we're building and running tests in the examples/ directory, shouldn't those just be files in the tests/ directory? If the key aspect is not running it, then perhaps we could add a no-run key to the manifest?

[alexcrichton]

Closing due to inactivity, but feel free to resubmit with a rebase!