[PR #868] [MERGED] Make Test Report locale independent #779

New Issue

adam · 2025-12-30T01:26:40+01:00

adam commented

2025-12-30 01:26:40 +01:00

📋 Pull Request Information

Original PR: https://github.com/apple/pkl/pull/868
Author: @StefMa
Created: 12/24/2024
Status: ✅ Merged
Merged: 1/2/2025
Merged by: @bioball

Base: main ← Head: fix-test-report

📝 Commits (6)

cabd005 Make SimpleReport locale independent
2f61b5d Test for locale independence
b652fc8 Merge branch 'apple:main' into fix-test-report
33e756f Add missing import
f94d715 Fix formatting
77f1571 Wrap test into try/finally

📊 Changes

2 files changed (+50 additions, -1 deletions)

View changed files

📝 pkl-cli/src/test/kotlin/org/pkl/cli/CliTestRunnerTest.kt (+45 -0)
📝 pkl-core/src/main/java/org/pkl/core/stdlib/test/report/SimpleReport.java (+5 -1)

📄 Description

fixes #861

According to AI (and javadoc 😅) the problem is that String.format takes Locale.getDefault if not specified. Indeed I have set de-EN on my machine so it takes comma decimals instead of dot decimals.

~~I haven't tested this yet, just created it on mobile...~~

This is my macobok setting (not sure if relevant or the correct screen):

If I run the newly added test without Locale.ROOT in SimpleReport, I get the following error:

Running with that Locale.ROOT:
BUILD SUCCESSFUL in 37s 🎉

AI summary

This pull request includes changes to improve locale independence in the CliTestRunner and to ensure consistent locale usage in the SimpleReport class. The most important changes include adding a locale independence test for CliTestRunner and specifying the root locale for formatting in SimpleReport.

Locale independence improvements:

pkl-cli/src/test/kotlin/org/pkl/cli/CliTestRunnerTest.kt: Added a new test CliTestRunner locale independence test to verify that CliTestRunner works correctly regardless of the default locale.
pkl-core/src/main/java/org/pkl/core/stdlib/test/report/SimpleReport.java: Imported java.util.Locale to use for specifying locale in formatting.
pkl-core/src/main/java/org/pkl/core/stdlib/test/report/SimpleReport.java: Modified the makeStatsLine method to use Locale.ROOT in String.format to ensure consistent formatting regardless of the default locale.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/apple/pkl/pull/868 **Author:** [@StefMa](https://github.com/StefMa) **Created:** 12/24/2024 **Status:** ✅ Merged **Merged:** 1/2/2025 **Merged by:** [@bioball](https://github.com/bioball) **Base:** `main` ← **Head:** `fix-test-report` --- ### 📝 Commits (6) - [`cabd005`](https://github.com/apple/pkl/commit/cabd0054e0ca4af7410bc2becdca645f8d9301db) Make SimpleReport locale independent - [`2f61b5d`](https://github.com/apple/pkl/commit/2f61b5d20e52bf833578f1be83fd6bb98b3d193f) Test for locale independence - [`b652fc8`](https://github.com/apple/pkl/commit/b652fc8d4267e25dabc7112e0ddb707ec38977c6) Merge branch 'apple:main' into fix-test-report - [`33e756f`](https://github.com/apple/pkl/commit/33e756ff60b1fd64c892850cf5d4945b420eb83e) Add missing import - [`f94d715`](https://github.com/apple/pkl/commit/f94d715af574fb3cac4a4922ca0739ffd5414563) Fix formatting - [`77f1571`](https://github.com/apple/pkl/commit/77f157109036d2bb01c03d791996ed765f31fe88) Wrap test into try/finally ### 📊 Changes **2 files changed** (+50 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `pkl-cli/src/test/kotlin/org/pkl/cli/CliTestRunnerTest.kt` (+45 -0) 📝 `pkl-core/src/main/java/org/pkl/core/stdlib/test/report/SimpleReport.java` (+5 -1) </details> ### 📄 Description fixes #861 According to AI (and javadoc 😅) the problem is that String.format takes Locale.getDefault if not specified. Indeed I have set de-EN on my machine so it takes comma decimals instead of dot decimals. ~I haven't tested this yet, just created it on mobile...~ This is my macobok setting (not sure if relevant or the correct screen): ![Screenshot 2024-12-24 at 7 05 15 AM](https://github.com/user-attachments/assets/2e84ef16-63e6-4633-a264-d8af7a9a1fb4) If I run the newly added test without `Locale.ROOT` in `SimpleReport`, I get the following error: ![Screenshot 2024-12-24 at 7 06 36 AM](https://github.com/user-attachments/assets/651932b6-362f-4a9f-862b-9d6c123d5fbc) Running with that `Locale.ROOT`: `BUILD SUCCESSFUL in 37s` 🎉 --- AI summary This pull request includes changes to improve locale independence in the `CliTestRunner` and to ensure consistent locale usage in the `SimpleReport` class. The most important changes include adding a locale independence test for `CliTestRunner` and specifying the root locale for formatting in `SimpleReport`. Locale independence improvements: * [`pkl-cli/src/test/kotlin/org/pkl/cli/CliTestRunnerTest.kt`](diffhunk://#diff-37144b29baadf10353a60ee1a542ada8b1988d561989383347319eb2fbce7c06R490-R530): Added a new test `CliTestRunner locale independence test` to verify that `CliTestRunner` works correctly regardless of the default locale. * [`pkl-core/src/main/java/org/pkl/core/stdlib/test/report/SimpleReport.java`](diffhunk://#diff-66bf95f097f7f82c74ddfb08bd576eca3353c82e16a1cd1fa905562152f92c1eR20): Imported `java.util.Locale` to use for specifying locale in formatting. * [`pkl-core/src/main/java/org/pkl/core/stdlib/test/report/SimpleReport.java`](diffhunk://#diff-66bf95f097f7f82c74ddfb08bd576eca3353c82e16a1cd1fa905562152f92c1eL147-R148): Modified the `makeStatsLine` method to use `Locale.ROOT` in `String.format` to ensure consistent formatting regardless of the default locale. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

adam added the pull-request label 2025-12-30 01:26:40 +01:00

adam closed this issue

2025-12-30 01:26:40 +01:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/pkl#779