get test suite passing on Windows #4770
Conversation
this is mostly intended for .scala files, since because """ preserves line endings, so we get different results if source files have CRLF. this was making a few tests fail on Windows (because they used """ to enclose expected output)
usually it hardly matters, but it's still a bug, and on Windows we can't delete an open file, so this can cause trouble for someone writing a test that relies on being able to generate icode files and then clean them up afterwards. (and in fact, two IcodeComparison-based tests were failing.)
includes comment with full details
this was causing a mysterious compilation failure on Windows. (it may not have been a sufficient cause in itself -- which is why I say "mysterious" -- but in any case, adding the newline made the failure go away. and besides, the newline should be there. so here it is.) (it's tempting to make a big commit that fixes this in every source file. resisting for now)
|
oh, I should have mentioned, if anyone is tempted to actually run this on Windows themselves, I wouldn't currently suggest that you bother. from Cygwin (we're not worrying about from cmd.exe anymore) some parts of the ant build require |
| // between calls). So we are careful to use `slurp` which does call `close`, and careful to | ||
| // check that `delete` returns true indicating successful deletion. | ||
| try icodeFiles sortBy (_.name) flatMap (f => f.slurp().lines.toList) | ||
| finally icodeFiles foreach (f => require(f.delete())) |
There was a problem hiding this comment.
As a matter of fact, I recall that the fix to slurp was because I ran tests on windows and hit the same issue.
https://github.com/scala/scala/blob/v2.11.7/src/reflect/scala/reflect/io/Streamable.scala#L112
Li Haoyi just complained about the behavior of the other methods (he was using Source I think) and I checked that indeed there are leakages, which is too bad.
OTOH, at the Java office, I just had to verify that when using try-with-res, you have to close wrapped resources or be very sure about the behavior. End of this article points out that not all close methods guarantee to close decorated resources.
http://www.oracle.com/technetwork/articles/java/trywithresources-401775.html
So basically, Damn you all to hell! [PAN to reveal STATUE OF LIBERTY half-buried in the sand.]
There was a problem hiding this comment.
require(icodeFiles forall (_.delete)). Or try ff flatMap (f => f.slurp.lines.toList.ensuring(f.delete)) except exceptions. Could use a macro enforcing that wrapped (some) tree in the try.
|
LGTM |
get test suite passing on Windows
this was manually tested (by ssh'ing to a Cygwin prompt on
jenkins-worker-windows-publish), since
scala/scala-jenkins-infra#36 isn't done yet
review by @adriaanm, @som-snytt, @retronym, @martijnhoekstra