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.

Sign up for GitHub

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

Jump to bottom

Test suite is broken when CLICOLOR_FORCE=1 #6607

Open
emillon opened this issue Nov 30, 2022 · 1 comment
Open

Test suite is broken when CLICOLOR_FORCE=1 #6607

emillon opened this issue Nov 30, 2022 · 1 comment
Labels
bug test-suite

Comments

@emillon
Copy link
Collaborator

emillon commented Nov 30, 2022

When this environment variable is set, stderr and stdout are assumed to understand colors and so color codes are used. As a consequence, things like expect tests are outputting different strings depending on the environment variable. We want the results of these tests to be independent from the value of this environment variable.
@emillon
Copy link
Collaborator Author

emillon commented Nov 30, 2022

Within dune itself, the 2 issues are:

  • expect tests inherit that variable and force ANSI color codes. So they are compared to the reference output without color codes and this fails
  • the patdiff arguments depend on that variable

This can be fixed for dune itself but the issue might affect all projects that use dune so we might need to come up with a fix in the expect test subsystem for example.

@emillon emillon mentioned this issue Nov 30, 2022
Merged
emillon added a commit to emillon/dune that referenced this issue Nov 30, 2022
@emillon
Set CLICOLOR_FORCE=0
ba1fddf 
Since ocaml#6340 we're considering `CLICOLOR_FORCE` to determine whether
stderr supports color. However this changes behavior observable from the
test suite, so `CLICOLOR_FORCE=1 make test` fails (this is tracked as
ocaml#6607).

The issue is that ocaml/setup-ocaml#631, this variable is set in the CI
environment. So we disable it until the situation is fixed (and this
variable does not have observable changes anymore).

Signed-off-by: Etienne Millon <[email protected]>
emillon added a commit to emillon/dune that referenced this issue Nov 30, 2022
@emillon
Set CLICOLOR_FORCE=0
4155f39 
Since ocaml#6340 we're considering `CLICOLOR_FORCE` to determine whether
stderr supports color. However this changes behavior observable from the
test suite, so `CLICOLOR_FORCE=1 make test` fails (this is tracked as
ocaml#6607).

The issue is that ocaml/setup-ocaml#631, this variable is set in the CI
environment. So we disable it until the situation is fixed (and this
variable does not have observable changes anymore).

Signed-off-by: Etienne Millon <[email protected]>
emillon added a commit to emillon/dune that referenced this issue Nov 30, 2022
@emillon
Set CLICOLOR_FORCE=0
de58b04 
Since ocaml#6340 we're considering `CLICOLOR_FORCE` to determine whether
stderr supports color. However this changes behavior observable from the
test suite, so `CLICOLOR_FORCE=1 make test` fails (this is tracked as
ocaml#6607).

The issue is that ocaml/setup-ocaml#631, this variable is set in the CI
environment. So we disable it until the situation is fixed (and this
variable does not have observable changes anymore).

Signed-off-by: Etienne Millon <[email protected]>
emillon added a commit to emillon/dune that referenced this issue Nov 30, 2022
@emillon
Set CLICOLOR_FORCE=0
2c0d471 
Since ocaml#6340 we're considering `CLICOLOR_FORCE` to determine whether
stderr supports color. However this changes behavior observable from the
test suite, so `CLICOLOR_FORCE=1 make test` fails (this is tracked as
ocaml#6607).

The issue is that ocaml/setup-ocaml#631, this variable is set in the CI
environment. So we disable it until the situation is fixed (and this
variable does not have observable changes anymore).

Signed-off-by: Etienne Millon <[email protected]>
emillon added a commit to emillon/dune that referenced this issue Nov 30, 2022
@emillon
Set CLICOLOR_FORCE=0
782a7b6 
Since ocaml#6340 we're considering `CLICOLOR_FORCE` to determine whether
stderr supports color. However this changes behavior observable from the
test suite, so `CLICOLOR_FORCE=1 make test` fails (this is tracked as
ocaml#6607).

The issue is that ocaml/setup-ocaml#631, this variable is set in the CI
environment. So we disable it until the situation is fixed (and this
variable does not have observable changes anymore).

Signed-off-by: Etienne Millon <[email protected]>
emillon added a commit that referenced this issue Nov 30, 2022
@emillon
Set CLICOLOR_FORCE=0 (#6608)
a093bd0 
* Set CLICOLOR_FORCE=0

Since #6340 we're considering `CLICOLOR_FORCE` to determine whether
stderr supports color. However this changes behavior observable from the
test suite, so `CLICOLOR_FORCE=1 make test` fails (this is tracked as
#6607).

The issue is that ocaml/setup-ocaml#631, this variable is set in the CI
environment. So we disable it until the situation is fixed (and this
variable does not have observable changes anymore).

Signed-off-by: Etienne Millon <[email protected]>
emillon added a commit to emillon/dune that referenced this issue Dec 1, 2022
@emillon
test: add a repro case for ocaml#6607
71a4494 
Signed-off-by: Etienne Millon <[email protected]>
emillon added a commit to emillon/dune that referenced this issue Dec 1, 2022
@emillon
test: add a repro case for ocaml#6607
c8c387b 
Signed-off-by: Etienne Millon <[email protected]>
emillon added a commit to emillon/dune that referenced this issue Dec 1, 2022
@emillon
test: add a repro case for ocaml#6607
2956975 
Signed-off-by: Etienne Millon <[email protected]>
emillon added a commit to emillon/dune that referenced this issue Dec 2, 2022
@emillon
test: add a repro case for ocaml#6607
c170506 
Signed-off-by: Etienne Millon <[email protected]>
emillon added a commit that referenced this issue Dec 2, 2022
@emillon
test: add a repro case for #6607 (#6612)
f244a6e 
Signed-off-by: Etienne Millon <[email protected]>

Signed-off-by: Etienne Millon <[email protected]>
moyodiallo pushed a commit to moyodiallo/dune that referenced this issue Dec 2, 2022
@emillon @moyodiallo
Set CLICOLOR_FORCE=0 (ocaml#6608)
118fad4 
* Set CLICOLOR_FORCE=0

Since ocaml#6340 we're considering `CLICOLOR_FORCE` to determine whether
stderr supports color. However this changes behavior observable from the
test suite, so `CLICOLOR_FORCE=1 make test` fails (this is tracked as
ocaml#6607).

The issue is that ocaml/setup-ocaml#631, this variable is set in the CI
environment. So we disable it until the situation is fixed (and this
variable does not have observable changes anymore).

Signed-off-by: Etienne Millon <[email protected]>
moyodiallo pushed a commit to moyodiallo/dune that referenced this issue Dec 2, 2022
@emillon @moyodiallo
test: add a repro case for ocaml#6607 (ocaml#6612)
417a450 
Signed-off-by: Etienne Millon <[email protected]>

Signed-off-by: Etienne Millon <[email protected]>
jchavarri added a commit to jchavarri/dune that referenced this issue Dec 5, 2022
@jchavarri
Merge branch 'main' into melange/non-melange-lib-error
81c1aed 
* main: (54 commits)
  doc: how we write `to_dyn` and `equal` (ocaml#6621)
  test(cache): test output of man pages
  test: dune utop for (subdir ..) (ocaml#6629)
  refactor: improve style of utop rules (ocaml#6628)
  test: depend on utop (ocaml#6627)
  refactor(stdune): Add Appendable_list.cons (ocaml#6624)
  doc: tighten wording in README.md
  test: add a repro case for ocaml#6607 (ocaml#6612)
  doc: cleanup status badges in README.md (ocaml#6618)
  doc(README): rewrap paragraphs and cleanup links
  coq: more resilient config query
  fix: link time code gen (ocaml#6606)
  fix(melange): run melc ppx with merlin (ocaml#6476)
  feature(melange): add compile_flags (ocaml#6569)
  refactor: move `modules: Modules.t` from `Dune_package.Lib` to `Lib_info` (ocaml#6605)
  Set CLICOLOR_FORCE=0 (ocaml#6608)
  fix: declare deps for ccomp detection (ocaml#6610)
  refactor: assume Cmdliner.Arg.conv is abstract (ocaml#6609)
  refactor: gen_rules pattern matching (ocaml#6604)
  Add CI for MSVC using dkml-workflows (ocaml#6540)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug test-suite
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@emillon @Alizter