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

Add UI testing to compiler test suite #33000

Closed
sophiajt opened this issue Apr 15, 2016 · 5 comments
Closed

Add UI testing to compiler test suite #33000

sophiajt opened this issue Apr 15, 2016 · 5 comments
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc

Comments

@sophiajt
Copy link
Contributor

sophiajt commented Apr 15, 2016

Currently, we don't have a way to easily test that errors have proper formatting. This means we could potentially have regressions in terms of readability that we don't immediately catch. It also means that possibly future improvements, like #32756, don't have a way to thoroughly catch where tweaks to formatting one one kind of error impact other errors in unintended ways.

A possible additional aspect of this kind of UI testing is that it allows us to not only test formatting but optionally we could also add the ability to test the highlighting to ensure that the errors are using the expected coloring.

Additionally, we currently have hacks in the unit test suite that check for column offsets (like compile-fail/symbol-names/issue-32709.rs). Having a more structured UI test allows tests to do specific formatting checks like column numbers.

@nikomatsakis
Copy link
Contributor

Here is my plan:

  1. The compiletest drive dumps the output into a file
  2. There is a foo.rs and a foo.out
  3. We diff the actual output vs foo.out
    • probably applying some filter to normalize the filename path for windows
  4. On failure, we tell you the result of the diff
  5. There is a script "update-expected-output" that regenerates the .out files

The last point is very important. This will get super annoying if every tweak to the UI requires hand-editing the .out files. But if we can just run the script and have the .out files reflect reality, great.

Colors would normally be disabled (because output of compiler is not a tty), but they could be tested explicitly via a // compileflags directive.

A nice feature of this is that if this is a PR that adjusts the rustc output, we will be able to look at the diffs in the .out fiels in the PR to get a sense of what it looks like.

One concern I have is that there may be differences across platforms. Ideally we would probably suppress those with filters, or else maybe have distinct .out files in particular cases.

cc @brson

@alexcrichton
Copy link
Member

Cargo's tests are pretty similar to this where we literally run cargo and then exhaustively match stdout/stderr. There's some tricks like using [..] to ignore chunks of the output, but other than that it's pretty simple.

@nikomatsakis
Copy link
Contributor

I left a FIXME with this issue in the code -- we had to do some hacks in the new JSON-based testing harness that I'd prefer to undo when such testing is available.

@steveklabnik steveklabnik added the A-testsuite Area: The testsuite used to check the correctness of rustc label Jun 6, 2016
@alexcrichton
Copy link
Member

I believe we've since added this, so closing.

@Eh2406
Copy link
Contributor

Eh2406 commented Sep 3, 2017

There is a FIXME related to this issue,
https://github.com/rust-lang/rust/blob/master/src/tools/compiletest/src/json.rs#L120
https://github.com/rust-lang/rust/blob/master/src/tools/compiletest/src/json.rs#L133
Now that the issue is closed can the FIXME be fix, or made more specific?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc
Projects
None yet
Development

No branches or pull requests

5 participants