Keeping -explaintypes usable - adding it to all pos tests (SI-6123)

[Paolo G. Giarrusso]

Hi all,
how can I test that -explaintypes does not break again as in SI-6123? Daniel Sobral mentioned it already happened, so I'd like to add a test for it and the question is if I can and how.

With the current scalac architecture, I fear that -explaintypes is doomed to break again, since errors can be buffered, but -explaintypes output can't (yet).

This is one more reason to add tests for it — or drop it.

And I want to test that it produces no output unless compilation fails (there might be warnings to explain, I'm undecided on that). With my draft fixes (https://github.com/Blaisorblade/scala/tree/issue/6123), I'm running

SCALAC_OPTS="-deprecation -explaintypes" ./test/partest --pos --pack

without getting any extra output (yet), so I think we are there. Issues:

*) partest does not check scalac's standard output - or anyway did not break when unexpected messages from -explaintypes appear.

*) partest does not support any SCALAC_POS_OPTS, can I add them? Adding -explaintypes for all testcases would mean changing the expected output for all negative testcases, and doing that again for every little change to how -explaintypes behaves (which I fear would be a lot).

WDYT?

[Hubert Plociniczak]

The way to configure the compilation of tests in partest is through the .flags file (e.g. test/files/pos/new-test.scala would need test/files/pos/new-test.flags).

However .check files for testing explaintypes would potentially contain a lot of output so the cost of maintaining those could be pretty big for everyone. Maybe explaintypes should become a -Y option?

hubert

[Szabolcs Berecz]

Maybe scalac is printing to stderr in this case?

I don't know what's going on here exactly, but I started working on SI-6289 a couple weeks ago which is mostly about ignored stderr output. Currently, anything printed to stderr is ignored.

Also, while working on this bug I noticed that a lot of stdout output is also ignored by partest. It's mostly warnings and deprecation messages but there might be others, I haven't checked everything. Running "partest --update-checks" adds 1-2 thousand lines to the check files. What's causing it: the log file is passed around as a File in partest and it's almost never opened in append mode. So, this can also be the cause of your issues.

[Paolo G. Giarrusso]

Il giorno lunedì 21 gennaio 2013 14:21:45 UTC+1, Hubert Plociniczak ha scritto:

The way to configure the compilation of tests in partest is through the .flags file (e.g. test/files/pos/new-test.scala would need test/files/pos/new-test.flags).

However .check files for testing explaintypes would potentially contain a lot of output so the cost of maintaining those could be pretty big for everyone. Maybe explaintypes should become a -Y option?

hubert

Maybe I didn't make myself clear, but my main proposal is to ensure that adding -explaintypes to *all* pos tests *does not produce* any output, so no check files would need any change. 0 maintenance costs (until -explaintypes is broken again). And I know of flags files, but using them here seems stupid - I could add -explaintypes to all of them, but I'd rather not.

Current merge rules require of me to have proper testcases, and I think that one (1) small file which fails and produces output using -explaintypes should be acceptable maintenance cost - but practice suggests that it's more important to test that -explaintypes is quiet when compilation succeeds.

[Paolo G. Giarrusso]

Il giorno lunedì 21 gennaio 2013 14:24:25 UTC+1, szabolcs.berecz ha scritto:

Maybe scalac is printing to stderr in this case?

Nope, it's using Console.println:

  protected def explain[T](op: String, p: (Type, T) => Boolean, tp1: Type, arg2: T): Boolean = {

    Console.println(indent + tp1 + " " + op + " " + arg2 + "?" /* + "("+tp1.getClass+","+arg2.getClass+")"*/)

and I remember clearly adding debug println statements without failing tests.

 

I don't know what's going on here exactly, but I started working on SI-6289 a couple weeks ago which is mostly about ignored stderr output. Currently, anything printed to stderr is ignored.

That would be very bad, but luckily that's only for javac. I just saw this failure from partest on master, and the output is on stderr.

testing: [...]/files/neg/migration28.scala                            [FAILED]

1d0

< warning: -Xmigration is deprecated: This setting is no longer useful and will be removed. Please remove it from your build.

7c6

< two warnings found

---

> one warning found

Also, while working on this bug I noticed that a lot of stdout output is also ignored by partest.

Yes, that's more like it.

It's mostly warnings and deprecation messages but there might be others, I haven't checked everything. Running "partest --update-checks" adds 1-2 thousand lines to the check files. What's causing it: the log file is passed around as a File in partest and it's almost never opened in append mode. So, this can also be the cause of your issues. 

That should also be filed under JIRA, in the same or another bug. Can you do that, since you have more clue?

Also, I reproduced the problem with the test and raised the priority of the bug to blocker. Doesn't that mean that tests for compatibility with Java are being ignored? That would be bad. Strictly speaking, no other change whatsoever should be merged until such a bug is fixed.

Or is the bug specific to executing individual tests? In that case (which would be weird), I apologize for the fuss.

[Szabolcs Berecz]

Here is the ticket for the ignored stdout: https://issues.scala-lang.org/browse/SI-7003
Half baked fix: https://github.com/khernyo/scala/commits/unstable/issue/7003

[Paolo G. Giarrusso]

Il giorno lunedì 21 gennaio 2013 20:12:09 UTC+1, szabolcs.berecz ha scritto:

Here is the ticket for the ignored stdout: https://issues.scala-lang.org/browse/SI-7003
Half baked fix: https://github.com/khernyo/scala/commits/unstable/issue/7003

Thanks, looking at that code also helped me implement my extension to partest to add --explaintypes to pos/run tests.

You've already seen that I depend on that bug.

[Szabolcs Berecz]

On Tue, Jan 22, 2013 at 4:32 AM, Paolo G. Giarrusso <p.gia...@gmail.com> wrote:

Il giorno lunedì 21 gennaio 2013 20:12:09 UTC+1, szabolcs.berecz ha scritto:

Here is the ticket for the ignored stdout: https://issues.scala-lang.org/browse/SI-7003
Half baked fix: https://github.com/khernyo/scala/commits/unstable/issue/7003

Thanks, looking at that code also helped me implement my extension to partest to add --explaintypes to pos/run tests.

You've already seen that I depend on that bug.

I'll try to fix it as soon as possible but I don't think it will be ready before the weekend.