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

Partest separate compilation with Java files in the neg category does not record Javac errors #6289

Closed
scabug opened this issue Aug 27, 2012 · 11 comments
Assignees
Labels
Milestone

Comments

@scabug
Copy link

scabug commented Aug 27, 2012

  1. Create a directory myBug in neg in Partest with:

file_1.scala:

class Const { val x = 1 }

file_2.java:

public class file_2 {
    public static void foo() {
        Const c = new Const();
        c.x = 2;
    }
}

Running:

test/partest --show-log test/files/neg/

gives:

Testing individual files
test/files/neg/myBug/file_2.java:4: error: x has private access in Const
        c.x = 2;
         ^
1 error
testing: [...]/files/neg/myBug                                        [  OK  ]
All of 1 tests were successful (elapsed time: 00:00:02)

although there is no check file with the produced output.

  1. Also, there are no compilation errors being printed out for run tests.
@scabug
Copy link
Author

scabug commented Aug 27, 2012

Imported From: https://issues.scala-lang.org/browse/SI-6289?orig=1
Reporter: @axel22
Affected Versions: 2.10.0-M6

@scabug
Copy link
Author

scabug commented Jan 4, 2013

@khernyo said:
The problem is that javac writes errors to stderr which is not checked in RunnerManager#runCommandImpl().

I have a fix which redirects stderr to stdout, but the implementation is a hack as I don't see how to do the redirection when using scala.sys.process._. I'm gonna spend some more time on it. (Another solution could be the -Xstdout option of javac, but it's non-standard, so I'd rather avoid it.)

Or maybe stdout and stderr should be handled separately? E.g. stdout is checked against **.check and stderr is checked against **.check-err

@scabug
Copy link
Author

scabug commented Jan 4, 2013

@som-snytt said:
My partest refactor from last summer (which collided with the other partest refactor) used javax.tools to run javac and collect errors, and does fail the example.

I dimly recall that other useful messages include: are all my compile groups free of deprecations, or whatever. I had ambitions for using all the outputs.

This code had a small performance boost, but not a killer feature. I guess this would be a minor feature.

Testing compiler (on files whose compilation should fail)
testing: [...]\files\neg\mybug                                        Error! x has private access in Const
Saving artifacts on status Fail
[FAILED]

@scabug
Copy link
Author

scabug commented Jan 4, 2013

@khernyo said:
I think it would be better to use javax.tools for compilation instead of playing with stdout and stderr.

Would you care to share your implementation' using javax.tools? I could reuse it to fix this bug.

@scabug
Copy link
Author

scabug commented Jan 21, 2013

@Blaisorblade said:
Does the problem disappear when running the whole test suite?
If yes, the problem should be easy to fix and unrelated to using javax.tools.
If not, this bug should be a blocker for merging any other pull request.

@scabug
Copy link
Author

scabug commented Jan 21, 2013

@khernyo said:
No, it does not disappear.

I have a half baked solution but I prioritized the "some stdout output is discarded by partest" issue as that one affect this one too. No ticket for this one, yet.

Fixing it is not hard but it's complicated by the fact that the output from java 6 and java 7 are different and it needs a hack to make them similar (javax.tools doesn't really help here)

I will share my findings and code later today.

@scabug
Copy link
Author

scabug commented Jan 23, 2013

@gkossakowski said:
Changed priority to Major and rescheduled it to 2.10.2-RC1. Actually, I think tickets about infrastructure should not have Fix version set because they are less dependent on release schedule.

@scabug
Copy link
Author

scabug commented Jan 23, 2013

@Blaisorblade said:
Just to ensure everything is clear: part of the test suite is not being effective. ("Does the problem disappear when running the whole test suite?" "No, it does not disappear."). Hence technical debt might be growing right now - meaning the day this is fixed you might discover than N testcases broke. Imagine that happening the day before the release date.

If yours was a conscious decision, Grzegorz, then I respect it, I'm just confused about it.

@scabug
Copy link
Author

scabug commented Jan 23, 2013

@adriaanm said:
I agree with Paolo, new features is one thing, but silent failure in our test infrastructure is quite serious.

@scabug
Copy link
Author

scabug commented Mar 23, 2013

@som-snytt said (edited on Apr 4, 2013 8:32:24 AM UTC):
This PR from paulp's partest-sprint is against master, further tweaks and testing required; I haven't diffed against 2.10.x.

Revised:
scala/scala#2328

Freebased:
scala/scala#2350
with additional nudge to the build by paulp

I just read that the computer keyboard is one of the top ten germiest spots in the house, after the kitchen sponge but before the kitchen sink.

One reason is the people drooling over partest in color:

https://groups.google.com/d/msg/scala-internals/X_O5MLpun5o/Ha_s9Eml_zQJ

@scabug
Copy link
Author

scabug commented Apr 5, 2013

@som-snytt said:
Disinfect your keyboards.

scala/scala#2353

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants