Enable 64-bit (out-of-process) integration tests #47870
Conversation
sharwell
force-pushed
the
enable-tests
branch
from
ca29060 to
3f892ce
Compare
sharwell
force-pushed
the
enable-tests
branch
from
e789ca9 to
9016991
Compare
sharwell
force-pushed
the
enable-tests
branch
from
5e41d9d to
fb9a9d6
Compare
This comment has been minimized.
This comment has been minimized.
src/EditorFeatures/Core.Wpf/InlineRename/InlineRenameService.cs
Outdated
Show resolved
Hide resolved
sharwell
force-pushed
the
enable-tests
branch
2 times, most recently
from
8edbb80 to
de1c265
Compare
This comment has been minimized.
This comment has been minimized.
sharwell
force-pushed
the
enable-tests
branch
from
de1c265 to
afe7677
Compare
sharwell
force-pushed
the
enable-tests
branch
3 times, most recently
from
0de183e to
840bcbc
Compare
sharwell
force-pushed
the
enable-tests
branch
from
ea96d4c to
4fae8b1
Compare
|
@tmat there are definitely bugs remaining but I got a clean integration test build on first try at this commit |
This method was corrected for 16.8 Preview 3.
sharwell
force-pushed
the
enable-tests
branch
from
a760513 to
8c3c5ae
Compare
| if (baseResult is ServiceDescriptor) | ||
| return baseResult; |
There was a problem hiding this comment.
📝 This path is always used by 16.8 Preview 3 and newer, so this override is effectively a NOP in current production. The rest of this method is an inlined version of the fixed code. I'll submit a follow-up pull request (#48153) to remove this method as soon as this pull request merges, and we can merge that follow-up as soon as roslyn-integration-CI is updated to 16.8 Preview 3 or newer.
| if (exception is OperationCanceledException) | ||
| { | ||
| // Wrap in InvalidOperationException since cancellation was not requested | ||
| throw new InvalidOperationException(exception.Message, exception); | ||
| } |
There was a problem hiding this comment.
📝 This should be removed for release. It's only used by OOP things, but it could cause a ReportUnlessCancelled to fail when it would otherwise continue.
| @@ -86,7 +87,7 @@ protected async ValueTask<T> RunServiceAsync<T>(Func<CancellationToken, ValueTas | |||
| protected async ValueTask RunServiceAsync(Func<CancellationToken, ValueTask> implementation, CancellationToken cancellationToken) | |||
| { | |||
| WorkspaceManager.SolutionAssetCache.UpdateLastActivityTime(); | |||
| using var _ = LinkToken(ref cancellationToken); | |||
There was a problem hiding this comment.
@tmat This was changing the cancellation token which was subsequently passed to ReportWithoutCrashUnlessCanceledAndPropagate.
There was a problem hiding this comment.
That was definitely intentional. The combined one is not used above.
There was a problem hiding this comment.
We want to cancel the processing of the service if either the client cancellation token is signaled or the client disconnects.
sharwell
force-pushed
the
enable-tests
branch
from
de1b48a to
b70ebdb
Compare
sharwell
force-pushed
the
enable-tests
branch
from
b70ebdb to
510428c
Compare
No description provided.