Skip to content

Add TreeView property tests #1080

Merged
zsd4yr merged 7 commits intodotnet:masterfrom
hughbe:tree-view
Jun 18, 2019
Merged

Add TreeView property tests #1080
zsd4yr merged 7 commits intodotnet:masterfrom
hughbe:tree-view

Conversation

@hughbe
Copy link
Contributor

@hughbe hughbe commented May 31, 2019

No description provided.

@hughbe hughbe requested a review from a team as a code owner May 31, 2019 19:43
Copy link
Contributor

@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.

Top job!
I glanced over the tests, for the most part they looked good with exception for the comment I left about arrangements.

I'll need more time to go though all 3.5K lines code in greater details.

@danmoseley
Copy link
Member

@RussKie I see you are a committer now - great! I'll let you own this one then 😺

Copy link
Contributor

@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.

👍

@RussKie
Copy link
Contributor

RussKie commented Jun 3, 2019

Do you wish squash yourself or me to squash-merge?

[InlineData(1, 15)]
[InlineData(2, 15)]
[InlineData(5, 15)]
[InlineData(6, 15)]
Copy link
Contributor

Choose a reason for hiding this comment

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

This change is actually breaking the tests
image

@RussKie
Copy link
Contributor

RussKie commented Jun 3, 2019

It looks like debug and release builds yield different results... This is wwird

@hughbe
Copy link
Contributor Author

hughbe commented Jun 3, 2019

Any idea why that would be the case? Maybe it depends on the machine?

@RussKie
Copy link
Contributor

RussKie commented Jun 3, 2019

Not quite sure at this stage. I will have to dig deeper into it.

@AdamYoblick
Copy link
Contributor

When running the tests locally, debug (.\build -test) fails while release (.\build -test -configuration Release) passes. I didn't step through the code, but it's reproable locally. @hughbe can you please try to step through in debug and see what you find?

Thanks :)

@hughbe
Copy link
Contributor Author

hughbe commented Jun 18, 2019

Great! I fixed the tests :)

@zsd4yr zsd4yr merged commit 232ed55 into dotnet:master Jun 18, 2019
@hughbe hughbe deleted the tree-view branch June 18, 2019 14:56
@weltkante
Copy link
Contributor

@hughbe the fixes seem to weaken the tests significantly, as it no longer tests that the user provided values are preserved. I see that its probably happening outside the WinForms codebase in the native control, but do you have any explanation why Indent and ItemHeight should not preserve the user specified values? Is this always the case or only under certain conditions? If its the latter, it would probably be worth to also have tests for the "common" cases where the user defined values are to be preserved.

@ghost ghost locked as resolved and limited conversation to collaborators Feb 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants