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

Winform_CTI add the CheckBox RT case #2310

Open
wants to merge 1 commit into
base: master
from

Conversation

@Zheng-Li01
Copy link
Member

Zheng-Li01 commented Nov 7, 2019

Add the CheckBox RT case to Xunit

Microsoft Reviewers: Open in CodeFlow
@Zheng-Li01 Zheng-Li01 requested a review from dotnet/dotnet-winforms as a code owner Nov 7, 2019
@Zheng-Li01

This comment has been minimized.

Copy link
Member Author

Zheng-Li01 commented Nov 7, 2019

@AdamYoblick,@RussKie, Could you please take a look at this pull and approve it if the changes are ok?

//
// CheckBox priority 1 tests
//

//
// Created by: Hallas - February 2001.
// Test fix: zhenlwa - January 7, 2007
//

Comment on lines +17 to +25

This comment has been minimized.

Copy link
@RussKie

RussKie Nov 7, 2019

Member

Not needed, please remove

this.Text = "CheckBoxP1";
base.InitTest(p);

// Ctreate and Initialize control

This comment has been minimized.

Copy link
@RussKie

RussKie Nov 7, 2019

Member

ditto


//zhenlwa: Before the show-up of form, the location of checkbox cannot be calculated correctly.
//pointToClick = CalculatePointToClick();
Comment on lines +63 to +65

This comment has been minimized.

Copy link
@RussKie

RussKie Nov 7, 2019

Member

ditto



// [Scenarios]
//@ AutoCheck()
//@ CheckedProperty()
//@ CheckStateProperty()
//@ ThreeState()
Comment on lines +415 to +421

This comment has been minimized.

Copy link
@RussKie

RussKie Nov 7, 2019

Member

ditto

const int MAX_TEXT_LEN = 256;
const int MAX_MENU_ITEM = 10; // max length of string for menu items
const int MIN_TEXT_LEN = 5; // minimal Text length when not empty string

private CheckBox cb;

bool eventFired = false; // true if CheckBox.Click was raised
int eventCount = 0; // number of CheckBox.Click events
Comment on lines +31 to +38

This comment has been minimized.

Copy link
@RussKie

RussKie Nov 7, 2019

Member

Please reformat the file and apply Roslyn Analyser fixes. E.g. order of usings, field names, missing accessors etc

This comment has been minimized.

Copy link
@AdamYoblick

AdamYoblick Nov 7, 2019

Member

Agree with this. It will clean up access modifiers and variable names, etc.

Winforms.sln Show resolved Hide resolved
const int MAX_TEXT_LEN = 256;
const int MAX_MENU_ITEM = 10; // max length of string for menu items
const int MIN_TEXT_LEN = 5; // minimal Text length when not empty string

private CheckBox cb;

bool eventFired = false; // true if CheckBox.Click was raised
int eventCount = 0; // number of CheckBox.Click events

This comment has been minimized.

Copy link
@AdamYoblick

AdamYoblick Nov 7, 2019

Member

Agree with this. It will clean up access modifiers and variable names, etc.

// When CheckOnClick = false, mouse clicks should not toggle Checked/CheckState.
//
[Scenario(true)]
public ScenarioResult AutoCheck(TParams p)

This comment has been minimized.

Copy link
@AdamYoblick

AdamYoblick Nov 7, 2019

Member

Please be more explicit with ALL scenario names. I don't know what AutoCheck means.

It looks like you're testing some combination of AutoCheck and ThreeState, so something explaining what's being tested would be prudent.

If it's testing too many things, then break it up into different scenarios please.

// Verify that changing ThreeState doesn't affect current Checked / CheckState
//
[Scenario(true)]
public ScenarioResult ThreeState(TParams p)

This comment has been minimized.

Copy link
@AdamYoblick

AdamYoblick Nov 7, 2019

Member

The comment here is a good indicator of what this scenario should be named. Something like ThreeState_Does_Not_Affect_Checked


<ItemGroup>
<Reference Include="Maui.Core">
<HintPath>\\winformssrvvm01\NETCoreMigration\RTTestCasesMigration\Maui.Core.dll</HintPath>

This comment has been minimized.

Copy link
@AdamYoblick

AdamYoblick Nov 7, 2019

Member

No no no. This will never work in CI automation. If you need the Maui.Core.dll, it should go into IntegrationTests/MauiTests/lib (along with any other dll's it needs)

This comment has been minimized.

Copy link
@AdamYoblick

AdamYoblick Nov 7, 2019

Member

See Directory.Build.props inside the MauiTests folder for an example of how to reference these files.

This comment has been minimized.

Copy link
@RussKie

RussKie Nov 8, 2019

Member

Ideally we shouldn't be checking binaries in, instead restore them from a NuGet feed.
Where do these assemblies come from?

This comment has been minimized.

Copy link
@AdamYoblick

AdamYoblick Nov 8, 2019

Member

They are part of Maui, an internal tool. I don’t think there’s a package for it but I could be wrong. ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.