Skip to content
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

Concat FluentCounterBadge tests #904

Merged
merged 5 commits into from
Oct 31, 2023

Conversation

kurnakovv
Copy link
Contributor

📖 Description

My changes concat duplicated tests in single.

As you can see

Has duplicate logic, and I think it's not good, because:

  • We violating DRY
  • Tests more slower, because we should start 2 tests, not 1 test

I hope this changes make your code better. Thank you for this super cool Blazor components! :)

✅ Checklist

General

  • I have added tests for my changes.
  • I have tested my changes.
  • I have updated the project documentation to reflect my changes.
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

Component-specific

  • I have added a new component
  • I have added Unit Tests for my new compontent
  • I have modified an existing component
  • I have validate Unit Tests for an existing component

FluentCounterBadge_BackgroundColorLightweightLuminanceLight & FluentCounterBadge_BackgroundColorLightweightLuminanceDark.
@kurnakovv
Copy link
Contributor Author

@microsoft-github-policy-service agree

@vnbaaij
Copy link
Collaborator

vnbaaij commented Oct 29, 2023

Actually, both tests are needed. The dark one however has a wrong value for the color parameters. We found this because of your work here. Thanks!

@dvoituron
Copy link
Collaborator

  1. Your PR is not executing the tests correctly, since you changed the method name.
    image

  2. I agree with Vincent, these two tests are useful. However, the Dark test is missing in the second test:
    GlobalState.Luminance = StandardLuminance.DarkMode;.

@kurnakovv
Copy link
Contributor Author

@dvoituron p.2 Sounds like logic, but now (I didn't commit yet), FluentCounterBadge_BackgroundColorLightweightLuminanceDark test throw unexpected assert:

  Message: 
    Bunit.HtmlEqualException : HTML comparison failed. 
    
    The following errors were found:
      1: The value of the attribute div(1) > div(1)[style] and actual attribute div(0) > div(1)[style] are different.
    
    Actual HTML: 
    <div class="counterbadge-container" >childcontent<div class="counterbadge" style="left: 50%; bottom: 50%; background-color: var(--neutral-fill-inverse-rest); color: var(--neutral-fill-rest); border: 1px solid var(--neutral-fill-inverse-rest);" title="1" >1</div>
    </div>
    
    Expected HTML: 
    <div class="counterbadge-container" >childcontent<div class="counterbadge" style="left: 50%; bottom: 50%; background-color: var(--neutral-layer-1); color: var(--neutral-fill-rest); border: 1px solid var(--neutral-layer-1);" title="1" >1</div>
    </div>

Do you know what's wrong? Code is invalid or test environment? I will so happy if you will help me :)

@vnbaaij and @dvoituron Thank's for your fast reaction, you are very good programmers! ;)

@dvoituron
Copy link
Collaborator

You have our FluentUI Unit Tests documentation here : https://github.com/microsoft/fluentui-blazor/blob/dev/unit-tests.md#fluentui-blazor-unit-tests

Probably you could update the .verified file with the new changes.

I will check this evening to give you more details.

@kurnakovv
Copy link
Contributor Author

@vnbaaij Thank's for merge dev -> concat-FluentCounterBadgeTests :)

@dvoituron Thank's for help, but I have some problems :( I updated FluentCounterBadge_BackgroundColorLightweightLuminanceDark.verify.razor and now:

Message: 
Bunit.HtmlEqualException : HTML comparison failed. 

The following errors were found:
  1: The value of the attribute div(1) > div(1)[style] and actual attribute div(0) > div(1)[style] are different.

Actual HTML: 
<div class="counterbadge-container" >childcontent<div class="counterbadge" style="left: 50%; bottom: 50%; background-color: var(--neutral-fill-inverse-rest); color: var(--neutral-fill-rest); border: 1px solid var(--neutral-fill-inverse-rest);" title="1" >1</div>
</div>

Expected HTML: 
<div class="counterbadge-container">childcontent<div class="counterbadge" style="left: 50%; bottom: 50%; background-color: var(--neutral-layer-1); color: var(--neutral-fill-rest); border: 1px solid var(--neutral-layer-1);" title="1">1</div>
</div>

But my code is the same as Expected.

What's wrong?

I will check this evening to give you more details

Thank you for this :)

@dvoituron
Copy link
Collaborator

You can easily detect the problem using Visual Studio Compare Files (or a similar tool). I've explained this in the video at the top of the Unit Tests page.

For your example, you have to update the received file to use the correct color for Dark theme: var(--neutral-fill-inverse-rest)

image

@kurnakovv
Copy link
Contributor Author

Super, now this is work! :)
image

@vnbaaij vnbaaij merged commit 2092755 into microsoft:dev Oct 31, 2023
1 check passed
@kurnakovv kurnakovv deleted the concat-FluentCounterBadgeTests branch October 31, 2023 09:11
@kurnakovv
Copy link
Contributor Author

@vnbaaij @dvoituron Thanks again, good luck to you and your product ;)

@vnbaaij
Copy link
Collaborator

vnbaaij commented Oct 31, 2023

If you want to and are up to it, please submit more PRs with tests. We need them!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants