winforms icon indicating copy to clipboard operation
winforms copied to clipboard

Add tests for VB runtime REVIEW 1ST DRAFT

Open paul1956 opened this issue 1 year ago • 12 comments

Fixes #5179 and uses Fluent Assertions

Proposed changes

Add tests for Assembly Public Key Is As Expected - used in other tests Clipboard Proxy Computer Info and Debug View Enhance Control Tests to support new Project Level Options A few Exception Tests more will be added with future PR File IO Proxy Tests Network Download Single Instance Helpers Temp Directory File Functions used by new tests General new utilities use by tests Time User Tests Web Listener Service

Customer Impact

Improve code quality via additional tests

Regression?

-No

Risk

Low - additional tests and code changes required to make them work

Test methodology

Visual Studio

Microsoft Reviewers: Open in CodeFlow

paul1956 avatar Aug 13 '24 03:08 paul1956

Codecov Report

Attention: Patch coverage is 87.34911% with 262 lines in your changes missing coverage. Please review.

Project coverage is 75.74253%. Comparing base (1d004bb) to head (51b5b5f). Report is 4 commits behind head on main.

Additional details and impacted files
@@                 Coverage Diff                 @@
##                main      #11863         +/-   ##
===================================================
+ Coverage   75.66441%   75.74253%   +0.07811%     
===================================================
  Files           3141        3150          +9     
  Lines         635604      637284       +1680     
  Branches       47012       47011          -1     
===================================================
+ Hits          480926      482695       +1769     
+ Misses        151230      151140         -90     
- Partials        3448        3449          +1     
Flag Coverage Δ
Debug 75.74253% <87.34911%> (+0.07811%) :arrow_up:
integration 18.15260% <0.00000%> (-0.10089%) :arrow_down:
production 49.26439% <52.79279%> (+0.06029%) :arrow_up:
test 97.04388% <100.00000%> (+0.01303%) :arrow_up:
unit 46.34624% <52.79279%> (+0.16340%) :arrow_up:

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

codecov[bot] avatar Aug 13 '24 05:08 codecov[bot]

@paul1956 main is now .NET 10.0 alpha and I have synced main with feature/10.0. I also retargeted your PR to main. On each of your branches, I believe you only need to run command git branch --set-upstream-to=upstream/main where your local winforms repo lives to set your branch up to pull updates from main branch again. Let me know if you run into any issues.

lonitra avatar Aug 15 '24 00:08 lonitra

@paul1956 - it will be easier to review your PRs if you marked the ones that depend on the previous changes as drafts. It's not clear if titles of your PRs are up-to-date or not.

Tanya-Solyanik avatar Sep 13 '24 18:09 Tanya-Solyanik

@Tanya-Solyanik I was wondering how to do that I will update now.

paul1956 avatar Sep 13 '24 20:09 paul1956

@paul1956 - you might not have access to these features - image

So adding Draft to the title will work, ping me to remove draft state when you are ready.

Tanya-Solyanik avatar Sep 18 '24 04:09 Tanya-Solyanik

@Tanya-Solyanik except in a comment how to I ping you?

paul1956 avatar Sep 18 '24 04:09 paul1956

@Tanya-Solyanik except in a comment how to I ping you?

like this :)

Tanya-Solyanik avatar Sep 18 '24 17:09 Tanya-Solyanik

@Tanya-Solyanik ready for review, thanks

paul1956 avatar Sep 18 '24 20:09 paul1956

Paul - this PR is just too big to handle in one go. We need to split this up.

What is the one with the highest priority in your opinion and why. Let's discuss this first, and then we are tackeling that one.

Please keep always in mind, we have limited resources, and this area, although important, doesn't have the highest priorities.

KlausLoeffelmann avatar Sep 24 '24 19:09 KlausLoeffelmann

@KlausLoeffelmann Network download tests which is the majority of the tests (they are now in #12221). Most of the rest of this is code cleanup from other PR feedback.

The changes to the actual network code is just splitting it up into about 10 pieces without code changes to make it easier to focus on getting rid of WebClient for download in the next PR. There are no logic changes in the VB Code in Microsoft.VisualBasic.Forms its almost all formatting which someone else can review. There is nothing in the PR that requires your review anymore. All the new network test code is in #12221.

The next PR #11867 is where all the code changes happen to remove WebClient from NetworkDownload and all the Async code is added. Without the tests that is very risky.

paul1956 avatar Sep 24 '24 22:09 paul1956

OK, I am losing a bit of track here, to be honest. Let me add a tracking issue for VB, and then let's add all the PRs there first and order them by priority.

KlausLoeffelmann avatar Sep 26 '24 06:09 KlausLoeffelmann

@Tanya-Solyanik Can you mark this draft. I have already stared to break it into very small mostly single function tests and cleanup (https://github.com/dotnet/winforms/pull/12226, https://github.com/dotnet/winforms/pull/12221, https://github.com/dotnet/winforms/pull/12227...) based in Tracking issue #12230. This will never be reopened I will just use it as source for many small tests and formatting changes and to make sure nothing is left out; I don't want to delete it as it is useful for me to improve test coverage of the VB Codebase (since it has everything that was not in draft) . https://github.com/dotnet/winforms/pull/12221 will be reviewed by @KlausLoeffelmann when he has time to devote as it has a lot of new VB Code including a test Download Server.

Everything not marked as draft can be reviewed in any order, as you team's workload permits. They are mostly tests or minor cleanup covered by tests and they are all based on Main, plus none touch the same files.

Thanks for all your help.

paul1956 avatar Oct 03 '24 08:10 paul1956

This has been replaced with other small PR's

paul1956 avatar Oct 30 '24 11:10 paul1956