Skip to content

Add VB FileCleanupTestsBase plus dependent tests REVIEW FIRST - #11987

Merged
lonitra merged 167 commits into
dotnet:mainfrom
paul1956:VB-FileCleanupTestsBase-AddDependantTests
Sep 18, 2024
Merged

Add VB FileCleanupTestsBase plus dependent tests REVIEW FIRST#11987
lonitra merged 167 commits into
dotnet:mainfrom
paul1956:VB-FileCleanupTestsBase-AddDependantTests

Conversation

@paul1956

@paul1956 paul1956 commented Aug 26, 2024

Copy link
Copy Markdown
Contributor

Fixes #11031 by adding more tests

Proposed changes

Add more tests to VB Runtime
This also adds a new test class to provide cleaner file cleanup, that is different than what C# provides in FileCleanupTestBase, this class will get additional functionality in another PR.

Customer Impact

Improve code quality

Regression?

  • No

Risk

None just adds tests

Test methodology

  • Visual Studio Tests

Test environment(s)

  • Visual Studio
Microsoft Reviewers: Open in CodeFlow

@paul1956
paul1956 requested a review from a team as a code owner August 26, 2024 22:11
@paul1956 paul1956 changed the title VB file cleanup tests base plus dependent tests Review After #11982 Add VB FileCleanupTestsBase plus dependent tests Review After #11982 Aug 27, 2024
@paul1956 paul1956 changed the title Add VB FileCleanupTestsBase plus dependent tests REVIEW AFTER #12105 DRAFT Add VB FileCleanupTestsBase plus dependent tests REVIEW FIRST Sep 14, 2024
@paul1956
paul1956 marked this pull request as ready for review September 14, 2024 01:25
@dotnet-policy-service dotnet-policy-service Bot removed the draft draft PR label Sep 14, 2024
@Tanya-Solyanik Tanya-Solyanik added the waiting-author-feedback The team requires more information from the author label Sep 16, 2024
@dotnet-policy-service dotnet-policy-service Bot removed the waiting-author-feedback The team requires more information from the author label Sep 16, 2024
@paul1956

paul1956 commented Sep 16, 2024

Copy link
Copy Markdown
Contributor Author

@Tanya-Solyanik I also realized the name of the file and class for FileIoProxyTests is wrong, it should be FileSystemProxyTests. I renamed it.

/// Returns the <see href="https://learn.microsoft.com/windows/win32/api/winuser/nf-winuser-getkeyboardlayoutnamew">
/// Returns the
/// <see href="https://learn.microsoft.com/windows/win32/api/winuser/nf-winuser-getkeyboardlayoutnamew">
/// keyboard layout identifier</see> of the current input language.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this better follows the pattern of the other see tags in the source code:

    /// <summary>
    ///  Returns the
    ///  <see href="https://learn.microsoft.com/windows/win32/api/winuser/nf-winuser-getkeyboardlayoutnamew">
    ///  keyboard layout identifier
    ///  </see>
    ///  of the current input language.
    /// </summary>

@paul1956 paul1956 Sep 16, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@ricardobossan The will be addressed in a separate PR #12151 I opened. This one already has too many changes unrelated to the to FileCleanupTestBase. #12151 just addresses "See href" based on Main. That could be merged before or after this one.

@lonitra lonitra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM apart from small typo

…ccessibiltyExtensions.cs

Co-authored-by: Loni Tra <lonitra@microsoft.com>
lonitra
lonitra previously approved these changes Sep 16, 2024
@lonitra lonitra added the ready-to-merge PRs that are ready to merge but worth notifying the internal team. label Sep 16, 2024
@paul1956

Copy link
Copy Markdown
Contributor Author

@lonitra I will open a new issue on AccessibilityExtensions spelling.

@paul1956

Copy link
Copy Markdown
Contributor Author

@lonitra anything preventing merge? Do I still need to fix spelling issue here?

@lonitra lonitra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, thank you

@lonitra
lonitra merged commit 336362a into dotnet:main Sep 18, 2024
@dotnet-policy-service dotnet-policy-service Bot added this to the 10.0 Preview1 milestone Sep 18, 2024
@dotnet-policy-service dotnet-policy-service Bot removed the ready-to-merge PRs that are ready to merge but worth notifying the internal team. label Sep 18, 2024
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 19, 2024
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.

Testing WinForms VB Code Issue

5 participants