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

Don't show "in progress" lines when not writing to a TTY #267

Merged
merged 2 commits into from
Aug 26, 2020

Conversation

craigfe
Copy link
Member

@craigfe craigfe commented Aug 17, 2020

The Alcotest output is currently quite hard to read when printed to CI logs. For instance, in OCaml-CI:

#18 3.224   ...           basic               0   Plain.
  [OK]          basic               0   Plain.
#18 3.224   ...           basic               1   Lwt.
  [OK]          basic               1   Lwt.
#18 3.224   ...           exceptions          0   Plain.
  [OK]          exceptions          0   Plain.
#18 3.224   ...           exceptions          1   Lwt toplevel.
  [OK]          exceptions          1   Lwt toplevel.
#18 3.224   ...           exceptions          2   Lwt internal.
  [OK]          exceptions          2   Lwt internal.
#18 3.224   ...           switches            0   Allocate resource.
  [OK]          switches            0   Allocate resource.
#18 3.224   ...           switches            1   Check resource deallocated.
  [OK]          switches            1   Check resource deallocated.

Partly this is because colours have been lost, but mostly it's due to emitting \r characters when not printing to a TTY. This PR disables the "..." lines when isatty returns false.

@craigfe
Copy link
Member Author

craigfe commented Aug 17, 2020

This includes a test that forces the old behaviour (isatty.ml), so we have some coverage over that.

@craigfe
Copy link
Member Author

craigfe commented Aug 17, 2020

Sample of the OCaml-CI output using this PR:

#18 5.880 Testing `LwtUtils'.
#18 5.880 This run has ID `ED20736C-C64E-4538-BA74-74F08EA66D8B'.
#18 5.880 
#18 5.880   [OK]          basic               0   Plain.
#18 5.880   [OK]          basic               1   Lwt.
#18 5.880   [OK]          exceptions          0   Plain.
#18 5.880   [OK]          exceptions          1   Lwt toplevel.
#18 5.880   [OK]          exceptions          2   Lwt internal.
#18 5.880   [OK]          switches            0   Allocate resource.
#18 5.880   [OK]          switches            1   Check resource deallocated.
#18 5.880 
#18 5.880 Full test results in `/src/_build/default/examples/lwt/_build/_tests/LwtUtils'.
#18 5.880 Test Successful in 0.000s. 7 tests run.

Much nicer 🙂

@craigfe craigfe force-pushed the check-if-isatty branch 2 times, most recently from 9d32724 to 23a36a6 Compare August 17, 2020 21:19
@craigfe
Copy link
Member Author

craigfe commented Aug 24, 2020

Will merge at the end of the week, unless there are objections.

@craigfe
Copy link
Member Author

craigfe commented Aug 26, 2020

CI failures unrelated (and known about upstream), so merging.

@craigfe craigfe merged commit 648ab48 into master Aug 26, 2020
craigfe added a commit to craigfe/opam-repository that referenced this pull request Aug 26, 2020
…lwt (1.2.2)

CHANGES:

- Fail gracefully when the user supplies an empty suite name. (mirage/alcotest#265, @craigfe)

- Fix compatibility with `fmt.0.8.8+dune` by adding a missing `fmt` dependency
  in `alcotest`'s dune file (mirage/alcotest#266, @NathanReb)

- Only show "in progress" lines when writing to a TTY. (mirage/alcotest#267, @craigfe)
@MisterDA MisterDA deleted the check-if-isatty branch March 27, 2023 14:01
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.

2 participants