Modernize VB ExceptionUtils add Test Coverage, fix Time Tests REVIEW 1st #11982

paul1956 · 2024-08-24T23:07:51Z

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?

Time Tests were broken and failing silently

Risk

Minimal 100% Test coverage of replaced code

Test methodology

100% Test Coverage

Test environment(s)

Visual Studio 100% Coverage

Microsoft Reviewers: Open in CodeFlow

Add Tests for ExceptionUtils

codecov · 2024-08-24T23:40:10Z

Codecov Report

Attention: Patch coverage is 67.07819% with 80 lines in your changes missing coverage. Please review.

Project coverage is 75.19972%. Comparing base (463173d) to head (a831f48).
Report is 13 commits behind head on main.

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

Flag	Coverage Δ
Debug	`75.19972% <67.07819%> (+0.01067%)`	⬆️
integration	`18.02949% <11.85185%> (-0.01097%)`	⬇️
production	`48.44499% <42.96296%> (+0.01855%)`	⬆️
test	`97.02183% <97.22222%> (-0.00010%)`	⬇️
unit	`45.45560% <40.74074%> (+0.05883%)`	⬆️

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

Remove unneeded Paentheses Cleanup spacing Sort UnsafeNativeMethods

…ionAnalyzerTest

paul1956 · 2024-08-27T02:04:00Z

@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 following two #11987 which adds test infrastructure (in response to a closed PR) and #11863 both add tests

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

Couple comments, otherwise generally LGTM

lonitra · 2024-09-05T16:19:46Z

src/Microsoft.VisualBasic.Forms/src/Microsoft/Properties/AssemblyInfo.vb

+
+Imports System.Runtime.CompilerServices
+
+<Assembly: InternalsVisibleTo("Microsoft.VisualBasic.Forms.Tests, PublicKey=002400000480000094000000060200000024000052534131000400000100010007d1fa57c4aed9f0a32e84aa0faefd0de9e8fd6aec8f87fb03766c834c99921eb23be79ad9d5dcc1dd9ad236132102900b723cf980957fc4e177108fc607774f29e8320e92ea05ece4e821c0a5efe8f1645c4c0c93c1ab99285d622caa652c1dfad63d745d6f2de5f17e5eaf0fc4963d261c8a12436518206dc093344d5ad293")>


Why is this needed?

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.

I see, @KlausLoeffelmann could you provide guidance on whether this is the preferred approach for writing tests which need visibility?

src/Microsoft.VisualBasic.Forms/tests/UnitTests/System/Windows/Forms/TimeTests.vb

...c.Forms/tests/UnitTests/System/Windows/TestUtilities/TestData/DualTimeZones.TimeZoneNames.vb

...soft.VisualBasic.Forms/tests/UnitTests/System/Windows/TestUtilities/TestData/TimeTestData.vb

…/TestUtilities/TestData/DualTimeZones.TimeZoneNames.vb Co-authored-by: Loni Tra <[email protected]>

paul1956 · 2024-09-10T10:26:08Z

@lonitra any more feedback? What are next steps?

lonitra

LGTM

paul1956 added 4 commits August 24, 2024 14:41

Modernize ExceptionUtils

f2d1dec

Fix broken Time Tests

8d1e1b4

Fix broken Time Tests

9ea27ee

Add Tests for ExceptionUtils

Fix GetArgumentExceptionWithArgNameTest_Succeed

41ad20a

paul1956 requested a review from a team as a code owner August 24, 2024 23:07

dotnet-policy-service bot assigned paul1956 Aug 24, 2024

paul1956 changed the title ~~Modernize ExceptionUtil,s fix Time Tests REVIEW 1st~~ Modernize ExceptionUtils fix Time Tests REVIEW 1st Aug 24, 2024

paul1956 changed the title ~~Modernize ExceptionUtils fix Time Tests REVIEW 1st~~ Modernize ExceptionUtils fix, Time Tests REVIEW 1st Aug 24, 2024

paul1956 changed the title ~~Modernize ExceptionUtils fix, Time Tests REVIEW 1st~~ Modernize ExceptionUtils, fix Time Tests REVIEW 1st Aug 25, 2024

paul1956 changed the title ~~Modernize ExceptionUtils, fix Time Tests REVIEW 1st~~ Modernize ExceptionUtils add Test Coverage, fix Time Tests REVIEW 1st Aug 25, 2024

paul1956 added 6 commits August 24, 2024 17:34

Apply using to FileLogTraceListener

18ff68f

Merge branch 'master' into Modernize-ExceptionUtils

b792c71

Remove usage of CompilerServices.Utils

f978352

Remove unneeded Paentheses Cleanup spacing Sort UnsafeNativeMethods

Sort Imports in Interaction.VB

8a328d5

Remove space on blank lines in MissingPropertySerializationConfigurat…

226de1d

…ionAnalyzerTest

Merge Master

1158c07

paul1956 changed the title ~~Modernize ExceptionUtils add Test Coverage, fix Time Tests REVIEW 1st~~ Modernize VB ExceptionUtils add Test Coverage, fix Time Tests REVIEW 1st Aug 27, 2024

paul1956 added 11 commits August 27, 2024 02:00

Add Collection("Sequential") Attribute to ControlTests

8d1455e

Remove Collection("Sequential") from ControlTests

03f815f

Rename ControlTests.vb to ControlInvokeTests.vb

5440751

Merge branch 'master' into Modernize-ExceptionUtils

46c55b2

Merge branch 'master' into Modernize-ExceptionUtils

f06c19c

Fix casing in MissingPropertySerializationConfigurationAnalyzerTest

c93df87

Fix comment is ControlInvokeTests

4963673

Merge branch 'master' into Modernize-ExceptionUtils

9f3335d

Fix Screen to be System.Windows.Forms.Screen

75fbba5

Clean up Computer Screen

f86d3d8

Merge branch 'master' into Modernize-ExceptionUtils

cb3fb7b

lonitra reviewed Sep 5, 2024

View reviewed changes

lonitra added the waiting-author-feedback The team requires more information from the author label Sep 5, 2024

dotnet-policy-service bot removed the waiting-author-feedback The team requires more information from the author label Sep 5, 2024

lonitra added the waiting-author-feedback The team requires more information from the author label Sep 5, 2024

dotnet-policy-service bot removed the waiting-author-feedback The team requires more information from the author label Sep 5, 2024

paul1956 added 9 commits September 5, 2024 15:30

Respond to PR feedback

5c2e5c8

Merge branch 'master' into Modernize-ExceptionUtils

5dfec82

Correct Test Name

a66e137

Improve Time tests

0eb12aa

Wrap long lines

369729b

Shorted long lines in FileSystemProxy

e275c6c

Sort FileSystemProxy

8072238

Minor cleanup

308a127

Merge branch 'master' into Modernize-ExceptionUtils

d44d99c

lonitra reviewed Sep 6, 2024

View reviewed changes

...c.Forms/tests/UnitTests/System/Windows/TestUtilities/TestData/DualTimeZones.TimeZoneNames.vb Outdated Show resolved Hide resolved

...soft.VisualBasic.Forms/tests/UnitTests/System/Windows/TestUtilities/TestData/TimeTestData.vb Show resolved Hide resolved

lonitra added the waiting-author-feedback The team requires more information from the author label Sep 6, 2024

Update src/Microsoft.VisualBasic.Forms/tests/UnitTests/System/Windows…

d9c7312

…/TestUtilities/TestData/DualTimeZones.TimeZoneNames.vb Co-authored-by: Loni Tra <[email protected]>

dotnet-policy-service bot removed the waiting-author-feedback The team requires more information from the author label Sep 6, 2024

paul1956 added 4 commits September 6, 2024 16:16

Respond to PR Feedback

081d039

Fix Merge Conflict

d8e68b5

Merge branch 'master' into Modernize-ExceptionUtils

246f911

Merge branch 'master' into Modernize-ExceptionUtils

3ff50cc

Revert TimeTestData based on PR Feedback

a831f48

lonitra approved these changes Sep 10, 2024

View reviewed changes

lonitra added the ready-to-merge PRs that are ready to merge but worth notifying the internal team. label Sep 10, 2024

lonitra merged commit ce8b46f into dotnet:main Sep 11, 2024

dotnet-policy-service bot added this to the 10.0 Preview1 milestone Sep 11, 2024

dotnet-policy-service bot removed the ready-to-merge PRs that are ready to merge but worth notifying the internal team. label Sep 11, 2024

github-actions bot locked and limited conversation to collaborators Oct 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Modernize VB ExceptionUtils add Test Coverage, fix Time Tests REVIEW 1st #11982

Modernize VB ExceptionUtils add Test Coverage, fix Time Tests REVIEW 1st #11982

Uh oh!

paul1956 commented Aug 24, 2024 •

edited

Loading

Uh oh!

codecov bot commented Aug 24, 2024 •

edited

Loading

Uh oh!

paul1956 commented Aug 27, 2024 •

edited

Loading

Uh oh!

lonitra left a comment

Uh oh!

lonitra Sep 5, 2024

Uh oh!

paul1956 Sep 5, 2024 •

edited

Loading

Uh oh!

lonitra Sep 5, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

paul1956 commented Sep 10, 2024

Uh oh!

lonitra left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		Imports System.Runtime.CompilerServices

		<Assembly: InternalsVisibleTo("Microsoft.VisualBasic.Forms.Tests, PublicKey=002400000480000094000000060200000024000052534131000400000100010007d1fa57c4aed9f0a32e84aa0faefd0de9e8fd6aec8f87fb03766c834c99921eb23be79ad9d5dcc1dd9ad236132102900b723cf980957fc4e177108fc607774f29e8320e92ea05ece4e821c0a5efe8f1645c4c0c93c1ab99285d622caa652c1dfad63d745d6f2de5f17e5eaf0fc4963d261c8a12436518206dc093344d5ad293")>

Modernize VB ExceptionUtils add Test Coverage, fix Time Tests REVIEW 1st #11982

Modernize VB ExceptionUtils add Test Coverage, fix Time Tests REVIEW 1st #11982

Uh oh!

Conversation

paul1956 commented Aug 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Customer Impact

Regression?

Risk

Test methodology

Test environment(s)

Microsoft Reviewers: Open in CodeFlow

Uh oh!

codecov bot commented Aug 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

paul1956 commented Aug 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lonitra left a comment

Choose a reason for hiding this comment

Uh oh!

lonitra Sep 5, 2024

Choose a reason for hiding this comment

Uh oh!

paul1956 Sep 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lonitra Sep 5, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

paul1956 commented Sep 10, 2024

Uh oh!

lonitra left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

paul1956 commented Aug 24, 2024 •

edited

Loading

codecov bot commented Aug 24, 2024 •

edited

Loading

paul1956 commented Aug 27, 2024 •

edited

Loading

paul1956 Sep 5, 2024 •

edited

Loading