-
Notifications
You must be signed in to change notification settings - Fork 0
Option 3 - fixed: Update date formatting tests to handle culture-specific formats #22
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?
Option 3 - fixed: Update date formatting tests to handle culture-specific formats #22
Conversation
…- Convert DateTagHelper unit tests to use InlineData pattern with multiple cultures
| TagHelperContext tagHelperContext = new TagHelperContext([], new Dictionary<object, object>(), Guid.NewGuid().ToString("N", CultureInfo.InvariantCulture)); | ||
| TagHelperOutput tagHelperOutput = new TagHelperOutput(string.Empty, [], (_, _) => | ||
| { | ||
| DefaultTagHelperContent tagHelperContent = new(); | ||
| return Task.FromResult<TagHelperContent>(tagHelperContent); | ||
| }); | ||
|
|
||
| tagHelperOutput.TagName = RenderingEngineConstants.SitecoreTagHelpers.DateHtmlTag; |
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.
Why do we need to new these up when they are added to the Fixture and can be passed in like the sut was previously?
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.
I have updated this to - use [InlineAutoNSubstituteData] attributes and receive dependency injected instances.
| TagHelperContext tagHelperContext = new TagHelperContext([], new Dictionary<object, object>(), Guid.NewGuid().ToString("N", CultureInfo.InvariantCulture)); | ||
| TagHelperOutput tagHelperOutput = new TagHelperOutput(string.Empty, [], (_, _) => | ||
| { | ||
| DefaultTagHelperContent tagHelperContent = new(); | ||
| return Task.FromResult<TagHelperContent>(tagHelperContent); | ||
| }); | ||
|
|
||
| tagHelperOutput.TagName = RenderingEngineConstants.SitecoreTagHelpers.DateHtmlTag; |
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.
Same again, these can be passed in from the fixture right?
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.
I have updated this to - use [InlineAutoNSubstituteData] attributes and receive dependency injected instances.
…ndency-injected instances
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 pull request updates date formatting tests to handle culture-specific formats, addressing test failures when running with different cultures like Danish (da-DK) where date separators change from "/" to "-".
- Converts DateTagHelper unit tests to use InlineData pattern with multiple cultures (en-US, da-DK, uk-UA)
- Updates integration tests to use CurrentCulture for date formatting instead of hardcoded US format
- Adds explicit type declarations and required using statements for System.Globalization
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 | Converts unit tests to multi-culture InlineData pattern with explicit culture handling |
| RichTextFieldTagHelperFixture.cs | Updates date expectations to use CurrentCulture formatting |
| DateFieldTagHelperFixture.cs | Replaces hardcoded date strings with culture-aware formatting |
| AllFieldTagHelpersFixture.cs | Updates date field assertions to use CurrentCulture |
| ViewFieldsBindingFixture.cs | Modifies date format expectations to handle culture differences |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| { | ||
| // Arrange | ||
| CultureInfo testCulture = new CultureInfo(cultureName); | ||
| const string dateFormat = "MM/dd/yyyy H:mm"; |
Copilot
AI
Aug 18, 2025
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.
The hardcoded date format 'MM/dd/yyyy H:mm' may not work correctly across all cultures being tested. Consider using culture-specific format patterns or testing with formats that are more universal across the specified cultures (en-US, da-DK, uk-UA).
| const string dateFormat = "MM/dd/yyyy H:mm"; | |
| const string dateFormat = "yyyy-MM-dd HH:mm"; |
| { | ||
| // Arrange | ||
| CultureInfo testCulture = new CultureInfo(cultureName); | ||
| string dateFormat = "MM/dd/yyyy H:mm"; |
Copilot
AI
Aug 18, 2025
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.
The hardcoded date format 'MM/dd/yyyy H:mm' may not work correctly across all cultures being tested. Consider using culture-specific format patterns or testing with formats that are more universal across the specified cultures (en-US, da-DK, uk-UA).
| string dateFormat = "MM/dd/yyyy H:mm"; | |
| string dateFormat = "yyyy-MM-dd HH:mm"; |
Files modified,