Normalize workspace

[djc]

What was changed

Move the integration tests around so that cargo c --all-targets --all-features works, and integration test compilation failures become visible in Rust-Analyzer-based IDEs.

Why?

I again got frustrated after finding compilation failures via cargo lint and cargo test-lint in an upcoming PR. Move the integration test code for the core crate into more conventional places so that Cargo / Rust-Analyzer can find all the code, while minimizing interface changes to running it.

Checklist

1. Closes: no issue
2. How was this tested: ran cargo lint and cargo test-lint and exercised the integ_runner
3. Any docs updates needed: no

[djc]

Would the feature flag be harder to forget when adding a new one? Could we make the failure mode more obvious when you forget to add it -- for example, by using --ignored instead of --include-ignored in the integ_runner? Could we make the slightly confusing output better by employing better module/file naming? IMO this use case is really what #[ignore] was intended for -- it seems like a pretty good fit to me.

(I'm pretty sure we can make it work a feature and I have configured VS Code to check with --all-features so that would work for me, but IMO it would be less friendly to new contributors.)

[Sushisource]

Sorry for the late replay - had some other urgent things and was on vacation for a while.

So I think the feature flag forgetting problem can probably be handled by just having the flag go around whole modules at once? Then you don't need to think about it most of the time, only when adding whole new modules. So seems like that'd work fairly nicely to me

using --ignored instead of --include-ignored in the integ_runner

AFAIK the only place I've used --include-ignored is when running normal tests to run the one weird state machine transition coverage test.

[dandavison]

FWIW I can confirm this improves the experience of using VSCode for tests/integ_tests -- e.g. rust-analyzer working and "Run/debug tests" code lenses active.

[Sushisource]

@Sushisource what do you think about just letting cargo test run all tests, add adding a command to run unit tests only?

Yeah, we can do that (though it's not immediately obvious to me how it'll work... but there's probably a way). Bonus points if the command has an option for running with https://nexte.st/ (would be nice to add that to the integ test runner too), but not a requirement.

[dandavison]

@djc This PR is a really valuable improvement. (I also work on sdk-core using rust-analyzer.) Sorry we've let it pick up merge conflicts. Do you have any appetite for getting these changes mergable again?

[djc]

@dandavison happy to rebase this if there's a path to getting it merged. But,

add adding a command to run unit tests only?

I'm not sure how you're envisioning this should work.

[dandavison]

happy to rebase this if there's a path to getting it merged

Great!

OK so to recap our requirements, I'm thinking they are:

• Be able to run unit tests, and integration tests, separately
• Not have to remember to add annotations
• Be rust-analyzer compatible
• Not do anything that makes nextest adoption harder

So what I'm thinking is

• move code as you did to be rust-analyzer compatible
• Have a command cargo unit-test that runs all tests except integration tests, and cargo integ-test that runs only integration tests (perhaps via filepath-based logic?).
• cargo test would run all tests.