Modernize VB ExceptionUtils add Test Coverage, fix Time Tests REVIEW 1st#11982
Conversation
Add Tests for ExceptionUtils
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11982 +/- ##
===================================================
+ Coverage 75.18905% 75.19972% +0.01067%
===================================================
Files 3073 3076 +3
Lines 632769 632876 +107
Branches 46794 46794
===================================================
+ Hits 475773 475921 +148
+ Misses 153610 153554 -56
- Partials 3386 3401 +15
Flags with carried forward coverage won't be shown. Click here to find out more. |
Remove unneeded Paentheses Cleanup spacing Sort UnsafeNativeMethods
|
@lonitra @Tanya-Solyanik @KlausLoeffelmann Not sure who I should be tagging, I have split my large VB PRs to 4 small(er) PR's. This one is logically the most complex but has 100% test coverage of the change. The final #11867 is what I started a year ago is to replace WebClient with HTTP initially for download. It also adds Async NetworkDownload functions but they require an API review so they are not public. This last PR only changes a few files and adds SR strings, but it relies on everything before for test coverage. |
lonitra
left a comment
There was a problem hiding this comment.
Couple comments, otherwise generally LGTM
|
|
||
| Imports System.Runtime.CompilerServices | ||
|
|
||
| <Assembly: InternalsVisibleTo("Microsoft.VisualBasic.Forms.Tests, PublicKey=002400000480000094000000060200000024000052534131000400000100010007d1fa57c4aed9f0a32e84aa0faefd0de9e8fd6aec8f87fb03766c834c99921eb23be79ad9d5dcc1dd9ad236132102900b723cf980957fc4e177108fc607774f29e8320e92ea05ece4e821c0a5efe8f1645c4c0c93c1ab99285d622caa652c1dfad63d745d6f2de5f17e5eaf0fc4963d261c8a12436518206dc093344d5ad293")> |
There was a problem hiding this comment.
If you mean the public key, it does not work without it. Possibly because there are 2 project namespaces with the same name, 1 in VB and another in C#. I opened an issue #11031 about this last year and it’s still open.
There was a problem hiding this comment.
I see, @KlausLoeffelmann could you provide guidance on whether this is the preferred approach for writing tests which need visibility?
…/TestUtilities/TestData/DualTimeZones.TimeZoneNames.vb Co-authored-by: Loni Tra <lonitra@microsoft.com>
|
@lonitra any more feedback? What are next steps? |
Fixes partial fix for #11215
Proposed changes
Remove Utils that conflicts with .Net Core but different in return values
Modernize ExceptionUtils, it was originally ported from VB-6 and much of it no longer is needed and its overly complicated.
Fix broken TimeTests
Customer Impact
None
Regression?
Risk
Test methodology
100% Test Coverage
Test environment(s)
Visual Studio 100% Coverage
Microsoft Reviewers: Open in CodeFlow