janestreet / ppx_expect

Cram like framework for OCaml
MIT License
146 stars 28 forks source link

Ppx_expect doesn't seem to work on windows #34

Closed hhugo closed 2 years ago

hhugo commented 3 years ago

I'm trying to make expect_test work on windows for js_of_ocaml. Here is what I get in https://github.com/ocsigen/js_of_ocaml/pull/1132

using ppx_expect.v0.14.1, ppx_inline_test.v0.14.1

cat debug-expect/dune

(library
 (name debug_expect)
 (inline_tests
  (modes js native))
 (preprocess (pps ppx_expect)))

cat debug-expect/debug_expect.ml

let%expect_test _ =
  print_endline "hello";
  [%expect {| hello |}]
Run opam exec -- dune runtest debug-expect
  opam exec -- dune runtest debug-expect
  shell: C:\Program Files\PowerShell\7\pwsh.EXE -command ". '{0}'"
  env:
    OPAMCOLOR: always
    OPAMERRLOGLEN: 0
    OPAMJOBS: 2
    OPAMPRECISETRACKING: 1
    OPAMSOLVERTIMEOUT: 1000
    OPAMVERBOSE: false
    OPAMYES: 1
    OPAMROOT: D:\.opam
    HOME: C:\Users\runneradmin
    MSYS: winsymlinks:native
    CYGWIN: winsymlinks:native
    CYGWIN_ROOT: D:\cygwin
    CYGWIN_ROOT_BIN: D:\cygwin\bin
    CYGWIN_ROOT_WRAPPERBIN: D:\cygwin\wrapperbin
    DUNE_CACHE: enabled
    DUNE_CACHE_TRANSPORT: direct
inline_test_runner_debug_expect alias debug-expect/runtest (exit 2)
(cd _build/default/debug-expect && .debug_expect.inline-tests/inline_test_runner_debug_expect.exe inline-test-runner debug_expect -source-tree-root .. -diff-cmd -)
File "debug-expect/debug_expect.ml", line 1, characters 0-70 threw End_of_file.
  Raised at Base__Exn.protectx in file "src/exn.ml", line 71, characters 4-114
  Called from Ppx_inline_test_lib__Runtime.time_and_reset_random_seeds in file "runtime-lib/runtime.ml", line 356, characters 15-52
  Called from Ppx_inline_test_lib__Runtime.test in file "runtime-lib/runtime.ml", line 444, characters 52-83

FAILED 1 / 1 tests
Error: Process completed with exit code 1.
hhugo commented 3 years ago

It works if I don't emit newline in the expect test. This suggest that ppx_expect should call open_out_bin/open_in_bin instead of open_out/open_in

dra27 commented 3 years ago

Indeed - the problem is that by default print_endline also tests the underlying CRLF conversion on Windows! ocamltest contains the most extreme comparison function for this (see Filecompare.compare_text_files in ocaml/ocaml). The central premise is that either all the newlines should be LF or all the newlines should CRLF but a mix (and also double-conversion CR+CRLF) should be considered bugs, so the ocamltest function goes to great lengths to ensure that CRLF is normalised to LF exactly when all the endings are CRLF.

hhugo commented 3 years ago

Digging a bit further on this issue, here is my current understanding. The current implementation collect file offsets using fseek and later use theses offsets to compute what lengths to read with really_input_string. The issue is that reading is in "translated/text" mode and that reading the length between offset doesn't mean reading just the content between offset. @dra27, does that make sense ?

aalekseyev commented 2 years ago

This seems to be fixed by 56cfa602173ff920472a5081e774ca0f84b95eec, maybe (pushed >2 weeks ago).