figures icon indicating copy to clipboard operation
figures copied to clipboard

Fallback symbols are not covered by tests in Unicode-capable environments, including CI

Open gibson042 opened this issue 3 years ago • 3 comments

Observed at https://github.com/sindresorhus/figures/pull/93#discussion_r939671383 , the test for replaceSymbols verifies the absence of replacements (in a Unicode-capable environment) OR replacement with fallback symbols (in other environments), but not both. It would be good to update the tests for more complete coverage, or failing that to at least update CI to include less capable environments.

gibson042 avatar Aug 07 '22 18:08 gibson042

Nice find @gibson042! Would you be interested in submitting a PR?

ehmicky avatar Aug 07 '22 19:08 ehmicky

Yes, but I may need help with this one. Is there a standard approach in these packages to mocking dependencies such that isUnicodeSupported() can be forced to return false inside a test? And if not, then what would be your alternative recommendation?

gibson042 avatar Aug 07 '22 19:08 gibson042

Good point.

Not sure how @sindresorhus feels about this, but I personally try to avoid mocking whenever possible because:

  • It can end up creating complex test setups
  • It makes the tests less similar than the environment the users actually experience, which makes the tests less useful, and can miss some bugs

In this situation, I think what'd be best would be to run the tests in CI in an environment where isUnicodeSupported() returns false. In principle, Windows machines on GitHub actions use ConsoleHost (through cmd.exe) which uses its own charset and has issues displaying many Unicode characters. Unfortunately , isUnicodeSupported() returns true when the CI environment variable is set. What was the reason behind this @sindresorhus?

If changing is-unicode-supported is not an option, I am wondering whether setting CI to an empty string in the GitHub actions workflow file for the Windows environment would work? :thinking:

ehmicky avatar Aug 08 '22 14:08 ehmicky

Unfortunately , isUnicodeSupported() returns true when the CI environment variable is set. What was the reason behind this @sindresorhus?

I don't remember. It's definitely not the correct behavior.

https://github.com/sindresorhus/is-unicode-supported/releases/tag/v2.0.0

sindresorhus avatar Oct 29 '23 14:10 sindresorhus