-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Restore stdio in test rig #24628
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
base: main
Are you sure you want to change the base?
Restore stdio in test rig #24628
Conversation
There is no more skipping. |
|
Added a bit of output for more stack trace. The test will always fail, to make re-running easier. My first guess was NPE in clinit. My second guess was a bad "nullable field" getting nulled after lazy val init; perhaps the outer pointer itself, which was used only for the lazy init. But the outer is not a class param in the example; its rhs is just a static module. Maybe the trait init? but that is empty. Maybe a class isn't initialized? Maybe a missing inner class, except the module is top level. I could not replicate locally (WSL) but interacting with the test rig made me want to contribute improvements, as I intended. I see that the flaky test is not included in the group with |
|
Woo-hoo! Oh wait, does that make any sense? I'll look at it later. |
d3fae08 to
7d224c9
Compare
|
The test for #2781 was aware that nulling stdio streams is unfriendly, but did it anyway, in the interests of science. Any test thereafter in the same runner JVM that used This doesn't seem to be a rampant problem, but for safety, the Equivalent in https://github.com/scala/scala/blob/v2.13.18/src/partest/scala/tools/partest/nest/Runner.scala#L242 |
| runMain(stdin.readLine()); | ||
| } | ||
| finally { | ||
| if (err != System.err) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why branch here? we can just call System.setErr regardless of if it changed or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to be finicky. I needed a block for a println while verifying, and then I decided to avoid any side-effects, however benign, of calling the setter.
Most of my time was spent formatting the Java code.
Worth adding that this class could be written in Scala.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like I have an opportunity edit it again anyway. I requested your review because of your proximate interest in the build passing correctly. By proximate, I mean near and dear to your heart.
7d224c9 to
47abcd0
Compare
Save and restore stdio in the test rig child process.
Also add a test from the duplicate ticket for 23245.