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 tests for VB runtime REVIEW 1ST #11863

Open
wants to merge 348 commits into
base: main
Choose a base branch
from

Conversation

paul1956
Copy link
Contributor

@paul1956 paul1956 commented Aug 13, 2024

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

Copy link

codecov bot commented Aug 13, 2024

Codecov Report

Attention: Patch coverage is 88.73239% with 256 lines in your changes missing coverage. Please review.

Project coverage is 75.52850%. Comparing base (0d0e1c9) to head (919d000).
Report is 3 commits behind head on main.

Additional details and impacted files
@@                 Coverage Diff                 @@
##                main      #11863         +/-   ##
===================================================
+ Coverage   75.42712%   75.52850%   +0.10137%     
===================================================
  Files           3102        3118         +16     
  Lines         634244      636140       +1896     
  Branches       46866       46881         +15     
===================================================
+ Hits          478392      480467       +2075     
+ Misses        152434      152252        -182     
- Partials        3418        3421          +3     
Flag Coverage Δ
Debug 75.52850% <88.73239%> (+0.10137%) ⬆️
integration 17.88971% <0.00000%> (-0.08987%) ⬇️
production 48.90322% <56.22837%> (+0.09952%) ⬆️
test 97.04082% <99.82290%> (+0.01457%) ⬆️
unit 46.03779% <56.22837%> (+0.20192%) ⬆️

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

@lonitra lonitra changed the base branch from feature/10.0 to main August 15, 2024 00:56
@lonitra
Copy link
Member

lonitra commented Aug 15, 2024

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

Replace redundent CloseProgressDialog
Add Tests for ExceptionUtils
Remove unneeded Paentheses
Cleanup spacing
Sort UnsafeNativeMethods
Clean up and correct all XML comments in ClipboardProxy (copy from System.Windows.Forms.Clipboard
@Tanya-Solyanik
Copy link
Member

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

like this :)

@paul1956 paul1956 changed the title Add tests for VB runtime REVIEW (2nd) AFTER #11987 DRAFT Add tests for VB runtime REVIEW 1ST Sep 18, 2024
@paul1956
Copy link
Contributor Author

@Tanya-Solyanik ready for review, thanks

@Tanya-Solyanik Tanya-Solyanik added waiting-review This item is waiting on review by one or more members of team and removed untriaged The team needs to look at this issue in the next triage labels Sep 19, 2024
@KlausLoeffelmann
Copy link
Member

KlausLoeffelmann commented Sep 24, 2024

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.

@paul1956
Copy link
Contributor Author

paul1956 commented Sep 24, 2024

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

@KlausLoeffelmann
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting-review This item is waiting on review by one or more members of team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WinForms project templates use latest VB language idioms
5 participants