test: ensure backward compatibility in test assertion - #13783
Merged
Conversation
…leString` assertion The job `Node.js v12` on workflow `linux_test` failed on develop with a mismatched `toLocaleString` assertion in `test.to_locale_string.js`. Root cause: the test hardcoded a literal expected string assuming de-DE locale formatting is always available, but Node.js versions before v13 ship "small-icu" builds by default, which only include full locale data for "en"; other locales such as de-DE silently fall back to default formatting. This commit computes the expected value dynamically via `.toLocaleString( 'de-DE' )`, matching the pattern already used by the sibling "default locale" and "locale and options" tests in the same file, so the assertion no longer depends on which ICU data is compiled into the runtime. Ref: https://github.com/stdlib-js/stdlib/actions/runs/30532379700
Contributor
Coverage Report
The above coverage report was generated for the changes in this PR. |
Signed-off-by: Athan <kgryte@gmail.com>
kgryte
marked this pull request as ready for review
July 30, 2026 22:23
kgryte
approved these changes
Jul 30, 2026
kgryte
added
Tests
kgryte
changed the title
toLocaleString assertion
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This pull request:
test.to_locale_string.jsfor@stdlib/dstructs/named-typed-tuplethat fails on Node.js versions shipping "small-icu" builds (e.g., Node.js v12 onlinux_test). Failing run: https://github.com/stdlib-js/stdlib/actions/runs/30532379700. The test hardcoded'tuple(price=123.456,789, quantity=9.876)'assumingde-DElocale data is always present; small-icu builds only carryenlocale data and silently fall back to default formatting, so the assertion never matched on those runners. The fix computesexpecteddynamically via.toLocaleString( 'de-DE' ), matching the pattern already used by the two sibling tests in the same file.Related Issues
This pull request has no related issues.
Questions
No.
Other
Verified
(123456.789).toLocaleString('de-DE')produces'123.456,789'on full-icu Node.js (confirmed the fix is a no-op on full-icu runners; the assertion still catches a locale-propagation regression there). Ran the fulltest.to_locale_string.jssuite locally (17/17 passing). Reviewed by three independent passes (correctness, regression scope, style/conventions); all approved with no blocking findings.Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
This PR was written primarily by Claude Code, run as an automated CI-failure triage routine. The root cause was diagnosed from CI logs and confirmed by direct local reproduction; the fix was reviewed by three independent automated review passes (correctness, regression scope, style) before this PR was opened.
@stdlib-js/reviewers
Generated by Claude Code