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 framework #33588

Merged
merged 5 commits into from
May 17, 2016
Merged

add UI testing framework #33588

merged 5 commits into from
May 17, 2016

Conversation

nikomatsakis
Copy link
Contributor

This adds a framework for capturing and tracking the precise output of rustc, which allows us to check all manner of minor details with the output. It's pretty strict right now -- the output must match almost exactly -- and hence maybe a bit too strict. But I figure we can add wildcards or whatever later. There is also a script intended to make updating the references easy, though the script could make things a bit easier (in particular, it'd be nice if it would find the build directory for you automatically).

One thing I was wondering about is the best way to test colors. Since windows doesn't embed those in the output stream, this test framework can't test colors on windows -- so I figure we can just write tests that are ignored on windows and which pass --color=always or whatever to rustc.

cc @jonathandturner
r? @alexcrichton

fn compare_output(&self, kind: &str, actual: &[u8], expected: &[u8]) -> usize {
if self.config.verbose {
println!("normalized {}:\n{}\n", kind, str::from_utf8(actual).unwrap_or("not utf8"));
println!("expected {}:\n{}\n", kind, str::from_utf8(expected).unwrap_or("not utf8"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be worth using String::from_utf8_lossy instead of "not uf8" on invalid utf-8

@retep998
Copy link
Member

It is possible to create a console screen buffer, redirect rustc to that, and then read the output including all the color attributes. A bit complicated, but nonetheless very much possible.

@alexcrichton
Copy link
Member

Nice! Some thoughts:

  • Could you add this to rustbuild as well? If you search for "rpass" it should just be a similar set of modifications to that.
  • Cargo has a similar method for diffing output, some other normalizations it applies are removing all carriage returns and making tabs more readable as <tab> to match in the output. Neither of those may apply too much here.
  • Cargo's method calculates differences between lines of input and output to print out out only the lines that mismatch. The only clever thing it does here is to use [..] as a placeholder for "any text"

I'd personally want to get rustbuild running these tests as well as printing the actual/expected on failure, but other than that the remaining pieces seem like they're nice-to-have and we can add later. Having a script to blanket overwrite all the previous output may also be a bit heavy-handed? In theory these shouldn't be too hard to run by hand, right? I'd just be worried that having these scripts checked in makes it "too easy" to blanket change the rustc output without thinking much.

@nikomatsakis
Copy link
Contributor Author

@alexcrichton

Cargo's method calculates differences between lines of input and output to print out out only the lines that mismatch. The only clever thing it does here is to use [..] as a placeholder for "any text"

I may try to at least get diffs of the output working.

I'd personally want to get rustbuild running these tests as well as printing the actual/expected on failure, but other than that the remaining pieces seem like they're nice-to-have and we can add later.

I agree those are necessary.

Having a script to blanket overwrite all the previous output may also be a bit heavy-handed?...I'd just be worried that having these scripts checked in makes it "too easy" to blanket change the rustc output without thinking much.

This was sort of the goal =) I figured we'd make it easy to update, then use review + git diff to check the results.

@retep998

It is possible to create a console screen buffer, redirect rustc to that, and then read the output including all the color attributes. A bit complicated, but nonetheless very much possible.

Cool. Nonetheless, after discussing with @jonathandturner and @alexcrichton, we were thinking we'd just move all color checking to operate at a more semantic level rather than using this framework (i.e., if we get more "presentation" info in the JSON as discussed here, we can check that this is as expected, and then we'll just trust that the translation to ANSI/Windows goes swimmingly).

@alexcrichton
Copy link
Member

@nikomatsakis all sounds good to me!

@nikomatsakis
Copy link
Contributor Author

@alexcrichton ok, I did the following:

  • normalize \r\n to \n (I have tested this on windows and this normalization wasn't needed, but it seems like a good idea)
  • normalize tabs to \r
  • ensured that we always dump the (normalized) actual and expected output
  • print a simple diff using cargo's code (maybe worth pulling in code from one of these projects instead, but that can happen later)
  • added to rust-build (but that...is not quite working yet, still investigating)

I chose not to:

  • keep the [..] wildcard:
    • I hope we can get away with normalization: otherwise, the compiler can't automatically produce the reference for you to copy in.
    • but it'd be simple enough to add back. maybe I should just add it proactively.

@@ -111,6 +111,8 @@ macro_rules! targets {
(check_pfail, CheckPFail { compiler: Compiler<'a> }),
(check_codegen, CheckCodegen { compiler: Compiler<'a> }),
(check_codegen_units, CheckCodegenUnits { compiler: Compiler<'a> }),
(check_incremental, CheckIncremental { compiler: Compiler<'a> }),
(check_ui, CheckUi { compiler: Compiler<'a> }),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you be sure to add these to the blanket Check target below?

@alexcrichton
Copy link
Member

Looks good to me! Just one minor nit and otherwise r=me

@nikomatsakis
Copy link
Contributor Author

@bors r=alexcrichton

@bors
Copy link
Contributor

bors commented May 13, 2016

📌 Commit 3ff521b has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented May 14, 2016

⌛ Testing commit 3ff521b with merge 9c87f71...

@bors
Copy link
Contributor

bors commented May 14, 2016

⛄ The build was interrupted to prioritize another pull request.

@bors
Copy link
Contributor

bors commented May 14, 2016

⌛ Testing commit 3ff521b with merge 9bbfe4d...

@bors
Copy link
Contributor

bors commented May 14, 2016

💔 Test failed - auto-linux-64-opt-rustbuild

@sanxiyn
Copy link
Member

sanxiyn commented May 15, 2016

This failed tidy.

bors added a commit that referenced this pull request May 16, 2016
@nikomatsakis
Copy link
Contributor Author

@bors r=alexcrichton

@bors
Copy link
Contributor

bors commented May 16, 2016

📌 Commit 474eb38 has been approved by alexcrichton

@nikomatsakis
Copy link
Contributor Author

@bors r=acrichto

@bors
Copy link
Contributor

bors commented May 16, 2016

📌 Commit 24cfa1e has been approved by acrichto

@bors
Copy link
Contributor

bors commented May 16, 2016

⌛ Testing commit 24cfa1e with merge cd6a400...

bors added a commit that referenced this pull request May 16, 2016
add UI testing framework

This adds a framework for capturing and tracking the precise output of rustc, which allows us to check all manner of minor details with the output. It's pretty strict right now -- the output must match almost exactly -- and hence maybe a bit too strict. But I figure we can add wildcards or whatever later. There is also a script intended to make updating the references easy, though the script could make things a *bit* easier (in particular, it'd be nice if it would find the build directory for you automatically).

One thing I was wondering about is the best way to test colors. Since windows doesn't embed those in the output stream, this test framework can't test colors on windows -- so I figure we can just write tests that are ignored on windows and which pass `--color=always` or whatever to rustc.

cc @jonathandturner
r? @alexcrichton
@bors bors merged commit 24cfa1e into rust-lang:master May 17, 2016
@nikomatsakis nikomatsakis deleted the compiletest-ui branch October 3, 2016 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants