sillsdev / l10nsharp

A .NET localization library for Windows Forms applications.
3 stars 10 forks source link

BL-4887 Add Unit test for Newline handling #35

Closed JohnThomson closed 6 years ago

JohnThomson commented 6 years ago

This change is Reviewable

StephenMcConnel commented 6 years ago

Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion.


src/L10NSharpTests/LocalizationManagerTests.cs, line 799 at r1 (raw file):

              Assert.That(manager.StringCache.GetValueForExactLangAndId("fr", "anotherId", false), Is.EqualTo("Two lines of\\nFrench"));
              Assert.That(LocalizationManager.GetString("anotherId", "Two lines of" + Environment.NewLine + "English"), Is.EqualTo("Two lines of" + LocalizedStringCache.kOSRealNewline + "French"));

Add maybe a third check with "\r\n" explicitly in the string being added dynamically instead of Environment.NewLine? Of course, I'd like to see how the test performs on Linux. :-)


Comments from Reviewable

StephenMcConnel commented 6 years ago

suggested added check, but otherwise looks like a good added test.


Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

StephenMcConnel commented 6 years ago

Reviewed 1 of 1 files at r1. Review status: all files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

JohnThomson commented 6 years ago

Review status: 0 of 2 files reviewed at latest revision, 1 unresolved discussion.


src/L10NSharpTests/LocalizationManagerTests.cs, line 799 at r1 (raw file):

Previously, StephenMcConnel (Steve McConnel) wrote…
Add maybe a third check with "\r\n" explicitly in the string being added dynamically instead of Environment.NewLine? Of course, I'd like to see how the test performs on Linux. :-)

Done. I've made the change I think is needed to make this work on Linux, so maybe you'll actually get some good out of this PR!


Comments from Reviewable

StephenMcConnel commented 6 years ago

Reviewed 2 of 2 files at r2. Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

StephenMcConnel commented 6 years ago
:lgtm:

Review status: :shipit: all files reviewed at latest revision, all discussions resolved, all commit checks successful.


Comments from Reviewable