Skip to content

Comments

Fix tests with NO_COLOR=1 set#1161

Merged
zwpaper merged 2 commits intolsd-rs:mainfrom
mgorny:no-color-tests
Oct 24, 2025
Merged

Fix tests with NO_COLOR=1 set#1161
zwpaper merged 2 commits intolsd-rs:mainfrom
mgorny:no-color-tests

Conversation

@mgorny
Copy link
Contributor

@mgorny mgorny commented Aug 31, 2025

Fix tests to force internal coloring independently of NO_COLOR in the calling environment, to ensure that they pass. This is needed when NO_COLOR is set globally in the environment, e.g. when collecting plain text test logs.

Technically, crossterm::style::force_color_output() leaks to other tests. However, there should be no harm in setting it globally, and the library does not seem to provide a trivial way to scope the change. Just call it in both places where it is actually necessary, to ensure that things work correctly also when only parts of the test suite are run.

Fixes #1136


TODO

  • Use cargo fmt
  • Add necessary tests
  • Update default config/theme in README (if applicable)
  • Update man page at lsd/doc/lsd.md (if applicable)

Fix tests to force internal coloring independently of `NO_COLOR`
in the calling environment, to ensure that they pass.  This is needed
when `NO_COLOR` is set globally in the environment, e.g. when collecting
plain text test logs.

Technically, `crossterm::style::force_color_output()` leaks to other
tests.  However, there should be no harm in setting it globally,
and the library does not seem to provide a trivial way to scope
the change.  Just call it in both places where it is actually necessary,
to ensure that things work correctly also when only parts of the test
suite are run.

Fixes lsd-rs#1136

Signed-off-by: Michał Górny <mgorny@gentoo.org>
@mgorny mgorny requested a review from zwpaper as a code owner August 31, 2025 16:08
@zwpaper zwpaper merged commit 818257e into lsd-rs:main Oct 24, 2025
19 checks passed
@mgorny
Copy link
Contributor Author

mgorny commented Oct 24, 2025

Thank you!

@mgorny mgorny deleted the no-color-tests branch October 24, 2025 17:31
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.

[Bug]: Tests fail if NO_COLOR=1 is set in containing environment

2 participants