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

who tests the tester? #47606

Open
nikomatsakis opened this issue Jan 20, 2018 · 9 comments
Open

who tests the tester? #47606

nikomatsakis opened this issue Jan 20, 2018 · 9 comments
Labels
A-compiletest Area: the compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc C-enhancement Category: An issue proposing an enhancement or a PR with one. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

So, by and large, compiletest is untested, as far as I know. That is -- there are no 'self tests' to make sure that it's acting as it should, invoking revisions the way we expect, and so forth. This could be a bit of a tricky thing to do, but it's definitely worth the effort.

For example, an early version of #47605 had a subtle bug where it appeared to be working, but in fact was not passing the revision info through to the final test. I don't think this would have bothered travis one bit, as that would have happily run the same revision over and over.

cc @spastorino @oli-obk ... we need like a compiletest team, don't why? :)

@nikomatsakis nikomatsakis added A-testsuite Area: The testsuite used to check the correctness of rustc T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 20, 2018
@oli-obk
Copy link
Contributor

oli-obk commented Jan 20, 2018

I have some beef with compiletest in general. Instead of having this unstable (as in nightly) library that contains a somewhat random list of test kinds that all behave subtly different, we should probably write a stable extensible library that is distributed via crates.io, developed like any other library (so it has tests) and uses cargo instead of manually calling rustc and hoping for the best.

So, the list of issues off the top of my head:

  • nightly only
  • rustc submodules need to use the crates.io version, which breaks with rustc changes
  • using dependencies in tests frequently causes "multiple matching crates" because of old compilation artifacts in the target direcory
  • not extensible with new test kinds (cc @killercup for suggestion tests)

@spastorino
Copy link
Member

I'd be happy to work on a library like this if we can have a conversation about what is exactly needed :).

@killercup
Copy link
Member

@spastorino I started working on a very simple version of this to test clippy's diagnostics output as well as that its suggestions work as intended (when applied by rustfix). I'll have something up in later today, and will ping you in the clippy PR :)

In the meantime, feel free to say hello in the #clippy channel on irc.mozilla.org :)

@nikomatsakis
Copy link
Contributor Author

I have some beef with compiletest in general.

While I don't disagree, I feel a bit wary here. I'm fine with making a new testing setup, but it feels like the kind of thing that will derail progress for a long time. And I'm not convinced that the compiler's needs (e.g., things like mir-opt tests) will be satisfied by a more generic setup.

Unless you have a clear candidate of what test runner we should adopt, I'd rather see us move towards simplifying and centralizing things gradually and then at some later point perhaps switch. (Centralizing run-pass, compile-fail, and ui seems like an example of something we can do to simplify things now.)

@TimNN TimNN added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jan 23, 2018
pietroalbini added a commit to pietroalbini/rust that referenced this issue Dec 15, 2018
…r=alexcrichton

Bootstrap: Add testsuite for compiletest tool

This adds a test suite for compiletest so that the tester is tested, too.

The (currently) single unit test of the compiletest tool was never executed
on CI. At least I couldn't find any references of it in the logs.

The compiletest tests can then also be executed with:

    ./x.py test src/tools/compiletest --stage 0

cc rust-lang#47606
pietroalbini added a commit to pietroalbini/rust that referenced this issue Dec 15, 2018
…ttests, r=oli-obk

Add some unit tests to compiletest

Based on rust-lang#56792, otherwise the tests won't be executed on CI.

Just a small start, I would like to add more testing to compiletest in the future but that will require some refactoring first.

cc rust-lang#47606
@steveklabnik
Copy link
Member

Triage; Looks like Pietro has added a bit of testing, but it's "just a start"

@jyn514
Copy link
Member

jyn514 commented Nov 9, 2022

One way to test compiletest is to add more UI tests that test compiletests' behavior, instead of rustc's. See for example https://github.com/rust-lang/rust/pull/103298/files#diff-6542cbbb7ad12df6cb2daadaf279589c5c1712906cd14222c03132ad5a49d590. I think using that instead of a new framework has several advantages:

  • No need to set up a new testing framework
  • Existing contributors are already familiar with UI tests
  • These double as "integration tests" since they test exactly the property we're interested in, how compiletest runs a UI test from beginning to end

I think it should be fairly simple to keep adding tests to src/test/ui/compiletest-selftest as necessary; I don't have a set of initial tests in mind though.

@jyn514 jyn514 added E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Nov 9, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Nov 9, 2022

I wrote a new ui test framework that miri uses and I'm working on getting ready for clippy to use. It uses itself to test its own output.

@jyn514
Copy link
Member

jyn514 commented Nov 9, 2022

Sounds like there is duplicate work between that and #103266

@Alexendoo
Copy link
Member

Alexendoo commented Nov 15, 2022

It could well be, mainly I went forward with #103266 to avoid the other duplicate work of adding things to https://github.com/Manishearth/compiletest-rs that the in tree one already has. For example the JSON output after every test failure is pretty rough

@jieyouxu jieyouxu added the A-compiletest Area: the compiletest test runner label May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compiletest Area: the compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc C-enhancement Category: An issue proposing an enhancement or a PR with one. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

9 participants