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

Do not use symlink when writing and reading log files. #353

Merged
merged 3 commits into from
Jun 13, 2022
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
5 changes: 5 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
### dev

- Fix a bug when running test concurently. Alcotest could fail to
output the content of the log file. (#353, @hhugo)

### 1.5.0 (2021-10-09)

- Make Alcotest compatible with `js_of_ocaml.3.11.0`. Users can depend on the
Expand Down
29 changes: 19 additions & 10 deletions src/alcotest-engine/log_trap.ml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,13 @@ module Make
struct
open Promise.Syntax

type state = { root : string; uuid : string; suite_name : string }
type state = {
root : string;
uuid : string;
suite_name : string;
has_alias : bool;
}

type t = Inactive | Active of state

(** Take a string path and collapse a leading [$HOME] path segment to [~]. *)
Expand All @@ -30,7 +36,8 @@ struct

let active ~root ~uuid ~suite_name =
Platform.prepare_log_trap ~root ~uuid ~name:suite_name;
Active { root; uuid; suite_name }
let has_alias = Platform.file_exists (Filename.concat root suite_name) in
Active { root; uuid; suite_name; has_alias }

let pp_path = Fmt.using maybe_collapse_home Fmt.string

Expand Down Expand Up @@ -63,31 +70,31 @@ struct
in
ListLabels.iter display_lines ~f:(Fmt.pf ppf "%s@\n")

let log_dir { suite_name; uuid; root } =
(* We don't create symlinks on Windows. *)
let via_symlink = not Sys.win32 in
let log_dir ~via_symlink { suite_name; uuid; root; has_alias } =
let via_symlink = via_symlink && has_alias in
Filename.concat root (if via_symlink then suite_name else uuid)

let output_fpath t tname = Filename.concat (log_dir t) (Test_name.file tname)
let output_fpath ~via_symlink t tname =
Filename.concat (log_dir ~via_symlink t) (Test_name.file tname)

let active_or_exn = function
| Active t -> t
| Inactive -> failwith "internal error: no log location"

let pp_current_run_dir t ppf =
let t = active_or_exn t in
pp_path ppf (log_dir t)
pp_path ppf (log_dir ~via_symlink:true t)

let pp_log_location t tname ppf =
let t = active_or_exn t in
let path = output_fpath t tname in
let path = output_fpath ~via_symlink:true t tname in
pp_path ppf path

let recover_logs t ~tail tname =
match t with
| Inactive -> None
| Active t -> (
let fpath = output_fpath t tname in
let fpath = output_fpath ~via_symlink:false t tname in
match Platform.file_exists fpath with
| false -> None
| true -> Some (fun ppf -> pp_tail tail fpath ppf))
Expand All @@ -96,7 +103,9 @@ struct
match t with
| Inactive -> f x
| Active t ->
let fd = Platform.open_write_only (output_fpath t tname) in
let fd =
Platform.open_write_only (output_fpath ~via_symlink:false t tname)
in
let* () = Promise.return () in
let+ a = Platform.with_redirect fd (fun () -> f x) in
Platform.close fd;
Expand Down