search

fsprojects / FSharp.Compatibility

Compatibility libraries for F#
44 stars 22 forks source link

Travis build is failing #1

Closed fsgit closed 9 years ago

fsgit commented 9 years ago

The travis build is failing, can this be fixed? https://travis-ci.org/fsprojects/FSharp.Compatibility

jack-pappas commented 9 years ago

@fsgit Sure, I'll take a look at it.

jack-pappas commented 9 years ago

I fixed the actual compilation errors so the projects build again, and I've also removed the dependency on the FSharp.PowerPack community package (which was causing issues for some people). However, the travis-ci build is still failing because there is one test which isn't working correctly on Mono; I can't tell whether the function itself is broken, the test is broken, or if they're both OK and we're running into a bug in Mono.

jack-pappas commented 9 years ago

I just ran the tests on my local machine, and the failing test (PervasivesTests.IO_EndOfLine_Translations) also fails on Windows + .NET CLR for the same reason it fails on OS X + Mono, so it's narrowed down to either a bug in the function, or a bug in the test.

jmquigs commented 9 years ago

Hello, I randomly selected this project to look into after the call went out on twitter to fix builds for projects listed here: https://github.com/fsprojects/FsProjectsAdmin/blob/master/BuildStatuses.md

So far I have found that the "Sharing Violation" is because the "test.txt" file created by the unit test is still open when the unit test tries to check it. I think this is because OutChannelImpl does not implement IDisposable, so therefore the file is not closed. InChannelImpl has the same problem.

After that is fixed, the next problem is that the test fails when reading back the file, because it reads back zero bytes. This is a similar issue with OutChannelImpl; it inherits TextWriter (which is what the test uses, via fprintf), but it doesn't override any of the Write() functions. Therefore the output is essentially discarded, and the output file remains at zero size.

After fixing that, I can get the test to succeed up to "cewjk5", where it fails again. I haven't looked at that yet and thought it would be a good time to comment here.

In particular I'm confused about the intent of this commented out code: https://github.com/fsprojects/FSharp.Compatibility/blob/master/FSharp.Compatibility.OCaml/Pervasives.fs#L1122

vs the actual implementation: https://github.com/fsprojects/FSharp.Compatibility/blob/master/FSharp.Compatibility.OCaml/Pervasives.fs#L446

The commented out code actually appears to be implementing the required functionality; but if uncommented, it does not compile.

sergey-tihon commented 9 years ago

@jack-pappas Could you comment on proposed fixes? @jmquigs Could you prepare PR with your changes? So, it would be easier to review changes

dsyme commented 9 years ago

Fixed.