-
Notifications
You must be signed in to change notification settings - Fork 80
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
Use coloured output by default when running INSIDE_DUNE #242
Conversation
54c4906
to
f86d621
Compare
The output in ocaml-ci is a bit hard to read, but I guess this was the error:
(https://ci.ocamllabs.io:8100/job/2020-05-17/173931-ci-build-d08ed7) I rebuilt the jobs and they passed the second time :-/ |
The errors on
So, the escape codes look to be the same modulo differences in ANSI escape resetting that have no bearing on the rendered output. Sure enough,
@talex5: No idea what's causing the flaky failures post 4.04.0 though; ran the tests 100 times or so on my machine and got only passes. OCaml-CI is caching a bit too aggressively for me to prod it further: shouldn't clicking |
Good idea - I've filed an issue here: ocurrent/ocaml-ci#186 |
I would suggest just dropping this test, but I have future improvements in mind that require heavier usage of the I think the options here are:
|
It's fine to vendor |
I think I've tracked down all of the non-deterministic failures now. (At least, I've killed all the ones that I can reproduce on my machine...) Our handling of symlinks in Our use of Cmdliner throws away helpful information that we should probably be logging somewhere (an |
This drops support for OCaml 4.03 and 4.04. See discussion in mirage#242 (comment).
f86d621
to
6fe6420
Compare
Now depends on #245. |
This drops support for OCaml 4.03 and 4.04. See discussion in mirage#242 (comment).
6fe6420
to
500badf
Compare
This drops support for OCaml 4.03 and 4.04. See discussion in mirage#242 (comment).
500badf
to
9b7eac9
Compare
This drops support for OCaml 4.03 and 4.04. See discussion in mirage#242 (comment).
9b7eac9
to
ede1f9e
Compare
This can now be considered for merge. Vendoring is slightly non-trivial as we export the |
I'm a bit sceptical of this change: it hardcodes behaviour of the output depending on other environment variables ( At least, the behaviour of coloured output should be documented in the documentation (and how it behaves depending on the environment variables and command-line arguments, esp. what the order of precedence is). |
"hardcodes" seems a bit harsh to me. It changes the default behaviour, but never overrides behaviour requested explicitly by the user. This can be done in 4 different ways:
The man-page output is now:
This to me suggests the priority ordering If by 'documentation' you mean the Odoc output, our explanation of the command-line arguments (and their run-time equivalents) is currently very lacking and could certainly do with some general improvement. (The priority ordering is also fairly untested.) |
oh sorry I missed the man page update, thanks for doing that. |
This drops support for OCaml 4.03 and 4.04. See discussion in mirage#242 (comment).
ede1f9e
to
e6c383d
Compare
This changes the default value of the `--color` flag to `always` rather than `auto` when the `INSIDE_DUNE` variable is set. As discussed before (see mirage#207 and ocaml/dune#145), Alcotest output is not coloured by default if run via `dune runtest`, since Dune's buffering mechanism keeps the test process from having direct access to the terminal. We now have the `ALCOTEST_COLOR` variable for experienced users to override this behaviour, but I suspect the better solution is to default to _always_ showing colours, so that new users are not surprised by this interaction. People who really don't want colours can still disable it with the environment variable or the command-line option. Travis CI appears to filter out any ANSII colour codes in its logs (see https://travis-ci.org/github/mirage/irmin/jobs/686951739#L3684).
This drops support for OCaml 4.03 and 4.04. See discussion in mirage#242 (comment).
e6c383d
to
c3d096b
Compare
Rebased. Will merge this tomorrow if there are no outstanding comments / objections. |
…age (1.2.0) CHANGES: - Add an `alcotest-mirage` package, allowing the construction of MirageOS unikernels that run Alcotest test suites. (mirage/alcotest#238, @hannesm @linse) - Add `Alcotest.check'`, a variant of `Alcotest.check` with labeled arguments. (mirage/alcotest#239, @hartmut27) - Add a testable for the `bytes` type. (mirage/alcotest#253, @mefyl) - Many assorted improvements to Alcotest output formatting. (mirage/alcotest#246, @craigfe) - Default to `--color=always` when running inside Dune (mirage/alcotest#242, @craigfe). The value can be overridden by setting the `ALCOTEST_COLOR` variable in a `dune` file, for example: ```dune (env (_ (env-vars (ALCOTEST_COLOR auto)))) ``` - Support all UTF-8 characters in test names and suite names, by normalising them for file-system interactions. (mirage/alcotest#249, @gs0510; mirage/alcotest#246, @craigfe) - Fix various crashes when using non-filesystem-safe characters in test suite names (these break Alcotest when attempting to generate a corresponding log file). (mirage/alcotest#241, @mefyl; mirage/alcotest#246 @craigfe)
This changes the default value of the
--color
flag toalways
rather thanauto
when theINSIDE_DUNE
variable is set.As discussed before (see #207 and ocaml/dune#145), Alcotest output is not coloured by default if run via
dune runtest
, since Dune's buffering mechanism keeps the test process from having direct access to the terminal.We now have the
ALCOTEST_COLOR
variable for experienced users to override this behaviour, but I suspect the better solution is to default to always showing colours, so that new users are not surprised by this interaction. People who really don't want colours can still disable it with the environment variable or the command-line option.Travis CI appears to filter out any ANSII colour codes in its logs (see https://travis-ci.org/github/mirage/irmin/jobs/686951739#L3684).