skwasjer / IbanNet

C# .NET IBAN validator, parser, builder and generator
Apache License 2.0
119 stars 31 forks source link

Add "nl" resx files #97

Closed jeroenheijmans closed 1 year ago

jeroenheijmans commented 1 year ago

This adds Dutch ("NL") translations for all existing .resx files in all various projects.

Fixes #94

I was debating how (and if) to include updated test coverage for these changes. All current tests keep passing on my machine. But adding test coverage in a maintainable way presents a few choices about the test setup.

First up, I have confirmed to some degree with tests that this is working, by modifying an existing test into this one:

[Fact]
public void When_creating_instance_it_should_have_expected_message___AdhocTestOfLocalization()
{
    // Adhoc fact to verify if localization of error messages is working.
    CultureInfo.DefaultThreadCurrentUICulture = new CultureInfo("nl-NL");
    var country = new IbanCountry("NL") { DisplayName = "The display name" };
    var sut = new CountryNotAcceptedResult(country);
    sut.ErrorMessage.Should().Be("Bankrekeningnummers van het land The display name worden niet geaccepteerd.");
}

It hard-coded verifies that the error message picks up the Dutch translation.

Unfortunately there does not seem to be a super-widespread way to make tests run for multiple cultures using xUnit. This requires a custom attribute (with all associated maintenance) it seems.

At first thought, I see these options (that can be mixed and matched) as far as testing goes:

  1. Do nothing. Just rely on an occasional check that this is working.
  2. Write a smoke test with some reflection to ensure all resx items have been translated into all supported languages. (So test the presence of translations.)
  3. Create an xUnit attribute (or find a contrib package that does so) to re-run specific tests for different cultures. Apply this attribute to existing tests that check localization.
  4. Write one (bit more hard coded) test per csproj/language to verify that the resx for that language/project is being picked up.

Surely there are more options too?

I stuck with "1" for the moment. Let me know if I can and should support some more in this regard?

skwasjer commented 1 year ago

Thanks, lgtm.

I would not worry too much about covering localization throughout the solution atm. One trick is to set the locale before running dotnet test such that the test runner inherits the culture from the OS. That way, although all tests must be run for each locale, it is an agnostic approach that does not require polluting the tests themselves. Just another GitHub action job matrix.

I do not prefer a methodology that could prevent future CI pipelines to continue in the case of a missing translation. I might not have translators on-hand to help translating new text that is added, nor would I want it to block a new feature being released because of it. In such cases, the fallback to English should be allowed.

So for now, I am gonna agree with you and stay with no. 1) 👍