-
Notifications
You must be signed in to change notification settings - Fork 9
Fix culture-dependent date formatting tests to work with different system locales #48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Fix culture-dependent date formatting tests to work with different system locales #48
Conversation
… test cultures - Updated DateTagHelper to use CultureInfo.InvariantCulture when DateFormat is specified and no explicit Culture is set, ensuring consistent date output regardless of system locale. - Explicitly set culture to en-US in RichTextFieldTagHelperFixture and other relevant integration test fixtures to make test expectations stable and independent of the developer's environment. - Updated test assertions to use the correct culture where appropriate. - Resolves test failures related to date formatting when running tests under non-US locales (e.g., da-DK).
…gHelperFixture.cs, extracting the culture selection logic into a separate variable for better readability in DateTagHelper.cs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a couple of inline comments, also you've made code changes in the DateTagHelper.cs but I'm not seeing any changes to the Unit Tests for that class. If you've changed functionality then there should be a related unit test added to ensure it continues to function as expected.
src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/DateTagHelper.cs
Outdated
Show resolved
Hide resolved
...spNetCore.SDK.RenderingEngine.Integration.Tests/Fixtures/Binding/ViewFieldsBindingFixture.cs
Outdated
Show resolved
Hide resolved
...tCore.SDK.RenderingEngine.Integration.Tests/Fixtures/TagHelpers/DateFieldTagHelperFixture.cs
Outdated
Show resolved
Hide resolved
...e.SDK.RenderingEngine.Integration.Tests/Fixtures/TagHelpers/RichTextFieldTagHelperFixture.cs
Outdated
Show resolved
Hide resolved
…- Convert DateTagHelper unit tests to use InlineData pattern with multiple cultures
This reverts commit debfcde.
…ry operator" This reverts commit 30986a2.
…eFieldTagHelperFixture.cs, extracting the culture selection logic into a separate variable for better readability in DateTagHelper.cs" This reverts commit 979e588.
… and set test cultures" This reverts commit ccc68c7.
…ndency-injected instances
…ine Data Approach To main PR - Update date formatting tests to handle culture-specific formats - Inline Data Approach
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes culture-dependent date formatting tests that were failing on systems with non-US locales. The tests previously used hardcoded US date format expectations while the DateTagHelper implementation correctly uses the current system culture.
- Updated unit tests to use parameterized testing with multiple cultures (en-US, da-DK, uk-UA)
- Modified integration tests to dynamically generate expected date strings using current culture
- Added explicit culture specification in date formatting tests to ensure consistent behavior
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| DateTagHelperFixture.cs | Converted hardcoded date format tests to parameterized tests with explicit culture handling |
| AllFieldTagHelpersFixture.cs | Updated date field assertion to use current culture formatting |
| DateFieldTagHelperFixture.cs | Changed hardcoded date strings to dynamically generated culture-aware formats |
| RichTextFieldTagHelperFixture.cs | Added culture-aware date parsing and formatting for expected values |
| ViewFieldsBindingFixture.cs | Implemented invariant culture parsing with current culture formatting |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Description / Motivation
Issue
Tests in the Sitecore ASP.NET Core SDK were failing when run on systems with non-US locales (e.g., Danish locale). The failures occurred because the tests were using hardcoded US date format expectations while the
DateTagHelperimplementation correctly usesCultureInfo.CurrentCulturewhen no specific culture is provided.Root Cause
The test assertions were inconsistent with the actual implementation behavior:
CultureInfo.CurrentCulturewhen no culture is specified (line 65 in DateTagHelper.cs)This mismatch caused test failures when running on systems with different date formatting conventions (e.g., Danish locale uses "05-04-2012" format).
Solution
Updated test assertions to dynamically generate expected values using the current culture instead of hardcoded strings:
Unit Tests (
DateTagHelperFixture.cs):Integration Tests:
AllFieldTagHelpersFixture.cs: UseTestConstants.DateTimeValue.ToString("MM/dd/yyyy", CultureInfo.CurrentCulture)DateFieldTagHelperFixture.cs: Generate expected format strings dynamicallyRichTextFieldTagHelperFixture.cs: Use culture-aware date parsing and formattingViewFieldsBindingFixture.cs: Parse dates with invariant culture and format with current cultureBehavior
This fix ensures the test suite is portable across different regional settings while maintaining the correct functionality of culture-aware date formatting.
Fixes #34
Testing
Terms