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

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

Open
RalfJung opened this issue Nov 21, 2022 · 4 comments
Open

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

RalfJung opened this issue Nov 21, 2022 · 4 comments
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@RalfJung
Copy link
Member

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.

  • Identify which parts of the ui test suite are really just testing the library
  • Convert them to regular unit tests if possible
    • 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
  • Properly document this policy and find some way to reduce the chance of new such tests being added

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

@RalfJung RalfJung added C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Nov 21, 2022
@the8472
Copy link
Member

the8472 commented Nov 21, 2022

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
Copy link
Member

cuviper commented Nov 21, 2022

Some tests also modify global state or spawn child processes.

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

@the8472
Copy link
Member

the8472 commented Nov 21, 2022

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

@thomcc
Copy link
Member

thomcc commented Nov 21, 2022

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 #104658, so we should use the opportunity to go through and take a look at these, and move them where we can.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants