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

Fix handling of ~and_exit parameter when running via CLI functor #271

Merged
merged 3 commits into from
Sep 7, 2020

Conversation

craigfe
Copy link
Member

@craigfe craigfe commented Aug 26, 2020

Fix #270.

The logic that processes Cmdliner results was incorrect (again), resulting in failure to respect ~and_exit:false when an exception is raised by the user code. In future, we should consolidate this parameter handling logic in one place (i.e. in Core when necessary) to avoid such bugs.

Also cleans up the bad.ml example slightly, which was a bit hard to read before.

@samoht
Copy link
Member

samoht commented Aug 28, 2020

Do you know why do we need ppx_string? In any case, it seems there is a broken constraint.

#16 741.1 #=== ERROR while compiling ppx_string.v0.14.0 =================================#
#16 741.1 # context              2.0.7 | linux/x86_64 | ocaml-base-compiler.4.10.1 | file:///home/opam/opam-repository
#16 741.1 # path                 ~/.opam/4.10/.opam-switch/build/ppx_string.v0.14.0
#16 741.1 # command              ~/.opam/4.10/bin/dune build -p ppx_string -j 31
#16 741.1 # exit-code            1
#16 741.1 # env-file             ~/.opam/log/ppx_string-1-6dcac4.env
#16 741.1 # output-file          ~/.opam/log/ppx_string-1-6dcac4.out
#16 741.1 ### output ###
#16 741.1 #       ocamlc src/.ppx_string.objs/byte/ppx_string.{cmo,cmt} (exit 2)
#16 741.1 # (cd _build/default && /home/opam/.opam/4.10/bin/ocamlc.opt -w -40 -g -bin-annot -I src/.ppx_string.objs/byte -I /home/opam/.opam/4.10/lib/base -I /home/opam/.opam/4.10/lib/base/base_internalhash_types -I /home/opam/.opam/4.10/lib/base/caml -I /home/opam/.opam/4.10/lib/base/shadow_stdlib -I /home/opam/.opam/4.10/lib/ocaml-compiler-libs/common -I /home/opam/.opam/4.10/lib/ocaml-compiler-libs/shadow -I /home/opam/.opam/4.10/lib/ocaml-migrate-parsetree -I /home/opam/.opam/4.10/lib/ocaml/compiler-libs -I /home/opam/.opam/4.10/lib/ppx_compare/runtime-lib -I /home/opam/.opam/4.10/lib/ppx_derivers -I /home/opam/.opam/4.10/lib/ppx_enumerate/runtime-lib -I /home/opam/.opam/4.10/lib/ppx_hash/runtime-lib -I /home/opam/.opam/4.10/lib/ppx_sexp_conv/runtime-lib -I /home/opam/.opam/4.10/lib/ppxlib -I /home/opam/.opam/4.10/lib/ppxlib/ast -I /home/opam/.opam/4.10/lib/ppxlib/print_diff -I /home/opam/.opam/4.10/lib/ppxlib/stdppx -I /home/opam/.opam/4.10/lib/ppxlib/traverse_builtins -I /home/opam/.opam/4.10/lib/sexplib0 -I /home/opam/.opam/4.10/lib/stdlib-shims -intf-suffix .ml -no-alias-deps -o src/.ppx_string.objs/byte/ppx_string.cmo -c -impl src/ppx_string.pp.ml)
#16 741.1 # File "src/ppx_string.ml", line 98, characters 6-32:
#16 741.1 # 98 |       Stdio.In_channel.with_file loc.loc_start.pos_fname ~f:(fun ic ->
#16 741.1 #            ^^^^^^^^^^^^^^^^^^^^^^^^^^
#16 741.1 # Error: Unbound module Stdio
#16 741.1 # Hint: Did you mean Stdlib?
#16 741.1 #     ocamlopt src/.ppx_string.objs/native/ppx_string.{cmx,o} (exit 2)
#16 741.1 # (cd _build/default && /home/opam/.opam/4.10/bin/ocamlopt.opt -w -40 -g -I src/.ppx_string.objs/byte -I src/.ppx_string.objs/native -I /home/opam/.opam/4.10/lib/base -I /home/opam/.opam/4.10/lib/base/base_internalhash_types -I /home/opam/.opam/4.10/lib/base/caml -I /home/opam/.opam/4.10/lib/base/shadow_stdlib -I /home/opam/.opam/4.10/lib/ocaml-compiler-libs/common -I /home/opam/.opam/4.10/lib/ocaml-compiler-libs/shadow -I /home/opam/.opam/4.10/lib/ocaml-migrate-parsetree -I /home/opam/.opam/4.10/lib/ocaml/compiler-libs -I /home/opam/.opam/4.10/lib/ppx_compare/runtime-lib -I /home/opam/.opam/4.10/lib/ppx_derivers -I /home/opam/.opam/4.10/lib/ppx_enumerate/runtime-lib -I /home/opam/.opam/4.10/lib/ppx_hash/runtime-lib -I /home/opam/.opam/4.10/lib/ppx_sexp_conv/runtime-lib -I /home/opam/.opam/4.10/lib/ppxlib -I /home/opam/.opam/4.10/lib/ppxlib/ast -I /home/opam/.opam/4.10/lib/ppxlib/print_diff -I /home/opam/.opam/4.10/lib/ppxlib/stdppx -I /home/opam/.opam/4.10/lib/ppxlib/traverse_builtins -I /home/opam/.opam/4.10/lib/sexplib0 -I /home/opam/.opam/4.10/lib/stdlib-shims -intf-suffix .ml -no-alias-deps -o src/.ppx_string.objs/native/ppx_string.cmx -c -impl src/ppx_string.pp.ml)
#16 741.1 # File "src/ppx_string.ml", line 98, characters 6-32:
#16 741.1 # 98 |       Stdio.In_channel.with_file loc.loc_start.pos_fname ~f:(fun ic ->

@craigfe
Copy link
Member Author

craigfe commented Aug 28, 2020

Do you know why do we need ppx_string?

Yes: alcotest-asynccore_kernelppx_janeppx_string

Hopefully it'll be patched soon. Until then, we'll have to use Travis.

@craigfe craigfe force-pushed the fix-and-escape-with-cli branch from 7853d00 to 2651405 Compare September 3, 2020 12:35
@craigfe
Copy link
Member Author

craigfe commented Sep 3, 2020

Now rebased over #274.

@craigfe craigfe force-pushed the fix-and-escape-with-cli branch 2 times, most recently from f607349 to 5eedbfd Compare September 4, 2020 17:15
@craigfe craigfe force-pushed the fix-and-escape-with-cli branch from 5eedbfd to c4c8ce2 Compare September 7, 2020 16:51
@craigfe craigfe force-pushed the fix-and-escape-with-cli branch from c4c8ce2 to 78f7907 Compare September 7, 2020 18:10
@craigfe craigfe merged commit 7435438 into mirage:master Sep 7, 2020
craigfe added a commit to craigfe/opam-repository that referenced this pull request Sep 7, 2020
…lwt (1.2.3)

CHANGES:

- Require Dune 2.2. (mirage/alcotest#274, @craigfe)

- Fix a bug in the handling of the `~and_exit:false` option when the test suite
  fails. (mirage/alcotest#271, @craigfe)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Uncaught exception Test_error when and_exit flag is set to false
2 participants