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

Add test coverage for CheckedListBox.CheckedItemCollection and CheckedIndexCollection #11384

Merged
merged 3 commits into from
May 24, 2024

Conversation

Olina-Zhang
Copy link
Member

@Olina-Zhang Olina-Zhang commented May 16, 2024

Related #10453

Proposed changes

  • Add unit tests for CheckedListBox.CheckedItemCollection to test its properties: SyncRoot, IsSynchronized, IsFixedSize, IsReadOnly
  • Add unit tests for CheckedListBox.CheckedItemCollection to test its methods: Add, Clear, Insert, Remove, RemoveAt, Contains, IndexOf, CopyTo, GetEnumerator
  • Update unit tests for CheckedListBox.CheckedIndexCollection to test its methods: Contains, IndexOf
Microsoft Reviewers: Open in CodeFlow

Copy link

codecov bot commented May 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.29662%. Comparing base (f2118b8) to head (1c83a2a).
Report is 33 commits behind head on main.

Additional details and impacted files
@@                 Coverage Diff                 @@
##                main      #11384         +/-   ##
===================================================
+ Coverage   74.27372%   74.29662%   +0.02290%     
===================================================
  Files           3025        3027          +2     
  Lines         626861      627151        +290     
  Branches       46742       46761         +19     
===================================================
+ Hits          465593      465952        +359     
+ Misses        157926      157852         -74     
- Partials        3342        3347          +5     
Flag Coverage Δ
Debug 74.29662% <100.00000%> (+0.02290%) ⬆️
integration 17.99717% <ø> (+0.00123%) ⬆️
production 47.03623% <ø> (+0.03453%) ⬆️
test 96.98767% <100.00000%> (-0.00649%) ⬇️
unit 44.01403% <ø> (+0.04294%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@elachlan elachlan added the waiting-review This item is waiting on review by one or more members of team label May 17, 2024
Copy link
Contributor

@ricardobossan ricardobossan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than small comment, LGTM.

@ricardobossan ricardobossan added 📭 waiting-author-feedback The team requires more information from the author and removed waiting-review This item is waiting on review by one or more members of team labels May 20, 2024
@dotnet-policy-service dotnet-policy-service bot removed the 📭 waiting-author-feedback The team requires more information from the author label May 21, 2024

namespace System.Windows.Forms.Tests;

public class CheckedListBox_CheckedItemCollectionTests : IDisposable
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to create controls for each test to ensure that we start with a clean control every time

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that test class is created before every test is run, but are we guaranteed that the constructor is executed on the STA thread? and that DIspose method is always called even if the test fals?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are following this reference: constructor and dispose pattern.
A TestInitialize like method does not exist in xunit, we have added an empty construct for its uniform operation.
As far as we know, when test cases are run in pipeline, they are randomly assigned to the machine each time and then executed by the vstester, one test case at a time. So, there will be no case of multiple test cases running at the same time.
Are we going to continue to write like this or are we going to use the original creating controls for each test?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the pointer to docs. My other concern was that winforms controls should be used from the same thread they are created on. But in your cases you are not calling CreateControl, only the constructor. So Disposable pattern looks fine.

@Tanya-Solyanik Tanya-Solyanik added the 📭 waiting-author-feedback The team requires more information from the author label May 21, 2024
@dotnet-policy-service dotnet-policy-service bot removed the 📭 waiting-author-feedback The team requires more information from the author label May 22, 2024
@lonitra lonitra added the ready-to-merge PRs that are ready to merge but worth notifying the internal team. label May 23, 2024
@lonitra lonitra merged commit 87c7499 into dotnet:main May 24, 2024
8 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the 9.0 Preview6 milestone May 24, 2024
@dotnet-policy-service dotnet-policy-service bot removed the ready-to-merge PRs that are ready to merge but worth notifying the internal team. label May 24, 2024
@Olina-Zhang Olina-Zhang deleted the Add_UnitTest_CheckedItemCollection branch May 29, 2024 02:09
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.

None yet

5 participants