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

#7320 Fixing flaky ComboBox unit test #7323

Merged
merged 1 commit into from
Jun 22, 2022

Conversation

dkazennov
Copy link
Contributor

@dkazennov dkazennov commented Jun 17, 2022

Fixes #7320

Proposed changes

  • added new stable unit test ComboBox_Select_Item_By_Key without InputSimulator.SendAsync;
  • removed old integration tests ComboBox_Select_Item_By_DownArrowKeyAsync and ComboBox_Select_Item_By_UpArrowKeyAsync in UIIntegrationTests

Customer Impact

Regression?

  • No

Risk

  • Minimal

Test methodology

  • Unit tests

Test environment(s)

  • dotnet 7.0.0-preview.4.22229.4
Microsoft Reviewers: Open in CodeFlow

@dkazennov dkazennov requested a review from a team as a code owner June 17, 2022 14:48
@ghost ghost assigned dkazennov Jun 17, 2022
@dkazennov dkazennov added test-bug Problem in test source code (most likely) draft draft PR labels Jun 17, 2022
@ghost ghost removed the draft draft PR label Jun 17, 2022
Copy link
Member

@RussKie RussKie left a comment

Choose a reason for hiding this comment

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

Great work!

new stable unit test ComboBox_Select_Item_By_Key without InputSimulator.SendAsync;

👍

Both ways seem to work, but the new unit test seems more convenient and efficient (there is only one test with WinFormsTheory. It doesn't require the asynchronicity and doesn't generate a form). We should leave only one.

Works for me. Happy for the UI integration tests to go if we get the same test coverage.

@RussKie RussKie added the 📭 waiting-author-feedback The team requires more information from the author label Jun 20, 2022
@dkazennov dkazennov force-pushed the Issue_7320_FlakyComboBoxTest branch from 9a88a5e to 59f5a5e Compare June 20, 2022 07:50
@ghost ghost removed the 📭 waiting-author-feedback The team requires more information from the author label Jun 20, 2022
@dkazennov
Copy link
Contributor Author

Removed old integration tests ComboBox_Select_Item_By_DownArrowKeyAsync and ComboBox_Select_Item_By_UpArrowKeyAsync in UIIntegrationTests.

@dkazennov dkazennov added the waiting-review This item is waiting on review by one or more members of team label Jun 20, 2022
Copy link
Contributor

@dmitrii-drobotov dmitrii-drobotov left a comment

Choose a reason for hiding this comment

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

LGTM 👍


public void AddItems(int numToAdd)
{
for (int i = 0; i <= numToAdd; i++)
Copy link
Contributor

Choose a reason for hiding this comment

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

Now it adds numToAdd + 1 items

Suggested change
for (int i = 0; i <= numToAdd; i++)
for (int i = 0; i < numToAdd; i++)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We add 10 items. Then we select a first item with the index 0 and after that we simulate the down key pressure. The first item selected with the key will have the index 1. So we need one more item as a starting item.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Though comboBox.AddItems(expectedKeyPressesCount+1); would be better. Thus we will be adding the exact number of items given in the argument.

@RussKie RussKie added 📭 waiting-author-feedback The team requires more information from the author waiting-review This item is waiting on review by one or more members of team and removed waiting-review This item is waiting on review by one or more members of team labels Jun 21, 2022
@dkazennov dkazennov dismissed stale reviews from RussKie and dmitrii-drobotov via d4b2748 June 21, 2022 07:09
@dkazennov dkazennov force-pushed the Issue_7320_FlakyComboBoxTest branch from bbf7ead to d4b2748 Compare June 21, 2022 07:09
@ghost ghost removed the 📭 waiting-author-feedback The team requires more information from the author label Jun 21, 2022
@dkazennov dkazennov added the waiting-review This item is waiting on review by one or more members of team label Jun 21, 2022
@RussKie
Copy link
Member

RussKie commented Jun 22, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@RussKie RussKie enabled auto-merge (squash) June 22, 2022 01:35
@RussKie RussKie removed the waiting-review This item is waiting on review by one or more members of team label Jun 22, 2022
@RussKie RussKie merged commit ef0c855 into dotnet:main Jun 22, 2022
@ghost ghost added this to the 7.0 Preview6 milestone Jun 22, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jul 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
test-bug Problem in test source code (most likely)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flaky System.Windows.Forms.UITests.ComboBoxTests.ComboBox_Select_Item_By_UpArrowKeyAsync
4 participants