FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Use culture-dependent string for decimals in RuntimeMetadataTests by bamarsha · Pull Request #339 · microsoft/qsharp-runtime · GitHub

This repository was archived by the owner on Jan 12, 2024. It is now read-only.
/ qsharp-runtime Public archive

Use culture-dependent string for decimals in RuntimeMetadataTests - #339

Merged
Brianna Marshall (bamarsha) merged 2 commits into
masterfrom
samarsha/decimal-culture
Aug 18, 2020
Merged

Use culture-dependent string for decimals in RuntimeMetadataTests#339
Brianna Marshall (bamarsha) merged 2 commits into
masterfrom
samarsha/decimal-culture

Conversation

Copy link
Copy Markdown
Contributor

GetRuntimeMetadata returns strings containing decimals in the format of the current culture, so 2.1 would become "2.1" in an English culture but "2,1" in a German culture, for example. However, the tests use a hard-coded string "2.1", causing the tests to fail when the current culture uses a comma for decimals. While the tests pass on the build servers because they use an English culture, they can fail when run locally during development.

This PR might not be the best solution - maybe GetRuntimeMetadata should return a culture-invariant string, or all the unit tests should automatically run in the invariant culture regardless of local settings - but it does work around the immediate issue for me.

Copy link
Copy Markdown
Contributor

Oh that's interesting, I didn't know decimals are represented differently based on locale. This LGTM, thanks Sarah!

Brianna Marshall (bamarsha) deleted the samarsha/decimal-culture branch August 18, 2020 17:05
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 subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants


Back | FazBrowse Home | New Git URL