Stream: t-compiler

Topic: test suite


pnkfelix (Aug 01 2018 at 12:26, on Zulip):

I would really like to invest some time moving the rest of compile-fail over to ui

pnkfelix (Aug 02 2018 at 12:10, on Zulip):

I guess we would need to (or at least should) resolve #46841 in parallel with that effort.

pnkfelix (Aug 02 2018 at 12:11, on Zulip):

and that in turn is blocked on coming up with guidelines for directory names. Hmm.

varkor (Aug 02 2018 at 12:13, on Zulip):

I imagine coming up with some guidelines is just a matter of someone writing down some sensible rules.

varkor (Aug 02 2018 at 12:14, on Zulip):

Which, like actually converting the tests, is just a slog through the tests, and seeing what patterns there are in kinds of tests.

varkor (Aug 02 2018 at 12:14, on Zulip):

Though it would be nice if the guidelines could address all test suites and tackle them all at once.

varkor (Aug 02 2018 at 12:15, on Zulip):

Converting compile-test might not actually be too bad, with --bless in hand...

RalfJ (Aug 02 2018 at 15:07, on Zulip):

what is going to happen with tests whose UI output is platform-dependent?

RalfJ (Aug 02 2018 at 15:08, on Zulip):

e.g., they might contain the size of usize or so

RalfJ (Aug 02 2018 at 15:08, on Zulip):

(some miri tests do. others might exist as well.)

oli (Aug 02 2018 at 15:08, on Zulip):

I kept making them compile-fail tests

pnkfelix (Aug 02 2018 at 15:09, on Zulip):

yeah, tests with platform dependent UI output should not, in my opinion, be migrated to ui/

pnkfelix (Aug 02 2018 at 15:09, on Zulip):

but that's the exception, not the rule.

oli (Aug 02 2018 at 15:09, on Zulip):

maybe we could mark tests as platform dependent and generate multiple .stderr files?

oli (Aug 02 2018 at 15:09, on Zulip):

but that would be hard to maintain

pnkfelix (Aug 02 2018 at 15:09, on Zulip):

(I probably made statements that were too strong about my goals here. I'd be happy if 90% of the compile-tests/ moved to ui/)

Vadim Petrochenkov (Aug 02 2018 at 21:15, on Zulip):

UI tests support output postprocessing with regexes (search for "normalize-stderr" in src/test/ui), so platform-dependent output is not a problem.

RalfJ (Aug 03 2018 at 17:42, on Zulip):

@Vadim Petrochenkov I had no idea! Thats pretty cool.

nikomatsakis (Aug 06 2018 at 12:31, on Zulip):

I agree I would like to complete this transition

nikomatsakis (Aug 07 2018 at 22:06, on Zulip):

I was saying over in #wg-nll that it is extra important to do this, I think, because otherwise we are just not comparing the compile-fail tests for consistency with NLL.

nikomatsakis (Aug 07 2018 at 22:10, on Zulip):

I just adopted https://github.com/rust-lang/rust/issues/44844 as an NLL bug, towards that end

nikomatsakis (Aug 07 2018 at 22:11, on Zulip):

that reminds me, do we test that run-pass works with NLL?

nikomatsakis (Aug 07 2018 at 22:11, on Zulip):

should open an issue on that too...

nikomatsakis (Aug 07 2018 at 22:11, on Zulip):

sadly, gotta go now

davidtwco (Aug 08 2018 at 14:53, on Zulip):

Submitted #53196 for this after discussion with @nikomatsakis in a WG-NLL topic.

RalfJ (Aug 08 2018 at 15:29, on Zulip):

woah that PR is crazy big^^

simulacrum (Aug 08 2018 at 15:32, on Zulip):

And it already has merge conflicts. :)

varkor (Aug 08 2018 at 15:32, on Zulip):

Woah!

nagisa (Aug 08 2018 at 15:32, on Zulip):

+60k…

varkor (Aug 08 2018 at 15:32, on Zulip):

You can't even view the entire diff in GH.

varkor (Aug 08 2018 at 15:33, on Zulip):

Who knows what @davidtwco is trying to sneak in there in the last few thousand files ;)

nagisa (Aug 08 2018 at 15:33, on Zulip):

oh, you can’t view almost any diff in GH nowadays. They must be saving their compute or what

davidtwco (Aug 08 2018 at 15:33, on Zulip):

Annoyingly I messed up when doing one of the commits and included some files I intended to leave to the next commit which is making resolving the merge conflict a tad annoying. Needing to mess with the history a little to make it easier to rebase.

nagisa (Aug 08 2018 at 15:33, on Zulip):

there’s about one sort of PR that GH will still show a diff for properly. Its a +1-1 change that only affects exactly 1 file.

varkor (Aug 08 2018 at 15:34, on Zulip):

"a tad annoying" ⇒ 30k conflicts

nagisa (Aug 08 2018 at 15:34, on Zulip):

and that’s why github is terrible as a reviewing tool

davidtwco (Aug 08 2018 at 15:35, on Zulip):

I've squashed the last two commits and then I'm going to insert a new third commit that does what the original third commit was supposed to do and resolve the minor conflicts that will have with the squashed fourth commit. That should make rebasing a bit easier.

varkor (Aug 08 2018 at 15:35, on Zulip):

I'm surprised it's only -2k

davidtwco (Aug 08 2018 at 15:36, on Zulip):

I'm mostly saying this for my own benefit to convince myself it'll work.

varkor (Aug 08 2018 at 15:36, on Zulip):

Surely the ui test output can't be 58k lines...?

davidtwco (Aug 08 2018 at 15:36, on Zulip):

There's some duplication in auxiliary modules for tests too. Not much, but some.

davidtwco (Aug 08 2018 at 15:37, on Zulip):

Since I tried to group things together into folders so that the number of files in the root ui folder was < 1000 and could be viewed in GitHub's file browser as a result.

RalfJ (Aug 08 2018 at 15:42, on Zulip):

It might make sense to separate the function changles for checking error-pattern into a separate PR

RalfJ (Aug 08 2018 at 15:42, on Zulip):

so that the big one really just moves files around

davidtwco (Aug 08 2018 at 15:43, on Zulip):

I would have had I known I'd needed to have made that change when trying to do this initially.

RalfJ (Aug 08 2018 at 15:43, on Zulip):

^^

nikomatsakis (Aug 08 2018 at 15:45, on Zulip):

@davidtwco the changes for error-pattern seem right

nikomatsakis (Aug 08 2018 at 15:46, on Zulip):

/me grumbles about compieltest having no unit tests

nikomatsakis (Aug 08 2018 at 15:46, on Zulip):

what could go wrong?

davidtwco (Aug 08 2018 at 15:46, on Zulip):

Tests that were failing with it the way it was no longer fail, and tests that passed with it the way it was, still pass.

nikomatsakis (Aug 08 2018 at 15:48, on Zulip):

Did you manually test what happens if you give a bogus error pattern?

davidtwco (Aug 08 2018 at 15:49, on Zulip):

I think I did.

davidtwco (Aug 08 2018 at 15:50, on Zulip):

I'll double check because I can't quite remember.

davidtwco (Aug 08 2018 at 16:25, on Zulip):

It should be rebased. There may be one or two failures that I've not noticed.

davidtwco (Aug 08 2018 at 16:25, on Zulip):

Testing locally and will fix any I find.

nikomatsakis (Aug 08 2018 at 16:43, on Zulip):

@davidtwco ok

davidtwco (Aug 08 2018 at 16:43, on Zulip):

Looks like there is a failure in tidy.

nikomatsakis (Aug 08 2018 at 16:45, on Zulip):

ah the mercilous tidy

davidtwco (Aug 08 2018 at 17:01, on Zulip):

@Esteban Küber's comment on the issue makes me realize that I'll need to run on another platform to identify the tests with platform specific output.

Need to fix the issue with tidy first.

nikomatsakis (Aug 08 2018 at 17:01, on Zulip):

d'oh

nikomatsakis (Aug 08 2018 at 17:01, on Zulip):

you might try biting off fewer tests

nikomatsakis (Aug 08 2018 at 17:01, on Zulip):

(also)

nikomatsakis (Aug 08 2018 at 17:01, on Zulip):

i.e., it doens't have to be an "all at once" PR

davidtwco (Aug 08 2018 at 17:03, on Zulip):

Yeah, but at this point I've made the big PR and spent time trying to do some reorganization to cut down the number of files in the root directory, so I'm inclined to just try get this PR good to land as soon as I can.

nikomatsakis (Aug 08 2018 at 17:04, on Zulip):

it occurs to me that we ought to move run-pass too, probably?

nikomatsakis (Aug 08 2018 at 17:04, on Zulip):

/me tries not to scare @davidtwco away

simulacrum (Aug 08 2018 at 17:31, on Zulip):

Hm, with this we can probably also get rid of // error-pattern perhaps?

nikomatsakis (Aug 08 2018 at 17:38, on Zulip):

I think there are some cases where we still need it, not sure

nikomatsakis (Aug 08 2018 at 17:38, on Zulip):

though .. maybe not?

nikomatsakis (Aug 08 2018 at 17:39, on Zulip):

well, it is kind of an asserion over "stuff that should exist in the reference output"

nikomatsakis (Aug 08 2018 at 17:39, on Zulip):

or it could be thought of that way

nikomatsakis (Aug 08 2018 at 17:39, on Zulip):

this is certainly what the //~ ERROR annotations effectively server as

nikomatsakis (Aug 08 2018 at 17:39, on Zulip):

and I quite like the setup, I have to say

nikomatsakis (Aug 08 2018 at 17:39, on Zulip):

especially now that we have --bless

simulacrum (Aug 08 2018 at 17:40, on Zulip):

Yeah, it is essentially just an error annotation I guess

nikomatsakis (Aug 08 2018 at 18:37, on Zulip):

@davidtwco are you still working on https://github.com/rust-lang/rust/pull/53196 ? did you see the tidy failure?

davidtwco (Aug 08 2018 at 18:42, on Zulip):

@nikomatsakis I've seen it. Not had a chance to get to it yet. Will take another look in an hour or so.

davidtwco (Aug 08 2018 at 18:43, on Zulip):

Is there an easy way for me to test the platform specific output myself?

simulacrum (Aug 08 2018 at 18:44, on Zulip):

Not really -- it's Windows vs. Linux, I'd imagine (or mostly so)

simulacrum (Aug 08 2018 at 18:44, on Zulip):

You can grep for cfg.*windows in the directory, which might give some hints...

simulacrum (Aug 08 2018 at 18:45, on Zulip):

I'd just throw it at bors though

davidtwco (Aug 08 2018 at 19:38, on Zulip):

Fixed tidy issue.

davidtwco (Aug 08 2018 at 20:36, on Zulip):

So, Travis has a handful of interesting failures that I don't see locally.

nikomatsakis (Aug 08 2018 at 21:51, on Zulip):

@davidtwco yeah

nikomatsakis (Aug 08 2018 at 21:51, on Zulip):

I just went to check and saw that

nikomatsakis (Aug 08 2018 at 21:51, on Zulip):

any thoughts on those?

nikomatsakis (Aug 08 2018 at 21:51, on Zulip):

why..is the output so weird?

nikomatsakis (Aug 08 2018 at 21:51, on Zulip):

I guess it's the .. line numbers where the errors occur?

davidtwco (Aug 08 2018 at 21:53, on Zulip):

I'm not sure. Some of them seemed to be fixable by using $DIR in places where --bless didn't add it.

davidtwco (Aug 08 2018 at 21:54, on Zulip):

Some of them, such as the order of suggested trait implementations are stranger.

nikomatsakis (Aug 08 2018 at 21:54, on Zulip):

that may not be deterministic

nikomatsakis (Aug 08 2018 at 21:54, on Zulip):

which is actually something that is useful to know

nikomatsakis (Aug 08 2018 at 21:54, on Zulip):

there is probably a non-fx-hashset or something involved

davidtwco (Aug 08 2018 at 21:54, on Zulip):

Yeah - is there something I can do in the test or is that something we need to fix to use ui tests for it?

nikomatsakis (Aug 08 2018 at 21:54, on Zulip):

which test is it?

nikomatsakis (Aug 08 2018 at 21:55, on Zulip):

the output seems all jumbled together :/

nikomatsakis (Aug 08 2018 at 21:55, on Zulip):

I think we can fix the compiler to be deterministic, hopefully

davidtwco (Aug 08 2018 at 21:55, on Zulip):

Can't remember, on mobile, left my machine building fresh to see if that helps reproduce some of them.

nikomatsakis (Aug 08 2018 at 21:55, on Zulip):

I don't quite understand why that bors output looks so weird

nikomatsakis (Aug 08 2018 at 21:55, on Zulip):

I feel like when I run locally.. it's much easier to undersatnd...?

nikomatsakis (Aug 08 2018 at 21:56, on Zulip):

or am I forgetting how bad it can be :)

davidtwco (Aug 08 2018 at 21:56, on Zulip):

There's another test where it isn't complaining about creating a module where there's an auxiliary of the same name. That test hasn't really changed as far as I can tell.

davidtwco (Aug 08 2018 at 21:56, on Zulip):

There's some that don't show diffs.

davidtwco (Aug 08 2018 at 21:56, on Zulip):

And there's some that have diffs that seem to only show ordering changes.

nikomatsakis (Aug 08 2018 at 21:56, on Zulip):

thought:

nikomatsakis (Aug 08 2018 at 21:56, on Zulip):

maybe move just these troublesome tests back to compile-fail for now?

nikomatsakis (Aug 08 2018 at 21:56, on Zulip):

and we can do them in a follow-up PR?

nikomatsakis (Aug 08 2018 at 21:56, on Zulip):

if it's just a handful of tests...

davidtwco (Aug 08 2018 at 21:57, on Zulip):

Yeah. That might be an option. Depends how easy they are to resolve. Will need to see if I can reproduce these and fix them locally without much trouble. There's also the platform specific ones that we've not seen yet.

davidtwco (Aug 08 2018 at 22:00, on Zulip):

Is there a flag to x.py that disables tests being ignored so I run them all each time? That might be helpful for the less deterministic stuff.

davidtwco (Aug 08 2018 at 22:00, on Zulip):

I should check config.toml actually.

nikomatsakis (Aug 08 2018 at 22:02, on Zulip):

Is there a flag to x.py that disables tests being ignored so I run them all each time? That might be helpful for the less deterministic stuff.

I don't know...

simulacrum (Aug 09 2018 at 00:12, on Zulip):

"yes" -- rm -r build/<build triple>/test

simulacrum (Aug 09 2018 at 00:12, on Zulip):

@davidtwco ^

But nothing better than that today I think

davidtwco (Aug 09 2018 at 09:08, on Zulip):

So, interesting issue, I've got a test case where the compiler truncates some type that it is printing. In the case where that's a closure, it contains a path. But because it truncates it, that path isn't being normalized - it can't match the whole thing it is expecting.

davidtwco (Aug 09 2018 at 09:08, on Zulip):

But, on Travis, it can, because the path is shorter - on my machine, it cannot, the path to the rust working directory is too long.

davidtwco (Aug 09 2018 at 09:10, on Zulip):

In particular, the error from this line.

davidtwco (Aug 09 2018 at 09:22, on Zulip):

Only in compiletest though, because the path output by rustc is only absolute if the path to the source it was given was absolute - which it is for compiletest.

nikomatsakis (Aug 09 2018 at 10:10, on Zulip):

argh

nikomatsakis (Aug 09 2018 at 10:10, on Zulip):

can you use the "custom replacement" feature?

nikomatsakis (Aug 09 2018 at 10:10, on Zulip):

I forget how that works, in particular I don't remember if it supports a regular expression

nikomatsakis (Aug 09 2018 at 10:10, on Zulip):

one could imagine trying to normalize all closure types out in this case

davidtwco (Aug 09 2018 at 10:50, on Zulip):

Yeah, that worked for fixing that case.

davidtwco (Aug 09 2018 at 10:50, on Zulip):

Only got a few left to resolve.

davidtwco (Aug 09 2018 at 14:21, on Zulip):

Rebased again and fixed a majority of the test failures locally. There should be one or two that I've not yet figured out still to solve (and then all the platform-dependent ones).

nikomatsakis (Aug 09 2018 at 14:22, on Zulip):

:medal:

nikomatsakis (Aug 09 2018 at 14:22, on Zulip):

you deserve one of those :)

davidtwco (Aug 09 2018 at 15:02, on Zulip):

There are some commits where I've made changes either to the compiler or to compiletest in order to get things working. Nothing major or anything like that. The commits that have those changes should be relatively easy to tell apart from the commits that change thousands of files.

davidtwco (Aug 09 2018 at 15:16, on Zulip):

So, of the test failures from this Travis run - there are five. The last one is another path that needs normalized - I've fixed that locally. This leaves the following:

[00:46:35]     [ui] ui/coercion/coerce-unsafe-to-closure.rs
[00:46:35]     [ui] ui/invalid-module-declaration/invalid-module-declaration.rs
[00:46:35]     [ui] ui/issues/issue-46209-private-enum-variant-reexport.rs
[00:46:35]     [ui] ui/meta-expected-error-wrong-rev.rs#a

I can reproduce the second and fourth. I cannot reproduce the first and third. I have no idea why any of them are happening. The fourth one doesn't show a diff or anything, it just finds itself on that list.

davidtwco (Aug 09 2018 at 15:18, on Zulip):

The first one is:

[00:46:35] -       |                                        ^^^ the trait `std::ops::FnOnce<(&str,)>` is not implemented for `unsafe extern "rust-intrinsic" fn(_) -> _ {std::mem::transmute::<_, _>}`
[00:46:35] +       |                                        ^^^ the trait `std::ops::FnOnce<(&str,)>` is not implemented for `unsafe extern "rust-intrinsic" fn(_) -> _ {std::intrinsics::transmute::<_, _>}`

I don't know why I get the result I get locally or why it is different on Travis.

davidtwco (Aug 09 2018 at 15:19, on Zulip):

The second one is:

[00:46:35] ---- [ui] ui/invalid-module-declaration/invalid-module-declaration.rs stdout ----
[00:46:35]
[00:46:35] error: error pattern ' cannot declare a new module at this location' not found!
[00:46:35]
[00:46:35] error: error pattern ' maybe move this module' not found!
[00:46:35]
[00:46:35] error: multiple error patterns not found
[00:46:35] status: exit code: 1
[00:46:35] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/invalid-module-declaration/invalid-module-declaration.rs" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/invalid-module-declaration/invalid-module-declaration/a" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/invalid-module-declaration/invalid-module-declaration/auxiliary" "-A" "unused"

This one is because the auxiliary module should stop the module declared in the test from being declared. Nothing has changed about this test whatsoever (not even moved it) or about auxiliary handling in compiletest.

davidtwco (Aug 09 2018 at 15:19, on Zulip):

The third one is:

[00:46:35] 16   LL |     crate enum Crewman {
[00:46:35] 17      |     ------------------ help: consider making the enum public: `pub enum Crewman`
[00:46:35] 18
[00:46:35] -    error: enum is private and its variants cannot be re-exported
[00:46:35] -      --> $DIR/issue-46209-private-enum-variant-reexport.rs:14:13
[00:46:35] -       |
[00:46:35] -    LL |     pub use self::Professor::*;
[00:46:35] -       |             ^^^^^^^^^^^^^^^^^^
[00:46:35] -    ...
[00:46:35] -    LL |     enum Professor {
[00:46:35] -       |     -------------- help: consider making the enum public: `pub enum Professor`
[00:46:35] -
[00:46:35] 28   error: variant `JuniorGrade` is private and cannot be re-exported
[00:46:35] 29     --> $DIR/issue-46209-private-enum-variant-reexport.rs:16:32
[00:46:35] 30      |
[00:46:35]
[00:46:35] 33   ...
[00:46:35] 34   LL |     enum Lieutenant {
[00:46:35] 35      |     --------------- help: consider making the enum public: `pub enum Lieutenant`
[00:46:35] +
[00:46:35] +    error: enum is private and its variants cannot be re-exported
[00:46:35] +      --> $DIR/issue-46209-private-enum-variant-reexport.rs:14:13
[00:46:35] +       |
[00:46:35] +    LL |     pub use self::Professor::*;
[00:46:35] +       |             ^^^^^^^^^^^^^^^^^^
[00:46:35] +    ...
[00:46:35] +    LL |     enum Professor {
[00:46:35] +       |     -------------- help: consider making the enum public: `pub enum Professor`
[00:46:35] 36
[00:46:35] 37   error: variant `Full` is private and cannot be re-exported
[00:46:35] 38     --> $DIR/issue-46209-private-enum-variant-reexport.rs:16:45

For some reason, these errors are reported in different order on Travis, I've built the compiler with three different versions now with all the rebases, I can't get it to make that order locally.

davidtwco (Aug 09 2018 at 15:20, on Zulip):

The fourth one doesn't say anything at all.

davidtwco (Aug 09 2018 at 15:20, on Zulip):

Not a thing.

nikomatsakis (Aug 09 2018 at 15:22, on Zulip):

interesting

nikomatsakis (Aug 09 2018 at 15:22, on Zulip):

I have some ideas about some of those

nikomatsakis (Aug 09 2018 at 15:22, on Zulip):

but I definitely think we should leave those in compile-fail and file follow-up issues

nikomatsakis (Aug 09 2018 at 15:22, on Zulip):

I am eager to let you stop rebasing this thing

davidtwco (Aug 09 2018 at 15:22, on Zulip):

I've only had to rebase twice and both times (after fixing my initial messy commits) I've not had to really make any changes.

nikomatsakis (Aug 09 2018 at 15:23, on Zulip):

The first one is:

That looks like maybe the code that generates a path from a DefId is kind of non-deterministic-ish?

nikomatsakis (Aug 09 2018 at 15:23, on Zulip):

that is, there are probably multiple valid paths due to re-exports or whatever

nikomatsakis (Aug 09 2018 at 15:24, on Zulip):

For some reason, these errors are reported in different order on Travis,

would have to look at the privacy related code, I guess...

davidtwco (Aug 09 2018 at 15:24, on Zulip):

I could normalize that [the first one] and fix it that way - it's a ugly solution but we could add an issue so it gets resolved at some point.

nikomatsakis (Aug 09 2018 at 15:24, on Zulip):

as an aside, I am reminded that in previous compilers I worked on, we had support for multiple valid outputs, perhaps for this reason :)

nikomatsakis (Aug 09 2018 at 15:24, on Zulip):

but yeah these don't feel like 'normalizations'

nikomatsakis (Aug 09 2018 at 15:24, on Zulip):

though I guess adding with a FIXME might be ok

simulacrum (Aug 09 2018 at 15:24, on Zulip):

I think it makes sense to pull these out for now

davidtwco (Aug 09 2018 at 15:24, on Zulip):

They aren't, but they'd get it working.

nikomatsakis (Aug 09 2018 at 15:25, on Zulip):

I think it makes sense to pull these out for now

by this you mean leave as compile-fail tests?

davidtwco (Aug 09 2018 at 15:25, on Zulip):

Strangest thing is, the invalid-module-declaration error - I had that working in the very first push, after the first rebase it didn't.

davidtwco (Aug 09 2018 at 15:26, on Zulip):

The first rebase being immediately after the first push.

davidtwco (Aug 09 2018 at 15:26, on Zulip):

The last one is a "meta-test" of compiletest.

davidtwco (Aug 09 2018 at 15:27, on Zulip):

I wonder if the issue for that is that // should-fail isn't respected on UI tests?

simulacrum (Aug 09 2018 at 15:27, on Zulip):

Yeah, leaving them as compile-fail

nikomatsakis (Aug 09 2018 at 15:34, on Zulip):

I wonder if the issue for that is that // should-fail isn't respected on UI tests?

could be

davidtwco (Aug 09 2018 at 15:43, on Zulip):

I'm struggling to see how should-fail is implemented in any of the testing modes.

nikomatsakis (Aug 09 2018 at 15:47, on Zulip):

@davidtwco I believe it is used earlier on

nikomatsakis (Aug 09 2018 at 15:47, on Zulip):

I'd actually be a bit surprised if it is not respected by ui

nikomatsakis (Aug 09 2018 at 15:48, on Zulip):

let me see...

davidtwco (Aug 09 2018 at 15:48, on Zulip):

I've grepped around and can only find it used where it is parsed. That sets another field but I can't find it used at all.

nikomatsakis (Aug 09 2018 at 15:48, on Zulip):

yeah, here: link

nikomatsakis (Aug 09 2018 at 15:48, on Zulip):

basically we use the same runtime as is used for #[test]

nikomatsakis (Aug 09 2018 at 15:48, on Zulip):

and // should_fail translates into setting the should_panic field to true

davidtwco (Aug 09 2018 at 15:49, on Zulip):

I can't find should_panic being used though.

davidtwco (Aug 09 2018 at 15:49, on Zulip):

Other than there.

nikomatsakis (Aug 09 2018 at 15:49, on Zulip):

well that's because it's in the libtest runtime

davidtwco (Aug 09 2018 at 15:49, on Zulip):

Ah.

davidtwco (Aug 09 2018 at 15:49, on Zulip):

That makes more sense.

davidtwco (Aug 09 2018 at 15:51, on Zulip):

Does that mean that the bug is that it is returning a status of 256 despite the fact I can see the test does error?

davidtwco (Aug 09 2018 at 15:54, on Zulip):

Either way, I'm moving those tests to compile-fail now.

davidtwco (Aug 09 2018 at 16:00, on Zulip):

Moving back to compile-fail, all but invalid-module-declaration work.

davidtwco (Aug 09 2018 at 16:00, on Zulip):

I don't know what I've done to this test.

nikomatsakis (Aug 09 2018 at 16:05, on Zulip):

Does that mean that the bug is that it is returning a status of 256 despite the fact I can see the test does error?

maybe...

davidtwco (Aug 09 2018 at 16:05, on Zulip):

Alright, so, I know what happened, I think. This test never worked. It was always a ui test. But previously, error-pattern didn't work on ui test - I fixed that when working on this because we noticed it on some tests.

nikomatsakis (Aug 09 2018 at 16:05, on Zulip):

ah, interesting :)

davidtwco (Aug 09 2018 at 16:05, on Zulip):

It was always making an error - and that error matched the UI output.

davidtwco (Aug 09 2018 at 16:05, on Zulip):

It always passed. But it didn't assert that the two error patterns existed, and now it does, and well, those patterns don't match.

davidtwco (Aug 09 2018 at 16:06, on Zulip):

What should I do about this test?

nikomatsakis (Aug 09 2018 at 16:12, on Zulip):

what patterns is it assering?

nikomatsakis (Aug 09 2018 at 16:12, on Zulip):

and what is the actual output?

davidtwco (Aug 09 2018 at 16:12, on Zulip):

I've pushed with the patterns removed to see if anything else fails on Travis.

davidtwco (Aug 09 2018 at 16:12, on Zulip):

The error output is that it expects a file to be there for a module declared in the auxiliary code.

davidtwco (Aug 09 2018 at 16:12, on Zulip):

That should be there.

davidtwco (Aug 09 2018 at 16:13, on Zulip):

But there is also a duplicate module declared in a test. The patterns expect errors about that duplicate module name.

davidtwco (Aug 09 2018 at 16:13, on Zulip):

There aren't any.

davidtwco (Aug 09 2018 at 16:13, on Zulip):

You can see the test here - there's not much in it.

nikomatsakis (Aug 09 2018 at 16:14, on Zulip):

seems like the test is failing but never worked perhaps...

davidtwco (Aug 09 2018 at 16:14, on Zulip):

Yeah, I don't think it ever worked.

davidtwco (Aug 09 2018 at 16:15, on Zulip):

Given that error patterns weren't enforced before on UI tests - it literally was never testing that.

davidtwco (Aug 09 2018 at 16:16, on Zulip):

It's possible that nobody ran the test itself with the flags to make it use the auxiliary module to see if the expected error was there either.

davidtwco (Aug 09 2018 at 16:16, on Zulip):

(though I guess the ui output should have shown that)

davidtwco (Aug 09 2018 at 16:16, on Zulip):

(which I guess confirms it never did output the correct thing as we can view that)

davidtwco (Aug 09 2018 at 16:17, on Zulip):

Well, looking at the history, at some point in the past it did.

davidtwco (Aug 09 2018 at 16:17, on Zulip):

But as of the last commit to that stderr, it did not.

davidtwco (Aug 09 2018 at 16:20, on Zulip):

I'd land this with the test passing again (that is, without the error patterns) and perhaps make an issue for someone to look into the regression in that test's output.

nikomatsakis (Aug 09 2018 at 16:26, on Zulip):

I'd land this with the test passing again (that is, without the error patterns) and perhaps make an issue for someone to look into the regression in that test's output.

yes sounds good

nikomatsakis (Aug 09 2018 at 16:26, on Zulip):

cc @cramertj on that

davidtwco (Aug 09 2018 at 16:27, on Zulip):

If this Travis run passes (and I think :fingers_crossed: it will) then we just need to check platform specific output and that should be it.

davidtwco (Aug 09 2018 at 17:08, on Zulip):

Noticed the failure, one last test, easy fix - building locally now.

davidtwco (Aug 09 2018 at 17:15, on Zulip):

Alright, pushed that. Fifth time lucky.

davidtwco (Aug 09 2018 at 18:09, on Zulip):

:tada: UI tests passed on Travis. Is someone able to kick off a big test on other platforms.

kennytm (Aug 09 2018 at 18:11, on Zulip):

you could edit .travis.yml to temporarily enable any platforms on CI (don't enable everything though, this will block the bors's queue)

nikomatsakis (Aug 09 2018 at 18:11, on Zulip):

sounds reasonable -- I can also r+ :)

nikomatsakis (Aug 09 2018 at 18:11, on Zulip):

but it'd be better not to block the queue

kennytm (Aug 09 2018 at 18:12, on Zulip):

r+ would be easiest as this would be part of the queue :big_smile:

nikomatsakis (Aug 09 2018 at 18:13, on Zulip):

I'm not sure I understand but I'm happy to r+

davidtwco (Aug 09 2018 at 19:28, on Zulip):

@nikomatsakis I think bors ignored you again.

davidtwco (Aug 09 2018 at 19:28, on Zulip):

It didn't reply with a commit being approved message and its not where I'd expect on the queue page.

varkor (Aug 09 2018 at 20:14, on Zulip):

it seemed to be blocked by a "WIP" in the title (was there one before)?

varkor (Aug 09 2018 at 20:15, on Zulip):

and in @nikomatsakis's second message, he didn't r+ again

nikomatsakis (Aug 09 2018 at 20:15, on Zulip):

@davidtwco yeah I forgot to r+ again after removing wip

varkor (Aug 09 2018 at 21:46, on Zulip):

@davidtwco: it's already outdated

varkor (Aug 09 2018 at 21:47, on Zulip):

I'm surprise bors is running it if there's a merge conflict, though

simulacrum (Aug 09 2018 at 21:50, on Zulip):

Bors might not realize or GH might not understand. The first is more likely, though.

simulacrum (Aug 09 2018 at 21:50, on Zulip):

Shouldn't get merged I think...

davidtwco (Aug 09 2018 at 21:58, on Zulip):

I expect it to fail anyway. There's got to be some platform dependant tests in there that we'll catch.

varkor (Aug 09 2018 at 22:06, on Zulip):

https://ci.appveyor.com/project/rust-lang/rust/build/1.0.8517/job/h632yv766sg7w9hn

davidtwco (Aug 09 2018 at 22:07, on Zulip):

Yeah, just noticed that. Some strange differences in there.

davidtwco (Aug 09 2018 at 22:09, on Zulip):

I'll need to set up a build on a Windows machine, I think.

davidtwco (Aug 09 2018 at 22:11, on Zulip):

I assume the url diffs that it shows are related to some sort of merge it tried to do with HEAD.

varkor (Aug 09 2018 at 22:14, on Zulip):

"second-edition/ch19-04-advanced-types.html#dynamically-sized-types-and-sized"
vs
"second-edition/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait"
is pretty strange

varkor (Aug 09 2018 at 22:14, on Zulip):

(the first link is invalid)

davidtwco (Aug 09 2018 at 22:15, on Zulip):

#53082 seems to have changed that.

davidtwco (Aug 09 2018 at 22:16, on Zulip):

It did a merge with the rollup that happened before, that'll account for a bunch of the diffs.

davidtwco (Aug 09 2018 at 22:25, on Zulip):

So, the failures are:

URL ERROR:
    [ui] ui\associated-types\associated-types-unsized.rs
    [ui] ui\bad\bad-sized.rs
    [ui] ui\dst\dst-bad-assign-2.rs
    [ui] ui\dst\dst-bad-assign-3.rs
    [ui] ui\dst\dst-bad-assign.rs
    [ui] ui\dst\dst-bad-deep-2.rs
    [ui] ui\dst\dst-bad-deep.rs
    [ui] ui\dst\dst-object-from-unsized-type.rs
    [ui] ui\dst\dst-sized-trait-param.rs
    [ui] ui\extern\extern-types-unsized.rs
    [ui] ui\range\range-1.rs
    [ui] ui\str\str-mut-idx.rs
    [ui] ui\substs-ppaux.rs#normal
    [ui] ui\substs-ppaux.rs#verbose
    [ui] ui\traits\trait-bounds-not-on-bare-trait.rs
    [ui] ui\union\union-unsized.rs
    [ui] ui\unsized3.rs
    [ui] ui\unsized5.rs
    [ui] ui\unsized6.rs
    [ui] ui\unsized7.rs
    [ui] ui\unsized\unsized-bare-typaram.rs
    [ui] ui\unsized\unsized-enum.rs
    [ui] ui\unsized\unsized-inherent-impl-self-type.rs
    [ui] ui\unsized\unsized-struct.rs
    [ui] ui\unsized\unsized-trait-impl-self-type.rs
    [ui] ui\unsized\unsized-trait-impl-trait-arg.rs
    [ui] ui\wf\wf-array-elem-sized.rs

PATHS:
    [ui] ui\crateresolve1.rs
    [ui] ui\nolink-with-link-args.rs
    [ui] ui\not-utf8.rs

FILE NOT FOUND DIFFERENCE:
    [ui] ui\issues\issue-10755.rs
    [ui] ui\extern\external-doc-error.rs
    [ui] ui\macros\macros-nonfatal-errors.rs

LINK.EXE:
    [ui] ui\issues\issue-10755.rs
davidtwco (Aug 09 2018 at 22:26, on Zulip):

That's not half bad. URL errors are fixed by just running bless. File not found differences can be sorted with a normalize comment. Some of the path ones might be a tad more tricky but doable.

davidtwco (Aug 09 2018 at 22:27, on Zulip):

I wonder if any of the other platforms have different output for tests that we're not seeing because this build failed first. Would be annoying to need to go through the bors process once per platform that will have varying output.

davidtwco (Aug 09 2018 at 22:36, on Zulip):

Just noticed that one of the tests also has this:

+
+   note: the msvc targets depend on the msvc linker but `link.exe` was not found
+
+   note: please ensure that VS 2013, VS 2015 or VS 2017 was installed with the Visual C++ option

That'll be a pain to deal with.

davidtwco (Aug 09 2018 at 22:38, on Zulip):

no-link-with-link-args also has a bunch of extra wording at the bottom of its output that will make it harder to deal with.

nikomatsakis (Aug 09 2018 at 22:38, on Zulip):

@davidtwco uh....

nikomatsakis (Aug 09 2018 at 22:39, on Zulip):

that's annoying!

davidtwco (Aug 09 2018 at 22:40, on Zulip):

I might need to move one or two (more) tests back to compile-fail for the time being.

nikomatsakis (Aug 09 2018 at 22:55, on Zulip):

I guess in the limit we could have some kind of "skip stderr" flag

nikomatsakis (Aug 09 2018 at 22:55, on Zulip):

basically same thing as compile-fail, I guess

nikomatsakis (Aug 09 2018 at 22:55, on Zulip):

or "skip stderr on windows"

nikomatsakis (Aug 09 2018 at 22:55, on Zulip):

that...would be pretty ok

davidtwco (Aug 09 2018 at 22:57, on Zulip):

Yeah, it might be worth doing for these handful of tests.

nikomatsakis (Aug 09 2018 at 22:57, on Zulip):

I'm +1 on that

davidtwco (Aug 09 2018 at 22:57, on Zulip):

Or even a way to remove lines that match a regex.

nikomatsakis (Aug 09 2018 at 22:57, on Zulip):

it seems like checking the //~ ERROR etc is enough

nikomatsakis (Aug 09 2018 at 22:57, on Zulip):

Or even a way to remove lines that match a regex.

can't we normalize them to "" or something?

nikomatsakis (Aug 09 2018 at 22:57, on Zulip):

I guess the line remains

davidtwco (Aug 09 2018 at 22:58, on Zulip):

Yeah, if the lines aren't there on linux then it still fails.

nikomatsakis (Aug 09 2018 at 22:58, on Zulip):

well either seems ok

nikomatsakis (Aug 09 2018 at 22:58, on Zulip):

I'd probably just add a "skip stderr on <platform>" flag

nikomatsakis (Aug 09 2018 at 22:58, on Zulip):

but normalizing is obviously a touch better

nikomatsakis (Aug 09 2018 at 22:58, on Zulip):

and I guess it has another advantage:

nikomatsakis (Aug 09 2018 at 22:59, on Zulip):

devs on windows can run --bless and it will work

davidtwco (Aug 09 2018 at 22:59, on Zulip):

Yeah, that seems like a good idea for a follow-up PR. I'd probably just move them to compile-fail for this PR though. There are also ones I've already moved to compile-fail (~3 at the moment) that just don't work on ui at the moment.

davidtwco (Aug 09 2018 at 23:00, on Zulip):

EIther due to being just broken (the "metatest" one or invalid-module-declaration) or the output not being deterministic.

davidtwco (Aug 09 2018 at 23:00, on Zulip):

I'd really like to get this into a state where it can be landed tomorrow (I guess it's today now for me).

davidtwco (Aug 10 2018 at 08:44, on Zulip):

compile-fail to ui: Day 3
Everything is starting to look like a ui test...

nikomatsakis (Aug 10 2018 at 09:00, on Zulip):

@davidtwco are you waiting on an r+ or anything?

nikomatsakis (Aug 10 2018 at 09:01, on Zulip):

oh dear I see a lot of conflicts

davidtwco (Aug 10 2018 at 09:01, on Zulip):

Not at the moment. Doing a build after rebasing just now on my linux box, also have a Windows box building to work on the failures we see on Windows.

davidtwco (Aug 10 2018 at 09:22, on Zulip):

There's something worse about having to rebase because of my own pull requests.

davidtwco (Aug 10 2018 at 09:58, on Zulip):

What also might be a nice idea is a sort of // per-platform-stderr comment that has a .win.stderr and .linux.stderr instead of just .stderr for every compare mode and revision.

nikomatsakis (Aug 10 2018 at 10:02, on Zulip):

yeah, maybe, though I'd like to make it so that --bless works

nikomatsakis (Aug 10 2018 at 10:02, on Zulip):

I think maybe the idea of adding more kinds of normalization is better

davidtwco (Aug 10 2018 at 10:03, on Zulip):

Yeah, those stderr files would just need to be ignored on the other platform. Normalization is a better approach.

nikomatsakis (Aug 10 2018 at 10:03, on Zulip):

adding something that deletes lines entirely seems...not that hard?

nikomatsakis (Aug 10 2018 at 10:03, on Zulip):

note: we could also have platform specific normalization rules

davidtwco (Aug 10 2018 at 10:04, on Zulip):

Could have it so that there's a "only consider lines that match this regex on this platform" type of comment?

nikomatsakis (Aug 10 2018 at 10:06, on Zulip):

I'm not sure I understand

davidtwco (Aug 10 2018 at 10:07, on Zulip):

I guess it's just another way of saying "don't consider this line of the stderr on this platform".

davidtwco (Aug 10 2018 at 10:07, on Zulip):

Useful for the extra lines that sometimes are added to error output on Windows.

nikomatsakis (Aug 10 2018 at 10:09, on Zulip):

well i'd rather we have a windows-specific normalization that deletes them

nikomatsakis (Aug 10 2018 at 10:09, on Zulip):

because that way, when you run --bless, you get the result that other platforms want

davidtwco (Aug 10 2018 at 10:09, on Zulip):

Ah yeah, that makes sense.

davidtwco (Aug 10 2018 at 10:25, on Zulip):

Am I right in thinking that the only way this can be merged without conflicts is if it comes after a PR that made no changes to ui or compile-fail tests?

davidtwco (Aug 10 2018 at 10:28, on Zulip):

Alright, the PR doesn't conflict at the moment and _should_ fix all the Windows errors we saw (I can't test yet, still building LLVM on my Windows machine).

davidtwco (Aug 10 2018 at 10:28, on Zulip):

@nikomatsakis

nikomatsakis (Aug 10 2018 at 10:29, on Zulip):

Am I right in thinking that the only way this can be merged without conflicts is if it comes after a PR that made no changes to ui or compile-fail tests?

compile-fail tests yes... ui tests?

nikomatsakis (Aug 10 2018 at 10:29, on Zulip):

anyway r+ p=1

davidtwco (Aug 10 2018 at 10:29, on Zulip):

It moves around some tests to try keep the number of files/directories in the root of src/test/ui under 1000 so that GitHub can render them all at once. #46841

davidtwco (Aug 10 2018 at 10:30, on Zulip):

I'll need to rebase again after the current PR being tested finishes, so it'll need another r+ before it actually gets a chance to run.

nikomatsakis (Aug 10 2018 at 10:36, on Zulip):

d'oh

davidtwco (Aug 10 2018 at 12:16, on Zulip):

Going to have to rely on bors to test Windows I think, can't get rustc building with msvc because link.exe hits the library limit of 65535.

davidtwco (Aug 10 2018 at 12:19, on Zulip):

Seems to be #53014

nikomatsakis (Aug 10 2018 at 12:47, on Zulip):

hmm

nikomatsakis (Aug 10 2018 at 12:47, on Zulip):

I wonder if we should bump the priority on that (cc @scottmcm )

davidtwco (Aug 10 2018 at 12:49, on Zulip):

I'm fairly sure that the only issue that could come up on Windows is the difference between the "No such file or directory found" and "The system can not find the file specified" messages - I've put in a normalization for that, so it should be fixed, but I can't verify it.

davidtwco (Aug 10 2018 at 12:49, on Zulip):

Is there a reason a @bors try can't be used to test on other platforms? I'm not 100% sure what try does.

nikomatsakis (Aug 10 2018 at 12:52, on Zulip):

I don't know :)

davidtwco (Aug 10 2018 at 12:53, on Zulip):

Would just save me waiting on the bors queue to find out what needs fixed next.

simulacrum (Aug 10 2018 at 13:01, on Zulip):

@davidtwco Capacity, mostly -- you can manually fiddle with .travis.yml if you want to enable more, just don't enable all at once (go 1-2 at a time)

simulacrum (Aug 10 2018 at 13:01, on Zulip):

AppVeyor is much more limited on capacity so there we generally don't want to do try

davidtwco (Aug 10 2018 at 13:02, on Zulip):

That makes sense, I'll just wait on bors.

davidtwco (Aug 10 2018 at 14:42, on Zulip):

@nikomatsakis This needs a retry, failed because of internet weather.

davidtwco (Aug 10 2018 at 14:43, on Zulip):

At some point today I'll learn if it passes or not.

davidtwco (Aug 10 2018 at 14:45, on Zulip):

Thanks!

davidtwco (Aug 10 2018 at 17:01, on Zulip):

Managed to get my Windows build going by not passing -i - I can confirm that the non-NLL compare mode UI tests all pass on Windows.

davidtwco (Aug 10 2018 at 17:01, on Zulip):

I can't imagine any NLL ones will fail on Windows so should be good.

davidtwco (Aug 10 2018 at 17:01, on Zulip):

Outside of any rebases or any other platform dependant failures we've yet to see on other platforms, this PR should be good.

davidtwco (Aug 10 2018 at 17:03, on Zulip):

Turns out an NLL compare mode test did fail on Windows.

davidtwco (Aug 10 2018 at 17:03, on Zulip):

I spoke too soon.

davidtwco (Aug 10 2018 at 17:07, on Zulip):

In fact, not convinced that these two failures aren't just my machine (both link errors on tests that passed without compare-mode NLL, I can't imagine NLL affects link.exe on Windows so I'm assuming that the test is fine).

davidtwco (Aug 10 2018 at 17:10, on Zulip):

We'll see what the in-progress merge says I guess.

kennytm (Aug 10 2018 at 17:10, on Zulip):

pretty sure it will fail spuriously due to https://www.traviscistatus.com/incidents/pxj3py3xzn14 :shrug:

davidtwco (Aug 10 2018 at 17:10, on Zulip):

Yeah, I've noticed everything has been failing due to that today.

davidtwco (Aug 10 2018 at 17:43, on Zulip):

Well, Appveyor confirmed Windows now passes UI tests. That's nice.

davidtwco (Aug 10 2018 at 17:44, on Zulip):

But, Travis failed and killed the build.

davidtwco (Aug 10 2018 at 17:44, on Zulip):

Needs a retry.

nikomatsakis (Aug 10 2018 at 17:45, on Zulip):

:(

kennytm (Aug 10 2018 at 18:59, on Zulip):

https://ci.appveyor.com/project/rust-lang/rust/build/1.0.8538/job/qxoewi7y632c3tf4

nikomatsakis (Aug 10 2018 at 19:00, on Zulip):

https://ci.appveyor.com/project/rust-lang/rust/build/1.0.8538/job/qxoewi7y632c3tf4#L10790 <-- interesting part

davidtwco (Aug 10 2018 at 19:08, on Zulip):

Awesome, more tests to fix.

davidtwco (Aug 10 2018 at 21:51, on Zulip):

@nikomatsakis those should be fixed.

davidtwco (Aug 10 2018 at 22:31, on Zulip):

Thanks!

davidtwco (Aug 10 2018 at 23:51, on Zulip):

@nikomatsakis Fixed it again.

davidtwco (Aug 11 2018 at 08:12, on Zulip):

Did a rebase again - the Travis failures from last night (or early today I guess) were because the jobs merged with master which changed some tests instead of doing a merge conflict and saving us all the time. Should be good to test again until something else lands.

davidtwco (Aug 13 2018 at 08:30, on Zulip):

Doing another rebase locally now.

davidtwco (Aug 13 2018 at 09:57, on Zulip):

Alright, all rebased and ready for another bors run. @nikomatsakis

davidtwco (Aug 13 2018 at 13:25, on Zulip):

ping @nikomatsakis - any chance you could take a look and put another r+ on this so that I can see if there's more platforms that have issues?

davidtwco (Aug 13 2018 at 13:50, on Zulip):

Thanks!

davidtwco (Aug 13 2018 at 15:12, on Zulip):

Alright, fixed the failures from wasm32-unknown, @nikomatsakis, I guess another r+?

davidtwco (Aug 13 2018 at 15:48, on Zulip):

Thanks @kennytm

davidtwco (Aug 13 2018 at 15:49, on Zulip):

It'll fail because it's before a PR that makes UI test changes though. :frown:

kennytm (Aug 13 2018 at 15:53, on Zulip):

@davidtwco i think you mean to thank @varkor :smile:

davidtwco (Aug 13 2018 at 15:55, on Zulip):

Oops, I do. Thanks @varkor

davidtwco (Aug 13 2018 at 17:29, on Zulip):

Alright, fixed the armhf-gnu platform tests now. Pretty sure we've almost got all the platforms at this point. Need another r+ to get bors testing the remaining few.

varkor (Aug 13 2018 at 20:35, on Zulip):

@davidtwco: there are some conflicts now

davidtwco (Aug 13 2018 at 20:38, on Zulip):

@varkor: already got a build locally, just updating the PR now.

davidtwco (Aug 13 2018 at 20:40, on Zulip):

@varkor: PR updated

davidtwco (Aug 13 2018 at 20:41, on Zulip):

It's about to be invalidated again by the current PR (one of mine from last week) being merge tested by bors now.

varkor (Aug 13 2018 at 20:41, on Zulip):

ah, probably best to wait for that one to go through then?

davidtwco (Aug 13 2018 at 20:42, on Zulip):

I guess, though as soon as I do any rebase, there's always another PR being merged that will update the tests again.

davidtwco (Aug 13 2018 at 21:21, on Zulip):

Looks like my PR that was being merged and would have required this get rebased randomly failed, so this PR has a chance.

davidtwco (Aug 13 2018 at 22:38, on Zulip):

Ended up not being that lucky that time, updated the PR again.

davidtwco (Aug 14 2018 at 09:13, on Zulip):

Updated this PR again, fixed the failure from the last build and did a rebase. Looks like the currently in-progress PR doesn't add new tests so it would be ideal if this could come after it. @nikomatsakis @varkor @anyone-else-with-bors-rights

memoryruins (Aug 14 2018 at 09:38, on Zulip):

request: dont r+ my test PR until after david’s is landed if it might cause an issue haha

davidtwco (Aug 14 2018 at 09:40, on Zulip):

I wouldn't worry about it, every other PR will break this one and I'm still not sure whether it is passing on all platforms yet.

memoryruins (Aug 14 2018 at 09:57, on Zulip):

bless your patience

memoryruins (Aug 14 2018 at 10:01, on Zulip):

the amount of tests in there is impressive

davidtwco (Aug 14 2018 at 10:49, on Zulip):

Thanks @varkor

nikomatsakis (Aug 14 2018 at 12:04, on Zulip):

man I was hoping to see a "it landed :tada:" message here when I woke

varkor (Aug 14 2018 at 12:29, on Zulip):

@nikomatsakis: just go back to sleep ;)

davidtwco (Aug 14 2018 at 13:08, on Zulip):

Believe me, so was I.

davidtwco (Aug 14 2018 at 15:17, on Zulip):

It's finally over.

davidtwco (Aug 14 2018 at 15:34, on Zulip):

I've submitted #53353, #53352 and #53351 as follow-ups.

varkor (Aug 14 2018 at 15:52, on Zulip):

Great work @davidtwco!

lqd (Aug 14 2018 at 16:07, on Zulip):

@davidtwco :first_place_medal:

nikomatsakis (Aug 15 2018 at 13:28, on Zulip):

@davidtwco :snowboarder:

memoryruins (Aug 25 2018 at 21:14, on Zulip):

@simulacrum do cases like the following have a place on rustc-perf? fortunately it can be isolated in a single .rs file https://github.com/rust-lang/rust/issues/50614

simulacrum (Aug 26 2018 at 14:35, on Zulip):

Maybe, but I prefer that we don't add them unless it's something we want to track performance over time for

simulacrum (Aug 26 2018 at 14:35, on Zulip):

That seems like the kind of thing that is probably fix once

nikomatsakis (Aug 26 2018 at 15:16, on Zulip):

mm I'm not sure. I like adding regression tests.

nikomatsakis (Aug 26 2018 at 15:17, on Zulip):

In particular covering various extreme cases can be useful

nikomatsakis (Aug 26 2018 at 15:17, on Zulip):

although I guess this is not what I originally thought it was :)

nikomatsakis (Aug 26 2018 at 15:17, on Zulip):

anyway, we don't have a strict rule about this sort of case...sometimes we add a regr test, sometimes we don't...

simulacrum (Aug 26 2018 at 15:20, on Zulip):

regression tests are good but we currently can't add these without consideration for the time they're adding

nikomatsakis (Aug 26 2018 at 15:26, on Zulip):

seems fair :)

Last update: Nov 16 2019 at 02:20UTC