Simplify unit testing api. #73241
Simplify unit testing api. #73241
Conversation
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
|
|
||
| [ExportWorkspaceService(typeof(IUnitTestingDocumentTrackingService), ServiceLayer.Default)] | ||
| [Shared] | ||
| internal sealed class DefaultUnitTestingDocumentTrackingService : IUnitTestingDocumentTrackingService |
There was a problem hiding this comment.
the IUnitTestingDocumentTrackingService only ever had a single, default, no-op impl (this type). Given that, it means this type+interface was utterly pointless. I removed it and unwound accordingly.
|
|
||
| namespace Microsoft.CodeAnalysis.ExternalAccess.UnitTesting.SolutionCrawler; | ||
|
|
||
| internal static class IUnitTestingDocumentTrackingServiceExtensions |
There was a problem hiding this comment.
pointless extension since the underlying interface always returned null/empty
| @@ -37,7 +37,6 @@ private abstract class AbstractUnitTestingPriorityProcessor : UnitTestingGlobalO | |||
| _lazyAnalyzers = lazyAnalyzers; | |||
|
|
|||
| Processor = processor; | |||
| Processor._documentTracker.NonRoslynBufferTextChanged += OnNonRoslynBufferTextChanged; | |||
There was a problem hiding this comment.
no point registering for events that never fire.
| { | ||
| base.Shutdown(); | ||
|
|
||
| Processor._documentTracker.NonRoslynBufferTextChanged -= OnNonRoslynBufferTextChanged; |
There was a problem hiding this comment.
no need to unregister, so this override went away.
| // process any available project work, preferring the active project. | ||
| var preferableProjectId = Processor._documentTracker.SupportsDocumentTracking | ||
| ? Processor._documentTracker.TryGetActiveDocument()?.ProjectId | ||
| : null; |
There was a problem hiding this comment.
SupportsDocTracking was always fall, so this was always null.
| @@ -120,12 +120,6 @@ protected override async Task ExecuteAsync() | |||
| // okay, there must be at least one item in the map | |||
| ResetStates(); | |||
|
|
|||
| if (await TryProcessOneHigherPriorityDocumentAsync().ConfigureAwait(false)) | |||
There was a problem hiding this comment.
no concep of priority, so removed entirely.
| { | ||
| if (!Processor._documentTracker.SupportsDocumentTracking) | ||
| { | ||
| return false; |
There was a problem hiding this comment.
this always returned false.
CyrusNajmabadi
changed the title
|
@shyamnamboodiripad Just a heads up about this :) This just simplifies the amount of forked code we're maintaining. |
Thanks for the heads up @CyrusNajmabadi 👍🏾 I don't foresee any problems based on a quick look. However, I have not been working too closely on the unit testing side recently so also tagging @AbhitejJohn and @y87feng as an FYI in case we notice any problems in feedback / triage. |
|
@shyamnamboodiripad : Do you know what components we'd want to validate here? We'd want to loop in Artur if Live Unit Testing is involved as well. |
|
@AbhitejJohn this is the system that sends you the stream of events you use to perform live test discovery. Note: all the above code is literally a no-op. It did nothing (it literally returned 'false' for a value, that made all of the following code do nothing). So really in terms of validation, it just needs to be: does live test discovery still work? |
|
gotcha. thanks @CyrusNajmabadi . Is there a val build we could use to try this out? @y87feng can help coordinate a validation. |
No description provided.