Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify unit testing api. #73241

Merged
merged 2 commits into from
Apr 26, 2024
Merged

Simplify unit testing api. #73241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a3ad10a
Remove no longer used api
CyrusNajmabadi Apr 18, 2024
034b224
Merge branch 'main' into removeUnitTestingApi
CyrusNajmabadi Apr 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 0 additions & 31 deletions ...e/ExternalAccess/UnitTesting/SolutionCrawler/DefaultUnitTestingDocumentTrackingService.cs
View file
Open in desktop

This file was deleted.

30 changes: 0 additions & 30 deletions ...ortable/ExternalAccess/UnitTesting/SolutionCrawler/IUnitTestingDocumentTrackingService.cs
View file
Open in desktop

This file was deleted.

36 changes: 0 additions & 36 deletions ...ternalAccess/UnitTesting/SolutionCrawler/IUnitTestingDocumentTrackingServiceExtensions.cs
View file
Open in desktop

This file was deleted.

26 changes: 0 additions & 26 deletions ...esting/SolutionCrawler/UnitTestingWorkCoordinator.AbstractUnitTestingPriorityProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ public AbstractUnitTestingPriorityProcessor(
_lazyAnalyzers = lazyAnalyzers;

Processor = processor;
Processor._documentTracker.NonRoslynBufferTextChanged += OnNonRoslynBufferTextChanged;
Copy link
Member Author

Choose a reason for hiding this comment

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

no point registering for events that never fire.

}

public ImmutableArray<IUnitTestingIncrementalAnalyzer> Analyzers
Expand Down Expand Up @@ -117,31 +116,6 @@ protected async Task WaitForHigherPriorityOperationsAsync()
}
}
}

public override void Shutdown()
{
base.Shutdown();

Processor._documentTracker.NonRoslynBufferTextChanged -= OnNonRoslynBufferTextChanged;
Copy link
Member Author

Choose a reason for hiding this comment

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

no need to unregister, so this override went away.

}

private void OnNonRoslynBufferTextChanged(object? sender, EventArgs e)
{
// There are 2 things incremental processor takes care of
//
// #1 is making sure we delay processing any work until there is enough idle (ex, typing) in host.
// #2 is managing cancellation and pending works.
//
// we used to do #1 and #2 only for Roslyn files. and that is usually fine since most of time solution contains only roslyn files.
//
// but for mixed solution (ex, Roslyn files + HTML + JS + CSS), #2 still makes sense but #1 doesn't. We want
// to pause any work while something is going on in other project types as well.
//
// we need to make sure we play nice with neighbors as well.
//
// now, we don't care where changes are coming from. if there is any change in host, we pause ourselves for a while.
UpdateLastAccessTime();
}
}
}
}
Expand Down
3 changes: 0 additions & 3 deletions ...ing/SolutionCrawler/UnitTestingWorkCoordinator.UnitTestingIncrementalAnalyzerProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ private partial class UnitTestingIncrementalAnalyzerProcessor

private readonly UnitTestingRegistration _registration;
private readonly IAsynchronousOperationListener _listener;
private readonly IUnitTestingDocumentTrackingService _documentTracker;

private readonly UnitTestingNormalPriorityProcessor _normalPriorityProcessor;
private readonly UnitTestingLowPriorityProcessor _lowPriorityProcessor;
Expand Down Expand Up @@ -58,8 +57,6 @@ public UnitTestingIncrementalAnalyzerProcessor(
var lazyAllAnalyzers = new Lazy<ImmutableArray<IUnitTestingIncrementalAnalyzer>>(() => GetIncrementalAnalyzers(_registration, analyzersGetter, onlyHighPriorityAnalyzer: false));

// event and worker queues
_documentTracker = _registration.Services.GetRequiredService<IUnitTestingDocumentTrackingService>();

var globalNotificationService = _registration.Services.ExportProvider.GetExports<IGlobalOperationNotificationService>().FirstOrDefault()?.Value;

_normalPriorityProcessor = new UnitTestingNormalPriorityProcessor(listener, this, lazyAllAnalyzers, globalNotificationService, normalBackOffTimeSpan, shutdownToken);
Expand Down
7 changes: 1 addition & 6 deletions ...UnitTesting/SolutionCrawler/UnitTestingWorkCoordinator.UnitTestingLowPriorityProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,8 @@ protected override async Task ExecuteAsync()
// we wait for global operation, higher and normal priority processor to finish its working
await WaitForHigherPriorityOperationsAsync().ConfigureAwait(false);

// process any available project work, preferring the active project.
var preferableProjectId = Processor._documentTracker.SupportsDocumentTracking
? Processor._documentTracker.TryGetActiveDocument()?.ProjectId
: null;
Copy link
Member Author

Choose a reason for hiding this comment

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

SupportsDocTracking was always fall, so this was always null.


if (_workItemQueue.TryTakeAnyWork(
preferableProjectId,
preferableProjectId: null,
out var workItem, out var projectCancellation))
{
await ProcessProjectAsync(Analyzers, workItem, projectCancellation).ConfigureAwait(false);
Expand Down
77 changes: 0 additions & 77 deletions ...tTesting/SolutionCrawler/UnitTestingWorkCoordinator.UnitTestingNormalPriorityProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Copy link
Member Author

Choose a reason for hiding this comment

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

no concep of priority, so removed entirely.

{
// successfully processed a high priority document.
return;
}

// process one of documents remaining
if (!_workItemQueue.TryTakeAnyWork(
_currentProjectProcessing,
Expand Down Expand Up @@ -175,77 +169,6 @@ private void SetProjectProcessing(ProjectId currentProject)
_currentProjectProcessing = currentProject;
}

private IEnumerable<DocumentId> GetPrioritizedPendingDocuments()
{
// First the active document
var activeDocumentId = Processor._documentTracker.TryGetActiveDocument();
if (activeDocumentId != null)
{
yield return activeDocumentId;
}

// Now any visible documents
foreach (var visibleDocumentId in Processor._documentTracker.GetVisibleDocuments())
{
yield return visibleDocumentId;
}

// Any other high priority documents
foreach (var (documentId, _) in _higherPriorityDocumentsNotProcessed)
{
yield return documentId;
}
}

private async Task<bool> TryProcessOneHigherPriorityDocumentAsync()
{
try
{
if (!Processor._documentTracker.SupportsDocumentTracking)
{
return false;
Copy link
Member Author

Choose a reason for hiding this comment

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

this always returned false.

}

foreach (var documentId in GetPrioritizedPendingDocuments())
{
if (CancellationToken.IsCancellationRequested)
{
return true;
}

// this is a best effort algorithm with some shortcomings.
//
// the most obvious issue is if there is a new work item (without a solution change - but very unlikely)
// for a opened document we already processed, the work item will be treated as a regular one rather than higher priority one
// (opened document)
// see whether we have work item for the document
if (!_workItemQueue.TryTake(documentId, out var workItem, out var documentCancellation))
{
RemoveHigherPriorityDocument(documentId);
continue;
}

// okay now we have work to do
await ProcessDocumentAsync(Analyzers, workItem, documentCancellation).ConfigureAwait(false);

RemoveHigherPriorityDocument(documentId);
return true;
}

return false;
}
catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e))
{
throw ExceptionUtilities.Unreachable();
}
}

private void RemoveHigherPriorityDocument(DocumentId documentId)
{
// remove opened document processed
_higherPriorityDocumentsNotProcessed.TryRemove(documentId, out _);
}

private async Task ProcessDocumentAsync(ImmutableArray<IUnitTestingIncrementalAnalyzer> analyzers, UnitTestingWorkItem workItem, CancellationToken cancellationToken)
{
Contract.ThrowIfNull(workItem.DocumentId);
Expand Down
Loading