Skip to content

Conversation

@matsduf
Copy link
Contributor

@matsduf matsduf commented Jun 23, 2025

Purpose

This PR fixes the issue reported in #438 and #438 (comment)

How to test this PR

Run the unit tests with other locale than "en" or "C" and the unit tests should pass, e.g.

  • LC_ALL=da_DK.UTF-8 cpanm -v --test-only Zonemaster-CLI-*.tar.gz # Using a dist file
  • LC_ALL=da_DK.UTF-8 cpanm -v --test-only Zonemaster::CLI

@matsduf matsduf added this to the v2025.1 milestone Jun 23, 2025
@matsduf matsduf added the T-Bug Type: Bug in software or error in test case description label Jun 23, 2025
Copy link
Contributor

@marc-vanderwal marc-vanderwal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I was going to look into it but didn’t have the time to do so. Thanks!
Anyway, this looks good to me. That’s exactly the solution I was thinking of too.

Copy link
Contributor

@tgreenx tgreenx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we store the current values of those environment variables first and restore them at the end of the unit test? They may be in use by other programs, right?

@matsduf
Copy link
Contributor Author

matsduf commented Jun 23, 2025

Shouldn't we store the current values of those environment variables first and restore them at the end of the unit test? They may be in use by other programs, right?

My assumption is (was) that each t file is run in its own process. Can a t file change the environment for another t file?

@tgreenx
Copy link
Contributor

tgreenx commented Jun 23, 2025

Shouldn't we store the current values of those environment variables first and restore them at the end of the unit test? They may be in use by other programs, right?

My assumption is (was) that each t file is run in its own process. Can a t file change the environment for another t file?

After testing locally, it looks like it's fine yes.

@tgreenx tgreenx added the V-Patch Versioning: The change gives an update of patch in version. label Jun 23, 2025
@matsduf matsduf merged commit b6b78ed into zonemaster:develop Jun 23, 2025
3 checks passed
@matsduf matsduf deleted the fix-locale-in-unit-test branch June 23, 2025 14:44
@tgreenx tgreenx linked an issue Jul 1, 2025 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-Bug Type: Bug in software or error in test case description V-Patch Versioning: The change gives an update of patch in version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unit tests fail on non-English system

3 participants