search

rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.57k stars 12.74k forks source link

Tracking issue: convert library-testing ui tests to unit tests #104676

Open RalfJung opened 1 year ago

RalfJung commented 1 year ago

We generally prefer library features to be tested as unit tests, not ui tests: this clearly separates things testing the compiler from things testing the library, and also ui tests are easier and faster to run and they are formatted, checked, and linted together with the rest of the library more easily. Also our unit tests are actually run by Miri, but ui tests are not (and it's much harder to do so).

AFAIK some existing tests have already been converted, but I don't know if this was ever done systematically.

Cc @rust-lang/libs -- AFAIK this has already been policy for a while? @thomcc said there is no tracking issue though so here we go. :D

the8472 commented 1 year ago

it might not be possible for deliberately failing tests -- some of them might become compile_fail doc tests, but this does not always make sense

Some tests also modify global state or spawn child processes. Since libtest uses threads we need some helper binaries for that that the test-suite could spawn. There was a seconded compiler MCP to add a test attribute for that but I don't think it was ever implemented

cuviper commented 1 year ago

Some tests also modify global state or spawn child processes.

Isn't that what library/*/tests/ is for?

the8472 commented 1 year ago

To spawn a child you still need a helper binary that can be spawned.

thomcc commented 1 year ago

To be clear, converting these to be integration tests would be fine. The important thing is mostly that they use the libtest test runner. It's also fine if the conversion can't be done in all cases.

At the moment, we have some of these as uitests because tier3 targets (even the ones that support libstd) without support in getrandom@0.1 cannot run the stdlib test suite. We probably have some for other reasons, some of which are better. The first limitation will be lifted as of https://github.com/rust-lang/rust/pull/104658, so we should use the opportunity to go through and take a look at these, and move them where we can.