Stream: t-compiler/wg-nll

Topic: compare-mode notes


pnkfelix (May 28 2018 at 13:51, on Zulip):

@Santiago Pastorino @qmx here are some notes on the --compare-mode parameter to compiletest

pnkfelix (May 28 2018 at 13:52, on Zulip):

the set of compare "modes" is here: https://github.com/rust-lang/rust/blob/master/src/tools/compiletest/src/common.rs#L100

pnkfelix (May 28 2018 at 13:53, on Zulip):

there's only one currently (Nll) but the intent is to let people add others as needed.

pnkfelix (May 28 2018 at 13:54, on Zulip):

you can add the code to parse and print it immediately below the enum definition, using the pattern established for "nll"

pnkfelix (May 28 2018 at 13:56, on Zulip):

I think the only other thing you'd need to change is the code here: https://github.com/rust-lang/rust/blob/master/src/tools/compiletest/src/runtest.rs#L1728

pnkfelix (May 28 2018 at 13:56, on Zulip):

(which the compiler will tell you anyway once you've added the new variant for polonius)

Santiago Pastorino (May 28 2018 at 13:58, on Zulip):

cool, thanks

Santiago Pastorino (May 28 2018 at 18:23, on Zulip):

@nikomatsakis @pnkfelix so ... I'm going to do this now

Santiago Pastorino (May 28 2018 at 18:24, on Zulip):

I'm going to sit this code on top of previous PR, but I can't really prepare a proper PR until the other one lands, if I'm not wrong there's no way to do that

nikomatsakis (May 28 2018 at 18:27, on Zulip):

sounds fine

Santiago Pastorino (May 28 2018 at 18:27, on Zulip):

do we want to compare against nll then?

Santiago Pastorino (May 28 2018 at 18:28, on Zulip):

so to start talking with code

nikomatsakis (May 28 2018 at 18:28, on Zulip):

that would be ideal

nikomatsakis (May 28 2018 at 18:28, on Zulip):

I don't think that should be too hard

nikomatsakis (May 28 2018 at 18:28, on Zulip):

I guess we'd have to add some extra "per compare mode" data

nikomatsakis (May 28 2018 at 18:29, on Zulip):

to indicate the "base compare mode" (if any)

Santiago Pastorino (May 28 2018 at 18:29, on Zulip):

https://github.com/rust-lang/rust/pull/51138

Santiago Pastorino (May 28 2018 at 18:29, on Zulip):

yep

Santiago Pastorino (May 28 2018 at 18:29, on Zulip):

this PR compares against default

nikomatsakis (May 28 2018 at 18:29, on Zulip):

cool

Santiago Pastorino (May 28 2018 at 18:29, on Zulip):

anyway as a starting point is that what you wanted?

nikomatsakis (May 28 2018 at 18:29, on Zulip):

I edited the title to include [WIP] which will prevent it from being merged until we are ready :)

Santiago Pastorino (May 28 2018 at 18:29, on Zulip):

:+1:

nikomatsakis (May 28 2018 at 18:30, on Zulip):

@Santiago Pastorino does that PR modify compare-mode at all?

nikomatsakis (May 28 2018 at 18:30, on Zulip):

if so, I don't see the corresponding commit

nikomatsakis (May 28 2018 at 18:30, on Zulip):

maybe you didn't push it :)

Santiago Pastorino (May 28 2018 at 18:31, on Zulip):

:)

Santiago Pastorino (May 28 2018 at 18:31, on Zulip):

now is there

nikomatsakis (May 28 2018 at 18:31, on Zulip):

that looks right

Santiago Pastorino (May 28 2018 at 18:33, on Zulip):

ok, need to figure out how to compare against nll

Santiago Pastorino (May 28 2018 at 18:34, on Zulip):

so ... with this you can do -Zborrowck=compare and I guess it would compare all the cases against default?

Santiago Pastorino (May 28 2018 at 18:35, on Zulip):

if that's the case we want to swap default (current stable behavior) with nll

Santiago Pastorino (May 28 2018 at 18:35, on Zulip):

because we want default vs nll and nll vs polonius

nikomatsakis (May 28 2018 at 18:36, on Zulip):

I'm confused

nikomatsakis (May 28 2018 at 18:36, on Zulip):

are you saying, we would also modify the behavior of borrowck=compare?

nikomatsakis (May 28 2018 at 18:36, on Zulip):

if so, that seems like a separate thing, and I wouldn't worry about it

Santiago Pastorino (May 28 2018 at 18:38, on Zulip):

let me ask this better

Santiago Pastorino (May 28 2018 at 18:39, on Zulip):

how does compare-mode works from a user perspective?

Santiago Pastorino (May 28 2018 at 18:39, on Zulip):

you pass --compare-mode nll as flag and you get what exactly?

Santiago Pastorino (May 28 2018 at 18:40, on Zulip):

I guess it runs the whole test suite and compare nll vs default and gives diffs or something like that?

nikomatsakis (May 28 2018 at 18:41, on Zulip):

nope

nikomatsakis (May 28 2018 at 18:41, on Zulip):

:)

nikomatsakis (May 28 2018 at 18:41, on Zulip):

I mean, sort of

nikomatsakis (May 28 2018 at 18:41, on Zulip):

it runs the tests in NLL mode

nikomatsakis (May 28 2018 at 18:41, on Zulip):

for each test, it looks first for a foo.nll.stderr reference

nikomatsakis (May 28 2018 at 18:41, on Zulip):

if it finds one, it compares the output against that

nikomatsakis (May 28 2018 at 18:42, on Zulip):

otherwise, it looks for foo.stderr (the normal output) and compares against that

nikomatsakis (May 28 2018 at 18:42, on Zulip):

if the output doesn't match, it errors, as usual

nikomatsakis (May 28 2018 at 18:42, on Zulip):

the idea then is that — once everything is passing — you can search for *.nll.stderr and you see which tests have output in NLL mode that differs from the output in regular mode

nikomatsakis (May 28 2018 at 18:42, on Zulip):

so what we would do differently here is

nikomatsakis (May 28 2018 at 18:42, on Zulip):

in polonius mode we would first look for foo.polonius.stderr

Santiago Pastorino (May 28 2018 at 18:42, on Zulip):

I see when in polonius we need to check foo.polonius.stderr otherwise foo.nll.stderr

Santiago Pastorino (May 28 2018 at 18:42, on Zulip):

?

nikomatsakis (May 28 2018 at 18:43, on Zulip):

if we fail to find that, we would look for foo.nll.stderr

nikomatsakis (May 28 2018 at 18:43, on Zulip):

and .. yeah

Santiago Pastorino (May 28 2018 at 18:43, on Zulip):

:+1:

nikomatsakis (May 28 2018 at 18:43, on Zulip):

well, if no foo.nll.stderr, then look for foo.stderr

Santiago Pastorino (May 28 2018 at 18:43, on Zulip):

ok

nikomatsakis (May 28 2018 at 18:43, on Zulip):

you can then run with --bless (I presume this is impl'd...) and it will create the .polonius.stderr files

nikomatsakis (May 28 2018 at 18:43, on Zulip):

so we can compare

Santiago Pastorino (May 28 2018 at 18:45, on Zulip):

:+1:

Santiago Pastorino (May 28 2018 at 18:45, on Zulip):

https://github.com/rust-lang/rust/blob/master/src/bootstrap/test.rs#L1126-L1139

Santiago Pastorino (May 28 2018 at 18:46, on Zulip):

it run the tests in the new mode and old mode?

Santiago Pastorino (May 28 2018 at 18:46, on Zulip):

or what's the first try_run doing?

Santiago Pastorino (May 28 2018 at 18:47, on Zulip):

I mean, seems like it runs both things but that contradicts what you say that it runs the mode you passed and just compare with the stderr file

nikomatsakis (May 28 2018 at 18:49, on Zulip):

I don't believe it runs both things

nikomatsakis (May 28 2018 at 18:49, on Zulip):

but let me check :)

nikomatsakis (May 28 2018 at 18:49, on Zulip):

well, also, you are looking at the wrong code...

Santiago Pastorino (May 28 2018 at 18:49, on Zulip):

:P

nikomatsakis (May 28 2018 at 18:50, on Zulip):

https://github.com/rust-lang/rust/blob/master/src/tools/compiletest/src/runtest.rs#L2894-L2907

nikomatsakis (May 28 2018 at 18:50, on Zulip):

that is maybe more the relevant code

Santiago Pastorino (May 28 2018 at 18:50, on Zulip):

what does the compare-mode in the code I pasted means?

nikomatsakis (May 28 2018 at 18:51, on Zulip):

same thing probably, but that's the x.py sources

nikomatsakis (May 28 2018 at 18:51, on Zulip):

or not even

Santiago Pastorino (May 28 2018 at 18:51, on Zulip):

ok

nikomatsakis (May 28 2018 at 18:51, on Zulip):

it probably does run with both modes

Santiago Pastorino (May 28 2018 at 18:52, on Zulip):

if it runs in both modes I guess I need for polonius to run in polonius and nll

Santiago Pastorino (May 28 2018 at 18:53, on Zulip):

and not just changing the paths

nikomatsakis (May 28 2018 at 18:54, on Zulip):

no

nikomatsakis (May 28 2018 at 18:54, on Zulip):

we do not want to change the bootstrap sequence to run polonius :)

nikomatsakis (May 28 2018 at 18:54, on Zulip):

we just want to make it possible to run by hand ...

nikomatsakis (May 28 2018 at 18:54, on Zulip):

or maybe I don't follow?

Santiago Pastorino (May 28 2018 at 18:58, on Zulip):

hmm I see

Santiago Pastorino (May 28 2018 at 19:07, on Zulip):

@nikomatsakis I've pushed something, how to you test this you said?

nikomatsakis (May 28 2018 at 21:20, on Zulip):

seems like it would be useful to add --compare-mode as an argument to x.py test

nikomatsakis (May 28 2018 at 21:20, on Zulip):

I could drop a few notes on how to do that @Santiago Pastorino if you want

Santiago Pastorino (May 28 2018 at 21:22, on Zulip):

yes

Santiago Pastorino (May 28 2018 at 21:23, on Zulip):

sharing the output

Santiago Pastorino (May 28 2018 at 21:23, on Zulip):

your trick worked

Santiago Pastorino (May 28 2018 at 21:24, on Zulip):

test result: FAILED. 1389 passed; 57 failed; 19 ignored; 0 measured; 0 filtered out

Santiago Pastorino (May 28 2018 at 21:24, on Zulip):

if everything is right, this ^ is what's going on

Santiago Pastorino (May 28 2018 at 21:24, on Zulip):

https://gist.github.com/spastorino/ed32ad31bb3f094aa86d103f5d9cc507

Santiago Pastorino (May 28 2018 at 21:29, on Zulip):

I think there might be some issues with update-all-references

nikomatsakis (May 28 2018 at 21:33, on Zulip):

cool can't look in depth now but if you wanted to add a --compare-mode option,

nikomatsakis (May 28 2018 at 21:33, on Zulip):

I would edit this file I think https://github.com/rust-lang/rust/blob/master/src/bootstrap/flags.rs

nikomatsakis (May 28 2018 at 21:34, on Zulip):

trying to follow the model of test-args

Santiago Pastorino (May 28 2018 at 21:36, on Zulip):

:+1:

Santiago Pastorino (May 28 2018 at 22:59, on Zulip):

hey @nikomatsakis I had some spare minutes and committed the thing

Santiago Pastorino (May 28 2018 at 22:59, on Zulip):

https://github.com/rust-lang/rust/pull/51138

pnkfelix (May 29 2018 at 00:05, on Zulip):

That sounds more complicated than what I was imagining

pnkfelix (May 29 2018 at 00:06, on Zulip):

Note that today compare-mode does not itself do any comparison

pnkfelix (May 29 2018 at 00:06, on Zulip):

It just provides a hook for feeding in an alternative set of expected diagnostics

nikomatsakis (May 29 2018 at 00:06, on Zulip):

what are you referring to @pnkfelix by "that"?

pnkfelix (May 29 2018 at 00:07, on Zulip):

“That” is @Santiago Pastorino ‘s messages about employing -Zborrowck-mode=compare

nikomatsakis (May 29 2018 at 00:08, on Zulip):

ah, yes, agreed.

pnkfelix (May 29 2018 at 00:09, on Zulip):

All I was picturing was revising the bit of code that currently decides what .mode.stderr file to fall back on

nikomatsakis (May 29 2018 at 00:10, on Zulip):

@pnkfelix this is what https://github.com/rust-lang/rust/pull/51138/ does

pnkfelix (May 29 2018 at 00:25, on Zulip):

Ah okay. That’s basically all I imagined we’d use for now.

Santiago Pastorino (May 29 2018 at 00:25, on Zulip):

not sure I follow

pnkfelix (May 29 2018 at 00:26, on Zulip):

Though I too expected a more tabular approach. I considered a parent mapping, but I worry that could yield infinite loops if people set up bidirectional parent chain

nikomatsakis (May 29 2018 at 00:28, on Zulip):

seems like "don't do that" is an acceptable answer =)

pnkfelix (May 29 2018 at 00:29, on Zulip):

My “table” was going to be a method on the enum that produces the &[&str] of fallback strings to look for. Then it’s just a for loop; termination is self-evident.

nikomatsakis (May 29 2018 at 00:29, on Zulip):

seems also ok

Santiago Pastorino (May 29 2018 at 00:30, on Zulip):

I didn't understand quite correct, is the PR wrong in some way?

nikomatsakis (May 29 2018 at 00:30, on Zulip):

not wrong, just more hard-coded than expected; I left some comments

pnkfelix (May 29 2018 at 00:30, on Zulip):

@Santiago Pastorino pr is not “wrong”

pnkfelix (May 29 2018 at 00:30, on Zulip):

I’m just saying I agree with Niko’s nitpick

pnkfelix (May 29 2018 at 00:31, on Zulip):

But that’s something we can revise later

Santiago Pastorino (May 29 2018 at 00:31, on Zulip):

what's the nitpick?

pnkfelix (May 29 2018 at 00:31, on Zulip):

When we have more variants in the enum

nikomatsakis (May 29 2018 at 00:31, on Zulip):

I don't remember how the command-line options for compare-mode are specified

nikomatsakis (May 29 2018 at 00:31, on Zulip):

I think in my ideal world there would be some central "table" defined either using a macro or a constant

nikomatsakis (May 29 2018 at 00:32, on Zulip):

that lists each compare mode along with its command line options and parent mode(s) etc

pnkfelix (May 29 2018 at 00:32, on Zulip):

Probably should just point to the comment on the PR

nikomatsakis (May 29 2018 at 00:32, on Zulip):

what's the nitpick?

see this comment @Santiago Pastorino

pnkfelix (May 29 2018 at 00:32, on Zulip):

oh it sounds like Niko was going even further than I was with his thinking

Santiago Pastorino (May 29 2018 at 00:33, on Zulip):

ahhhh

Santiago Pastorino (May 29 2018 at 00:33, on Zulip):

sorry was with the phone and had no idea about those comments

Santiago Pastorino (May 29 2018 at 00:34, on Zulip):

yes, definitely agree

Santiago Pastorino (May 29 2018 at 00:34, on Zulip):

Maybe it's not worth making a super general mechanism here, given that we'll hopefully be removing polonius and nll "soon".

Santiago Pastorino (May 29 2018 at 00:34, on Zulip):

this is basically why my code is kind of very specific and easy to remove

Santiago Pastorino (May 29 2018 at 00:35, on Zulip):

but yeah agree

Santiago Pastorino (May 29 2018 at 00:36, on Zulip):

I guess I can implement what Niko said

nikomatsakis (May 29 2018 at 00:36, on Zulip):

at minimum a method defined on CompareMode seems fine

Santiago Pastorino (May 29 2018 at 00:36, on Zulip):

if you want that now

nikomatsakis (May 29 2018 at 00:37, on Zulip):

I'd prefer something more centralized, seems like a small diff

nikomatsakis (May 29 2018 at 00:37, on Zulip):

anyway, i'm going to bed :) we can chat tomorrow

Santiago Pastorino (May 29 2018 at 00:37, on Zulip):

:+1:

Santiago Pastorino (May 29 2018 at 17:03, on Zulip):

@nikomatsakis just force pushed this thing with your tiny suggestion

Santiago Pastorino (May 29 2018 at 17:04, on Zulip):

https://github.com/rust-lang/rust/pull/51138

Santiago Pastorino (May 29 2018 at 17:04, on Zulip):

we can do that table also

Santiago Pastorino (May 29 2018 at 17:04, on Zulip):

or if @Vytautas Astrauskas is interested in doing it I'm more than happy to leave that up to him

Santiago Pastorino (May 29 2018 at 17:04, on Zulip):

let me know :)

Vytautas Astrauskas (May 29 2018 at 17:53, on Zulip):

@Santiago Pastorino I would be interested, but I am not sure about my ability to look properly into it until tomorrow afternoon. (My train has been cancelled because of the obvious reasons and I am now somewhere in France :relaxed:.)

Santiago Pastorino (May 29 2018 at 17:59, on Zulip):

more than happy to leave it up to you :)

Santiago Pastorino (May 29 2018 at 18:00, on Zulip):

we need this https://github.com/rust-lang/rust/pull/51138 merged

Vytautas Astrauskas (May 29 2018 at 18:03, on Zulip):

OK, then I'll try to get it done. :relaxed:

Santiago Pastorino (May 29 2018 at 18:04, on Zulip):

@nikomatsakis https://github.com/rust-lang/rust/pull/51138 is ready for review, since the other one is already merged

Santiago Pastorino (May 29 2018 at 18:04, on Zulip):

I guess as soon as CI says so it's ready to merge

Santiago Pastorino (May 29 2018 at 18:10, on Zulip):

code is wrong :)

Santiago Pastorino (May 29 2018 at 19:34, on Zulip):

@nikomatsakis https://github.com/rust-lang/rust/pull/51138 should be good now, CI is running mingw-check is green so it will be good I guess :)

Santiago Pastorino (May 29 2018 at 20:39, on Zulip):

@pnkfelix @nikomatsakis https://github.com/rust-lang/rust/pull/51138 this is now ready for review, CI is green

pnkfelix (May 29 2018 at 20:43, on Zulip):

did you address @nikomatsakis 's comment here https://github.com/rust-lang/rust/pull/51138/commits/3f137432558d891389e09459801a0ccc93c84721#r191282951 ?

pnkfelix (May 29 2018 at 20:44, on Zulip):

hmm let me double check this

Santiago Pastorino (May 29 2018 at 20:44, on Zulip):

yep is the line here https://github.com/rust-lang/rust/pull/51138/commits/3f137432558d891389e09459801a0ccc93c84721#diff-d5384138c7b75a10635160076a792999R967

Santiago Pastorino (May 29 2018 at 20:44, on Zulip):

it wasn't going that

pnkfelix (May 29 2018 at 20:49, on Zulip):

@Santiago Pastorino maybe you can just tell me in words: What did you do to respond to Niko's feedback on the x.py changes?

pnkfelix (May 29 2018 at 20:49, on Zulip):

because right now I'm having trouble inferring it from the github presentation

pnkfelix (May 29 2018 at 20:53, on Zulip):

it wasn't going that

also I do not understand what the above sentence means. :)

Santiago Pastorino (May 29 2018 at 20:54, on Zulip):

I don't either

Santiago Pastorino (May 29 2018 at 20:54, on Zulip):

lol

Santiago Pastorino (May 29 2018 at 20:54, on Zulip):

I tried to say that that line is changed

Santiago Pastorino (May 29 2018 at 20:54, on Zulip):

and it's doing what Niko suggest

Santiago Pastorino (May 29 2018 at 20:55, on Zulip):

@Santiago Pastorino maybe you can just tell me in words: What did you do to respond to Niko's feedback on the x.py changes?

what changes specifically?

pnkfelix (May 29 2018 at 20:55, on Zulip):

oh, so is github just attaching an old comment of niko's to some new code?

Santiago Pastorino (May 29 2018 at 20:55, on Zulip):

exactly

pnkfelix (May 29 2018 at 20:55, on Zulip):

goofy

Santiago Pastorino (May 29 2018 at 20:55, on Zulip):

I guess that's because Niko commented on a different line

pnkfelix (May 29 2018 at 20:55, on Zulip):

okay I get it now

pnkfelix (May 29 2018 at 20:55, on Zulip):

for some reason I thought he was pointing you to make an edit somewhere further away

Santiago Pastorino (May 29 2018 at 20:55, on Zulip):

if about Niko's changes you mean the generalization, I was guessing that we were going to leave that for a followup PR

pnkfelix (May 29 2018 at 20:56, on Zulip):

/me can't resist showing a screen shot

pnkfelix (May 29 2018 at 20:57, on Zulip):
pnkfelix (May 29 2018 at 20:58, on Zulip):

that image might explain why I was confusedly trying to identify what niko was asking you to do. :)

Santiago Pastorino (May 29 2018 at 21:06, on Zulip):

I see a broken thing

pnkfelix (May 29 2018 at 21:07, on Zulip):

The actual link is : https://pasteboard.co/HntnegB.png

Santiago Pastorino (May 29 2018 at 21:07, on Zulip):

got it by clicking and downloading

pnkfelix (May 29 2018 at 21:07, on Zulip):

zulip wont render it inline

Santiago Pastorino (May 29 2018 at 21:07, on Zulip):

ahh yeah

Santiago Pastorino (May 29 2018 at 21:07, on Zulip):

that's why I was telling you

Santiago Pastorino (May 29 2018 at 21:07, on Zulip):

I think he commented on the other line

pnkfelix (May 29 2018 at 21:07, on Zulip):

anyway yeah I was just confused by github

pnkfelix (May 29 2018 at 21:08, on Zulip):

its been r+'ed now

Santiago Pastorino (May 29 2018 at 21:08, on Zulip):

he commented 968, instead of 967

Santiago Pastorino (May 29 2018 at 21:08, on Zulip):

@pnkfelix cool, thanks

Santiago Pastorino (May 29 2018 at 21:08, on Zulip):

@Vytautas Astrauskas you can start the other part when this is merged, it's now r+

Santiago Pastorino (May 31 2018 at 03:20, on Zulip):

https://github.com/rust-lang/rust/pull/51138 was merged

Santiago Pastorino (May 31 2018 at 03:20, on Zulip):

@Vytautas Astrauskas ^^^

Vytautas Astrauskas (Jun 04 2018 at 08:22, on Zulip):

I have been working on this “table refactoring” and got something wrong. While debugging I noticed that all panics in compiletest seem to be silently ignored.

Vytautas Astrauskas (Jun 04 2018 at 08:25, on Zulip):

Could someone explain me why? Also, is it possible to disable this at least temporary?

Vytautas Astrauskas (Jun 04 2018 at 08:31, on Zulip):

For example, if I change https://github.com/rust-lang/rust/blob/6232478d26b0feca02fd6660edbf78a5c6327ec5/src/tools/compiletest/src/runtest.rs#L2924 to

    #[allow(warnings)]
    fn expected_output_path(&self, kind: &str) -> PathBuf {
        panic!();

and run ./x.py test I still get Build completed successfully in 0:04:53.

pnkfelix (Jun 04 2018 at 10:35, on Zulip):

I am unaware of the behavior that you describe... it sounds like a big bug to me

pnkfelix (Jun 04 2018 at 10:35, on Zulip):

(perhaps a bug in x.py or its related infrastructure?)

Vytautas Astrauskas (Jun 04 2018 at 11:40, on Zulip):

@pnkfelix I tried to investigate; however, I do not understand how it works. More specifically, I have not managed to find a place where the panics are caught.

pnkfelix (Jun 04 2018 at 11:41, on Zulip):

have you tried running compiletest directly rather than via x.py ?

pnkfelix (Jun 04 2018 at 11:41, on Zulip):

if you do x.py with --verbose, then you'll see the actual command lines it is issuing

pnkfelix (Jun 04 2018 at 11:41, on Zulip):

and then you can run one of those yourself

pnkfelix (Jun 04 2018 at 11:42, on Zulip):

The idea here would be to isolate whether the panics are being supressed by the compiletest binary somehow, or if they are being caught by python, or something else.

Vytautas Astrauskas (Jun 04 2018 at 11:42, on Zulip):

have you tried running compiletest directly rather than via x.py ?

No, I did not. I will try this as soon as it finishes to compile.

pnkfelix (Jun 04 2018 at 11:42, on Zulip):

(My hypothesis is that x.py is deliberately capturing the stderr and error exit code as part of its general approach, and then it fails to bubble it up in this case.)

Vytautas Astrauskas (Jun 04 2018 at 11:43, on Zulip):

One question: does x.py provide the set of test files to compiletest, or it collects them itself?

pnkfelix (Jun 04 2018 at 11:45, on Zulip):

I believe x.py can (or must?) supply the root directories for compiletest to use

pnkfelix (Jun 04 2018 at 11:45, on Zulip):

and then compiletest collects the files starting from those roots

pnkfelix (Jun 04 2018 at 11:46, on Zulip):

The "can or must?" is a question for me just because I always supply the subset of the full directory set myself when I invoke x.py test

Vytautas Astrauskas (Jun 04 2018 at 11:46, on Zulip):

I noticed that when I run with --compare-mode nll the version with the panic injected reports test result: FAILED. 0 passed; 4 failed; 3016 ignored; 0 measured; 0 filtered out (note that all tests were ignored).

Vytautas Astrauskas (Jun 04 2018 at 11:47, on Zulip):

I see. I will play around. Thanks!

Vytautas Astrauskas (Jun 04 2018 at 12:05, on Zulip):

@pnkfelix Here is the gist: https://gist.github.com/vakaras/10f67d18817182c46cc670bcee7d392d

pnkfelix (Jun 04 2018 at 12:05, on Zulip):

Aha

pnkfelix (Jun 04 2018 at 12:05, on Zulip):

The ignored tests

Vytautas Astrauskas (Jun 04 2018 at 12:06, on Zulip):

It seems that the problem is in compiletest.

pnkfelix (Jun 04 2018 at 12:06, on Zulip):

are usually from

pnkfelix (Jun 04 2018 at 12:06, on Zulip):

the files that are touched

pnkfelix (Jun 04 2018 at 12:06, on Zulip):

in a previous run

pnkfelix (Jun 04 2018 at 12:06, on Zulip):

There are files that are stamped

pnkfelix (Jun 04 2018 at 12:06, on Zulip):

on a given run

pnkfelix (Jun 04 2018 at 12:06, on Zulip):

to avoid re-running tests unnecessarily

pnkfelix (Jun 04 2018 at 12:06, on Zulip):

and I think the timestamp chekcs

pnkfelix (Jun 04 2018 at 12:06, on Zulip):

are only compared to the rustc build's timestamp

pnkfelix (Jun 04 2018 at 12:06, on Zulip):

and not (I think) to the compiletest time stamp

pnkfelix (Jun 04 2018 at 12:07, on Zulip):

So this might not be a bug in compiletest itself

pnkfelix (Jun 04 2018 at 12:07, on Zulip):

the way I would double check this

pnkfelix (Jun 04 2018 at 12:07, on Zulip):

is remove the timestamp files

Vytautas Astrauskas (Jun 04 2018 at 12:08, on Zulip):

Where can I find these timestamp files?

pnkfelix (Jun 04 2018 at 12:08, on Zulip):

I was just finding the directory

pnkfelix (Jun 04 2018 at 12:08, on Zulip):

look in your equivalent of:

pnkfelix (Jun 04 2018 at 12:09, on Zulip):

build/x86_64-unknown-linux-gnu/test/

Vytautas Astrauskas (Jun 04 2018 at 12:14, on Zulip):

So, after executing rm -f build/x86_64-unknown-linux-gnu/test/ui/*/stamp, I get test result: ok. 504 passed; 0 failed; 982 ignored; 0 measured; 0 filtered out.

Vytautas Astrauskas (Jun 04 2018 at 12:18, on Zulip):

Ok, I am confused. When I executed ./x.py test again, I got many messages like this: thread '[ui] ui/svh-change-type-static.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:2883:9.

pnkfelix (Jun 04 2018 at 12:24, on Zulip):

Sorry, I don't

pnkfelix (Jun 04 2018 at 12:24, on Zulip):

I don't think there's enough info about the command sequence

pnkfelix (Jun 04 2018 at 12:24, on Zulip):

for me to provide useful feedback

pnkfelix (Jun 04 2018 at 12:24, on Zulip):

I guess keep your eye out for future seeming discrepancies, and if you can reproduce one reliably, then post the command sequence to a gist or something and share it

Vytautas Astrauskas (Jun 04 2018 at 12:25, on Zulip):

I think I finally got it. So, it seems that you were right.

pnkfelix (Jun 04 2018 at 12:25, on Zulip):

but at this point the important thing is that you are now observing the panics.

Vytautas Astrauskas (Jun 04 2018 at 12:27, on Zulip):

I think repro steps are (still need to test):
1. Clone repository.
2. Run ./x.py build && ./x.py test – all tests should pass.
3. Modify some of the compiletest files in a way that causes a panic.
4. Run ./x.py test – since rustc was not recompiled and all tests were already executed with it, all of them are ignored.

pnkfelix (Jun 04 2018 at 12:31, on Zulip):

Right, so maybe the core problem here is that we are not comparing the file timestamp for compiletest itself to the timestamps of the tests, right?

Vytautas Astrauskas (Jun 04 2018 at 12:33, on Zulip):

I think so.

Vytautas Astrauskas (Jun 04 2018 at 12:33, on Zulip):

Would it be hard to implement such comparison?

pnkfelix (Jun 04 2018 at 12:37, on Zulip):

Probably not. (Famous last words.)

pnkfelix (Jun 04 2018 at 12:37, on Zulip):

But check if there's already a bug filed about this

pnkfelix (Jun 04 2018 at 12:38, on Zulip):

because sometimes when there is such a bug, there's also a record of previous attempts to fix (and where they went wrong or why it was harder than expected)

Vytautas Astrauskas (Jun 04 2018 at 12:45, on Zulip):

I do not see any related issue in the issue tracker.

Vytautas Astrauskas (Jun 04 2018 at 12:51, on Zulip):

I will try to fix this one once I have a free moment.

Last update: Nov 22 2019 at 01:00UTC