Skip to content

Remove dependency on RemoteExecutor on some files#12379

Closed
ricardobossan wants to merge 1 commit into
dotnet:mainfrom
ricardobossan:Issue_4500_Remove_Dependency_On_RemoteExecutor_In_Tests
Closed

Remove dependency on RemoteExecutor on some files#12379
ricardobossan wants to merge 1 commit into
dotnet:mainfrom
ricardobossan:Issue_4500_Remove_Dependency_On_RemoteExecutor_In_Tests

Conversation

@ricardobossan
Copy link
Copy Markdown
Member

@ricardobossan ricardobossan commented Oct 24, 2024 •
edited by dotnet-policy-service Bot
Loading

Fixes #

Proposed changes

Customer Impact

Regression?

  • Yes / No

Risk

Screenshots

Before

After

Test methodology

Accessibility testing

Test environment(s)

Microsoft Reviewers: Open in CodeFlow

@ricardobossan ricardobossan self-assigned this Oct 24, 2024
@dotnet-policy-service dotnet-policy-service Bot added the draft draft PR label Oct 24, 2024
@ricardobossan ricardobossan force-pushed the Issue_4500_Remove_Dependency_On_RemoteExecutor_In_Tests branch from e3d91a7 to 4b05a67 Compare December 31, 2024 00:37
@ricardobossan ricardobossan force-pushed the Issue_4500_Remove_Dependency_On_RemoteExecutor_In_Tests branch from 4b05a67 to acddbd1 Compare January 7, 2025 19:49
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 7, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.12353%. Comparing base (0ce1218) to head (4612a1a).
Report is 18 commits behind head on main.

Additional details and impacted files 
@@                 Coverage Diff                 @@
##                main      #12379         +/-   ##
===================================================
+ Coverage   76.03858%   76.12353%   +0.08495%     
===================================================
  Files           3181        3187          +6     
  Lines         639670      639990        +320     
  Branches       47215       47229         +14     
===================================================
+ Hits          486396      487183        +787     
+ Misses        149756      149269        -487     
- Partials        3518        3538         +20
Flag Coverage Δ
Debug 76.12353% <100.00000%> (+0.08495%) ⬆️
integration 18.16929% <ø> (-0.00311%) ⬇️
production 49.99701% <ø> (+0.16343%) ⬆️
test 97.03652% <100.00000%> (+0.00454%) ⬆️
unit 47.25564% <ø> (+0.20114%) ⬆️

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

@Tanya-Solyanik Tanya-Solyanik added the waiting-author-feedback The team requires more information from the author label Jan 7, 2025
@ricardobossan ricardobossan force-pushed the Issue_4500_Remove_Dependency_On_RemoteExecutor_In_Tests branch from acddbd1 to dd8fb7a Compare January 7, 2025 21:26
@dotnet-policy-service dotnet-policy-service Bot removed the waiting-author-feedback The team requires more information from the author label Jan 7, 2025
@ricardobossan ricardobossan force-pushed the Issue_4500_Remove_Dependency_On_RemoteExecutor_In_Tests branch 2 times, most recently from 22a7e71 to db59ce0 Compare January 9, 2025 00:57
@ricardobossan
Copy link
Copy Markdown
Member Author

ricardobossan commented Jan 10, 2025

/azp run

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines Bot commented Jan 10, 2025

Azure Pipelines successfully started running 1 pipeline(s).

@ricardobossan ricardobossan force-pushed the Issue_4500_Remove_Dependency_On_RemoteExecutor_In_Tests branch from db59ce0 to b2d32e2 Compare January 16, 2025 20:12
Tanya-Solyanik
Tanya-Solyanik reviewed Jan 16, 2025
View reviewed changes
Comment thread src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/Application.ParkingWindowTests.cs Outdated

// verify the remote process succeeded
Assert.Equal(RemoteExecutor.SuccessExitCode, invokerHandle.ExitCode);
Control.CheckForIllegalCrossThreadCalls = false;
Copy link
Copy Markdown
Contributor

@Tanya-Solyanik Tanya-Solyanik Jan 16, 2025

Choose a reason for hiding this comment

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

This should be in a try/finally block

@ricardobossan ricardobossan force-pushed the Issue_4500_Remove_Dependency_On_RemoteExecutor_In_Tests branch 3 times, most recently from 8a9dea8 to ee214ee Compare January 17, 2025 23:58
@ricardobossan ricardobossan force-pushed the Issue_4500_Remove_Dependency_On_RemoteExecutor_In_Tests branch from ee214ee to b440f5d Compare January 18, 2025 20:58
@ricardobossan
Copy link
Copy Markdown
Member Author

ricardobossan commented Jan 18, 2025

@Tanya-Solyanik I have re-enabled all tests that were previously skipped with the explanation:
Crash with AbandonedMutexException. See: https://github.com/dotnet/arcade/issues/5325.

Additionally, I restored the correct state of the Application.ParkingWindowTests.cs method by setting:
Control.CheckForIllegalCrossThreadCalls = true;.

Summary of Attempts:

  • Approach 1:
    Removed RemoteExecutor only from Application.ParkingWindowTests.cs while simply re-enabling the other tests.

  • Approach 2:
    Removed RemoteExecutor from all tests.

Both approaches passed local tests but failed automated CI checks.

For now, I’m pausing work on this issue to focus on higher-priority tasks. Let me know if you have any suggestions or would like me to revisit this sooner!

@ricardobossan ricardobossan mentioned this pull request Jan 18, 2025
Open
@Tanya-Solyanik
Copy link
Copy Markdown
Contributor

Tanya-Solyanik commented Jan 21, 2025

We'll re-open when we are working on this again.

@github-actions github-actions Bot locked and limited conversation to collaborators Feb 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@Tanya-Solyanik Tanya-Solyanik Tanya-Solyanik left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@ricardobossan ricardobossan

Labels

draft draft PR

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ricardobossan @Tanya-Solyanik