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.

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

Adapt tests for internationalization #11628

Merged
merged 1 commit into from Jan 24, 2024

Conversation

Hofer-Julian
Copy link
Contributor

@Hofer-Julian Hofer-Julian commented Jan 24, 2024

I've noticed that two tests fail on my system when running toolkit check pr. The reason for this is that my locale is set to German. ls is translated, so checking the error message will only work on systems set to English.

I've adapted the test to check the exit code instead.

Alternatively, we could set the locale, I am not sure if the nu! macro supports that though.

@fdncred
Copy link
Collaborator

fdncred commented Jan 24, 2024

There is a locale override

$env.NU_TEST_LOCALE_OVERRIDE = 'en_US.utf8';

It also looks like some tests support locale

https://github.com/nushell/nushell/blob/ff5815c0a305477ec37cb738938430a17a408782/crates/nu-command/tests/commands/date/format.rs#L22-L83

#[test]
fn int_into_string_decimals_0() {
let actual = nu!(
locale: "en_US.UTF-8",
pipeline(
r#"
10 | into string --decimals 0
"#
)
);
assert_eq!(actual.out, "10");
}
#[test]
fn int_into_string_decimals_1() {
let actual = nu!(
locale: "en_US.UTF-8",
pipeline(
r#"
10 | into string --decimals 1
"#
)
);
assert_eq!(actual.out, "10.0");
}
#[test]
fn int_into_string_decimals_10() {
let actual = nu!(
locale: "en_US.UTF-8",
pipeline(
r#"
10 | into string --decimals 10
"#
)
);
assert_eq!(actual.out, "10.0000000000");
}

and a function with_locale_override in our nu-test-support crate
https://github.com/nushell/nushell/blob/ff5815c0a305477ec37cb738938430a17a408782/crates/nu-test-support/src/locale_override.rs#L9-L40

@Hofer-Julian
Copy link
Contributor Author

@fdncred As far as I can tell, neither of them influence the output of an external command like ^ls

I also tried adding

$env.LANG = 'en_US.UTF-8'
$env.LANGUAGE = 'en'

But it still doesn't seem to stop ls from outputting German

@Hofer-Julian
Copy link
Contributor Author

But it still doesn't seem to stop ls from outputting German

Correction, that does work. I just had to import the adapted version of toolkit.nu.

I adapted the PR accordingly.

@fdncred
Copy link
Collaborator

fdncred commented Jan 24, 2024

Right. I wasn't saying ls supports it. I was saying that people have implemented some locale things in testing. So, if we want to do that with ls, someone would need to make ls aware of these locale settings. I believe ls is aware of locale as it understands decimals and commas for numbers but that may be handled somewhere closer to table rendering.

@Hofer-Julian
Copy link
Contributor Author

Hofer-Julian commented Jan 24, 2024

I believe ls is aware of locale as it understands decimals and commas for numbers but that may be handled somewhere closer to table rendering

Indeed. External ls (not nushell's ls) reads env vars which I've adapted in the updated version of this PR.

@fdncred fdncred merged commit 323207c into nushell:main Jan 24, 2024
19 checks passed
@fdncred
Copy link
Collaborator

fdncred commented Jan 24, 2024

thanks

@hustcer hustcer added this to the v0.90.0 milestone Feb 3, 2024
dmatos2012 pushed a commit to dmatos2012/nushell that referenced this pull request Feb 20, 2024
I've noticed that two tests fail on my system when running `toolkit
check pr`. The reason for this is that my locale is set to German. `ls`
is translated, so checking the error message will only work on systems
set to English.

I've adapted the test to check the exit code instead.

Alternatively, we could set the locale, I am not sure if the `nu!` macro
supports that though.
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.

None yet

3 participants