Skip to content

DataGridTests: Fix test failure by using Invariant Culture#10220

Merged
ScarletKuro merged 3 commits intoMudBlazor:devfrom
meenzen:fix/data-grid-tests
Nov 19, 2024
Merged

DataGridTests: Fix test failure by using Invariant Culture#10220
ScarletKuro merged 3 commits intoMudBlazor:devfrom
meenzen:fix/data-grid-tests

Conversation

@meenzen
Copy link
Contributor

@meenzen meenzen commented Nov 9, 2024

Description

On my machine this test was always failing because for some reason a NNBSP char instead of a whitespace is added:

string actual   = "1/1/0001 12:00:00\u202FAM"
string expected = "1/1/0001 12:00:00 AM"

Because the type of whitespace does not actually matter I've just replaced it with a normal whitespace to make the assertion happy.

How Has This Been Tested?

n/a

Type of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation (fix or improvement to the website or code docs)

Checklist

  • The PR is submitted to the correct branch (dev).
  • My code follows the code style of this project.
  • I've added relevant tests.

@github-actions github-actions bot added bug Unexpected behavior or functionality not working as intended PR: needs review labels Nov 9, 2024
@meenzen meenzen requested a review from ScarletKuro November 9, 2024 21:32
@mikes-gh
Copy link
Contributor

On my machine this test was always failing because for some reason a NNBSP char instead of a whitespace is added:

I think we should find the reason instead of this workaround.

@ScarletKuro
Copy link
Member

Yeah, it would be nice to find the root cause of this problem.
If you open DataGridPropertyColumnNullCheckTest in the viewer, does it also render  ?
What I don’t understand is why it only appears between the time and the AM|PM?

1/1/0001 12:00:00\u202FAM

It seems like there’s none between the date and the time, and the "some text" is not affected either. So, it doesn’t seem like #9916 is the cause.

What happens if you remove or replace SetCulture? We use it in other places too, though we don't deal with datetime there.

@meenzen
Copy link
Contributor Author

meenzen commented Nov 18, 2024

I had a hunch this might be related to ICU... A quick google search confirmed it:

microsoft/icu#134

@mikes-gh
Copy link
Contributor

Just to say I am on mac and get the same error

@mikes-gh
Copy link
Contributor

@ScarletKuro I tried your suggestion of setting culture to Invariant and it gives

       [Test]
        [SetCulture("")]
        [SetUICulture("")]
        public void DataGridPropertyNullCheck()
        {
            var comp = Context.RenderComponent<DataGridPropertyColumnNullCheckTest>();
            var cells = comp.FindAll("td").ToArray();

            // First Row
            cells[0].TextContent.Should().Be("1/1/0001 12:00:00 AM");
Expected cells[0].TextContent to be "1/1/0001 12:00:00 AM" with a length of 20, but "01/01/0001 00:00:00" has a length of 19, differs near "01/" (index 0).

@mikes-gh mikes-gh changed the title DataGridTests: Fix test failure by replacing NNBSP with whitespace DataGridTests: Fix test failure by using Invariant Culture Nov 19, 2024
@sonarqubecloud
Copy link

@codecov
Copy link

codecov bot commented Nov 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.52%. Comparing base (f028a0f) to head (34f3b22).
Report is 23 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev   #10220      +/-   ##
==========================================
+ Coverage   91.22%   91.52%   +0.30%     
==========================================
  Files         412      414       +2     
  Lines       12541    12989     +448     
  Branches     2448     2452       +4     
==========================================
+ Hits        11440    11888     +448     
+ Misses        557      555       -2     
- Partials      544      546       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@mikes-gh
Copy link
Contributor

@ScarletKuro works locally for me now, Ready to merge.

@ScarletKuro ScarletKuro merged commit e1476a6 into MudBlazor:dev Nov 19, 2024
@ScarletKuro
Copy link
Member

ty

ScarletKuro pushed a commit to ScarletKuro/MudBlazor that referenced this pull request Nov 21, 2024
LLauter pushed a commit to cannellamedia/MudBlazor that referenced this pull request Dec 16, 2024
CoreyHayward pushed a commit to CoreyHayward/MudBlazor that referenced this pull request Jan 17, 2025
ScarletKuro pushed a commit that referenced this pull request Jan 17, 2025
Co-authored-by: Anu6is <[email protected]>
Co-authored-by: Samuel Meenzen <[email protected]>
Co-authored-by: Mike Surcouf <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Unexpected behavior or functionality not working as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants