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 reporting terminal dimensions #381

Merged
merged 5 commits into from
Sep 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,18 @@

- Add `match_raises`, a generalized version of `check_raises`
(#88, #386, @JoanThibault)
- Update JaneStreet core and async to v0.16 (#390 @tmcgilchrist)

- Update JaneStreet core and async to v0.16 (#390 @tmcgilchrist)

- Fix division by zero when size of the terminal is incorrectly
reported as zero. (fix #356, #381, @MisterDA)

- Enable terminal size reporting on macOS and Windows. Also report the
terminal size even when the test is run buffered by Dune.
(#381, @MisterDA)

- Allow overriding the number of columns with `ALCOTEST_COLUMNS` env
var. (#322, #381, @MisterDA)

### 1.7.0 (2023-02-24)

Expand Down
5 changes: 5 additions & 0 deletions alcotest-help.txt
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,11 @@ ENVIRONMENT
ALCOTEST_COLOR
See option --color.

ALCOTEST_COLUMNS
Number of columns after which Alcotest truncates or splits written
lines. Default is to auto-detect using the terminal's dimensions,
or fallback to 80 columns.

ALCOTEST_COMPACT
See option --compact.

Expand Down
16 changes: 15 additions & 1 deletion src/alcotest-engine/cli.ml
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,22 @@ module Make (P : Platform.MAKER) (M : Monad.S) :
in
Cmdliner.Cmd.Env.info "ALCOTEST_SOURCE_CODE_POSITION" ~doc

let alcotest_columns =
let doc =
"Number of columns after which Alcotest truncates or splits written \
lines. Default is to auto-detect using the terminal's dimensions, or \
fallback to 80 columns."
in
Cmdliner.Cmd.Env.info "ALCOTEST_COLUMNS" ~doc

let envs =
[ ci_env; github_action_env; ocamlci_env; alcotest_source_code_position ]
[
ci_env;
github_action_env;
ocamlci_env;
alcotest_source_code_position;
alcotest_columns;
]

let set_color =
let env = Cmd.Env.info "ALCOTEST_COLOR" in
Expand Down
1 change: 1 addition & 0 deletions src/alcotest-stdlib-ext/alcotest_stdlib_ext.ml
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ end
module Option = struct
let is_some = function Some _ -> true | None -> false
let map f = function Some x -> Some (f x) | None -> None
let bind o f = match o with Some o -> f o | None -> None

let get_exn = function
| Some x -> x
Expand Down
1 change: 1 addition & 0 deletions src/alcotest-stdlib-ext/alcotest_stdlib_ext.mli
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ end

module Option : sig
val map : ('a -> 'b) -> 'a option -> 'b option
val bind : 'a option -> ('a -> 'b option) -> 'b option
val is_some : _ option -> bool
val get_exn : 'a option -> 'a
val value : default:'a -> 'a option -> 'a
Expand Down
11 changes: 6 additions & 5 deletions src/alcotest/alcotest.ml
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,12 @@ module Unix_platform (M : Alcotest_engine.Monad.S) = struct
let stdout_isatty () = Unix.(isatty stdout)

let stdout_columns () =
if Sys.win32 then None
else
match Terminal.get_dimensions () with
| Some { columns; _ } -> Some columns
| None -> None
match Option.bind (Sys.getenv_opt "ALCOTEST_COLUMNS") int_of_string_opt with
| Some columns when columns > 0 -> Some columns
| _ -> (
match Terminal.get_dimensions () with
| Some { columns; _ } when columns > 0 -> Some columns
| _ -> None)

external before_test :
output:out_channel -> stdout:out_channel -> stderr:out_channel -> unit
Expand Down
37 changes: 20 additions & 17 deletions src/alcotest/alcotest_stubs.c
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
#include <caml/version.h>

#if OCAML_VERSION < 50000
#define CAML_NAME_SPACE
#endif

#include <caml/alloc.h>
#include <caml/memory.h>
#include <caml/mlvalues.h>
#ifndef _MSC_VER
#include <unistd.h>
#endif

#if OCAML_VERSION < 41200
#define Val_none Val_int(0)
Expand All @@ -19,20 +21,14 @@ static value caml_alloc_some(value v)
}
#endif

// Detect platform
#if defined(_WIN32)
#define OCAML_ALCOTEST_WINDOWS
#elif defined(__unix__) || defined(__unix) || (defined(__APPLE__) && defined(__MACH__))
#if defined(_POSIX_VERSION)
#define OCAML_ALCOTEST_POSIX
#endif
#endif

// Windows support
#if defined(OCAML_ALCOTEST_WINDOWS)
#if defined(_WIN32)
#define WIN32_LEAN_AND_MEAN
#define VC_EXTRALEAN
#include <windows.h>
#if !defined(_MSC_VER)
#include <unistd.h>
#endif

CAMLprim value ocaml_alcotest_get_terminal_dimensions(value unit)
{
Expand All @@ -56,17 +52,24 @@ CAMLprim value ocaml_alcotest_get_terminal_dimensions(value unit)
CAMLreturn(result);
}

// POSIX support
#elif defined(OCAML_ALCOTEST_POSIX)
#elif defined (__unix__) || (defined (__APPLE__) && defined (__MACH__))
#include <unistd.h>
#include <sys/ioctl.h>
#include <fcntl.h>

CAMLprim value ocaml_alcotest_get_terminal_dimensions(value unit)
{
CAMLparam1(unit);
CAMLlocal2(result, pair);
struct winsize ws;
int z = ioctl(STDOUT_FILENO, TIOCGWINSZ, &ws);
if (z == 0)
int fd = open("/dev/tty", O_RDONLY | O_NONBLOCK | O_CLOEXEC);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case someone else is looking at this and doesn't know why STDOUT_FILENO was replaced by "/dev/tty" (like I was), the difference is when the output of alcotest is piped (say through a grep): stdout wouldn't have a size so the formatting would be different (and probably be confusing to users?), while the tty always return the same size (... I don't have strong opinion regarding this change, in general it's weird for piped programs to respect terminal size but it seems fine for alcotest)

if (fd < 0) {
result = Val_none;
CAMLreturn(result);
}
int z = ioctl(fd, TIOCGWINSZ, &ws);
close(fd);
if (z != -1)
{
pair = caml_alloc_tuple(2);
Store_field(pair, 0, Val_int(ws.ws_row));
Expand Down
4 changes: 3 additions & 1 deletion test/e2e/dune
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
; Don't run tests as if Alcotest was run in CI
(CI false)
; Don't guess source code position for compat with < 4.12.
(ALCOTEST_SOURCE_CODE_POSITION false))))
(ALCOTEST_SOURCE_CODE_POSITION false)
; Set to 80 columns for output reproducibility
(ALCOTEST_COLUMNS 80))))

(executable
(name gen_dune_rules)
Expand Down
Loading