Skip to content

Commit 6f70f70

Browse files
CyrusNajmabadiCyrusNajmabadi
authored
Add unit tests for gladstone extension handling service. (#78034)
2 parents cc835bf + 77b5108 commit 6f70f70

File tree

9 files changed

+784
-54
lines changed

9 files changed

+784
-54
lines changed

‎src/Features/Core/Portable/Extensions/ExtensionFolder.cs‎

Lines changed: 12 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ private sealed class ExtensionFolder
2828
/// <summary>
2929
/// Lazily computed assembly loader for this particular folder.
3030
/// </summary>
31-
private readonly AsyncLazy<(IAnalyzerAssemblyLoaderInternal? assemblyLoader, Exception? extensionException)> _lazyAssemblyLoader;
31+
private readonly AsyncLazy<(IExtensionAssemblyLoader? assemblyLoader, Exception? extensionException)> _lazyAssemblyLoader;
3232

3333
/// <summary>
3434
/// Mapping from assembly file path to the handlers it contains. Should only be mutated while the <see
@@ -43,52 +43,16 @@ public ExtensionFolder(
4343
_extensionMessageHandlerService = extensionMessageHandlerService;
4444
_lazyAssemblyLoader = AsyncLazy.Create(cancellationToken =>
4545
{
46-
#if NET
47-
// These lines should always succeed. If they don't, they indicate a bug in our code that we want
48-
// to bubble out as it must be fixed.
49-
var analyzerAssemblyLoaderProvider = _extensionMessageHandlerService._solutionServices.GetRequiredService<IAnalyzerAssemblyLoaderProvider>();
50-
var analyzerAssemblyLoader = analyzerAssemblyLoaderProvider.CreateNewShadowCopyLoader();
51-
52-
// Catch exceptions here related to working with the file system. If we can't properly enumerate,
53-
// we want to report that back to the client, while not blocking the entire extension service.
54-
try
55-
{
56-
// Allow this assembly loader to load any dll in assemblyFolderPath.
57-
foreach (var dll in Directory.EnumerateFiles(assemblyFolderPath, "*.dll"))
58-
{
59-
cancellationToken.ThrowIfCancellationRequested();
60-
try
61-
{
62-
// Check if the file is a valid .NET assembly.
63-
AssemblyName.GetAssemblyName(dll);
64-
}
65-
catch
66-
{
67-
// The file is not a valid .NET assembly, skip it.
68-
continue;
69-
}
70-
71-
analyzerAssemblyLoader.AddDependencyLocation(dll);
72-
}
73-
74-
return ((IAnalyzerAssemblyLoaderInternal?)analyzerAssemblyLoader, (Exception?)null);
75-
}
76-
catch (Exception ex) when (ex is not OperationCanceledException)
77-
{
78-
// Capture any exceptions here to be reported back in CreateAssemblyHandlersAsync.
79-
return (null, ex);
80-
}
81-
#else
82-
return ((IAnalyzerAssemblyLoaderInternal?)null, (Exception?)null);
83-
#endif
46+
var analyzerAssemblyLoaderProvider = _extensionMessageHandlerService._solutionServices.GetRequiredService<IExtensionAssemblyLoaderProvider>();
47+
return analyzerAssemblyLoaderProvider.CreateNewShadowCopyLoader(assemblyFolderPath, cancellationToken);
8448
});
8549
}
8650

8751
public void Unload()
8852
{
8953
// Only if we've created the assembly loader do we need to do anything.
9054
_lazyAssemblyLoader.TryGetValue(out var tuple);
91-
tuple.assemblyLoader?.Dispose();
55+
tuple.assemblyLoader?.Unload();
9256
}
9357

9458
private async Task<AssemblyMessageHandlers> CreateAssemblyHandlersAsync(
@@ -109,8 +73,14 @@ private async Task<AssemblyMessageHandlers> CreateAssemblyHandlersAsync(
10973
}
11074

11175
var assembly = analyzerAssemblyLoader.LoadFromPath(assemblyFilePath);
112-
var factory = _extensionMessageHandlerService._customMessageHandlerFactory;
113-
Contract.ThrowIfNull(factory);
76+
var factory = _extensionMessageHandlerService._solutionServices.GetService<IExtensionMessageHandlerFactory>();
77+
if (factory is null)
78+
{
79+
return new(
80+
DocumentMessageHandlers: ImmutableDictionary<string, IExtensionMessageHandlerWrapper>.Empty,
81+
WorkspaceMessageHandlers: ImmutableDictionary<string, IExtensionMessageHandlerWrapper>.Empty,
82+
ExtensionException: null);
83+
}
11484

11585
// We're calling into code here to analyze the assembly at the specified file and to create handlers we
11686
// find within it. If this throws, then we will capture that exception and return it to the caller to

‎src/Features/Core/Portable/Extensions/ExtensionMessageHandlerService.cs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,10 @@ private readonly record struct AssemblyMessageHandlers(
2323
Exception? ExtensionException);
2424

2525
private sealed partial class ExtensionMessageHandlerService(
26-
SolutionServices solutionServices,
27-
IExtensionMessageHandlerFactory? customMessageHandlerFactory)
26+
SolutionServices solutionServices)
2827
: IExtensionMessageHandlerService
2928
{
3029
private readonly SolutionServices _solutionServices = solutionServices;
31-
private readonly IExtensionMessageHandlerFactory? _customMessageHandlerFactory = customMessageHandlerFactory;
3230

3331
/// <summary>
3432
/// Lock for <see cref="_folderPathToExtensionFolder"/>, <see cref="_cachedDocumentHandlers_useOnlyUnderLock"/>, and <see

‎src/Features/Core/Portable/Extensions/ExtensionMessageHandlerServiceFactory.cs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,9 @@ namespace Microsoft.CodeAnalysis.Extensions;
1212
[ExportWorkspaceServiceFactory(typeof(IExtensionMessageHandlerService)), Shared]
1313
[method: ImportingConstructor]
1414
[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
15-
internal sealed partial class ExtensionMessageHandlerServiceFactory(
16-
[Import(AllowDefault = true)] IExtensionMessageHandlerFactory? customMessageHandlerFactory)
15+
internal sealed partial class ExtensionMessageHandlerServiceFactory()
1716
: IWorkspaceServiceFactory
1817
{
1918
public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices)
20-
=> new ExtensionMessageHandlerService(workspaceServices.SolutionServices, customMessageHandlerFactory);
19+
=> new ExtensionMessageHandlerService(workspaceServices.SolutionServices);
2120
}

‎src/Features/Core/Portable/Extensions/IExtensionAssemblyLoaderProvider.cs‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System;
6+
using System.Composition;
7+
using System.IO;
8+
using System.Reflection;
9+
using System.Threading;
10+
using Microsoft.CodeAnalysis.Host;
11+
using Microsoft.CodeAnalysis.Host.Mef;
12+
13+
namespace Microsoft.CodeAnalysis.Extensions;
14+
15+
/// <summary>
16+
/// Abstraction around <see cref="IAnalyzerAssemblyLoaderProvider"/> so that we can mock that behavior in tests.
17+
/// </summary>
18+
internal interface IExtensionAssemblyLoaderProvider : IWorkspaceService
19+
{
20+
(IExtensionAssemblyLoader? assemblyLoader, Exception? extensionException) CreateNewShadowCopyLoader(
21+
string assemblyFolderPath, CancellationToken cancellationToken);
22+
}
23+
24+
/// <summary>
25+
/// Abstraction around <see cref="IAnalyzerAssemblyLoader"/> so that we can mock that behavior in tests.
26+
/// </summary>
27+
internal interface IExtensionAssemblyLoader
28+
{
29+
Assembly LoadFromPath(string assemblyFilePath);
30+
void Unload();
31+
}
32+
33+
[ExportWorkspaceServiceFactory(typeof(IExtensionAssemblyLoaderProvider)), Shared]
34+
[method: ImportingConstructor]
35+
[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
36+
internal sealed class DefaultExtensionAssemblyLoaderProviderFactory() : IWorkspaceServiceFactory
37+
{
38+
public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices)
39+
=> new DefaultExtensionAssemblyLoaderProvider(workspaceServices);
40+
41+
private sealed class DefaultExtensionAssemblyLoaderProvider(HostWorkspaceServices workspaceServices)
42+
: IExtensionAssemblyLoaderProvider
43+
{
44+
#pragma warning disable IDE0052 // Remove unread private members
45+
private readonly HostWorkspaceServices _workspaceServices = workspaceServices;
46+
#pragma warning restore IDE0052 // Remove unread private members
47+
48+
public (IExtensionAssemblyLoader? assemblyLoader, Exception? extensionException) CreateNewShadowCopyLoader(
49+
string assemblyFolderPath, CancellationToken cancellationToken)
50+
{
51+
cancellationToken.ThrowIfCancellationRequested();
52+
53+
#if NET
54+
// These lines should always succeed. If they don't, they indicate a bug in our code that we want
55+
// to bubble out as it must be fixed.
56+
var analyzerAssemblyLoaderProvider = _workspaceServices.GetRequiredService<IAnalyzerAssemblyLoaderProvider>();
57+
var analyzerAssemblyLoader = analyzerAssemblyLoaderProvider.CreateNewShadowCopyLoader();
58+
59+
// Catch exceptions here related to working with the file system. If we can't properly enumerate,
60+
// we want to report that back to the client, while not blocking the entire extension service.
61+
try
62+
{
63+
// Allow this assembly loader to load any dll in assemblyFolderPath.
64+
foreach (var dll in Directory.EnumerateFiles(assemblyFolderPath, "*.dll"))
65+
{
66+
cancellationToken.ThrowIfCancellationRequested();
67+
try
68+
{
69+
// Check if the file is a valid .NET assembly.
70+
AssemblyName.GetAssemblyName(dll);
71+
}
72+
catch
73+
{
74+
// The file is not a valid .NET assembly, skip it.
75+
continue;
76+
}
77+
78+
analyzerAssemblyLoader.AddDependencyLocation(dll);
79+
}
80+
81+
return (new DefaultExtensionAssemblyLoader(analyzerAssemblyLoader), null);
82+
}
83+
catch (Exception ex) when (ex is not OperationCanceledException)
84+
{
85+
// Capture any exceptions here to be reported back in CreateAssemblyHandlersAsync.
86+
return (null, ex);
87+
}
88+
#else
89+
return default;
90+
#endif
91+
}
92+
}
93+
94+
private sealed class DefaultExtensionAssemblyLoader(
95+
IAnalyzerAssemblyLoaderInternal assemblyLoader) : IExtensionAssemblyLoader
96+
{
97+
public void Unload() => assemblyLoader.Dispose();
98+
99+
public Assembly LoadFromPath(string assemblyFilePath)
100+
=> assemblyLoader.LoadFromPath(assemblyFilePath);
101+
}
102+
}

‎src/Features/Core/Portable/Extensions/IExtensionMessageHandlerFactory.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,14 @@
55
using System.Collections.Immutable;
66
using System.Reflection;
77
using System.Threading;
8+
using Microsoft.CodeAnalysis.Host;
89

910
namespace Microsoft.CodeAnalysis.Extensions;
1011

1112
/// <summary>
1213
/// Factory for creating instances of extension message handlers.
1314
/// </summary>
14-
internal interface IExtensionMessageHandlerFactory
15+
internal interface IExtensionMessageHandlerFactory : IWorkspaceService
1516
{
1617
/// <summary>
1718
/// Creates <see cref="IExtensionMessageHandlerWrapper{Solution}"/> instances for each

‎src/Tools/ExternalAccess/Extensions/Internal/ExtensionMessageHandlerFactory.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
namespace Microsoft.CodeAnalysis.Extensions;
1313

14-
[Export(typeof(IExtensionMessageHandlerFactory)), Shared]
14+
[ExportWorkspaceService(typeof(IExtensionMessageHandlerFactory)), Shared]
1515
[method: ImportingConstructor]
1616
[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
1717
internal sealed class ExtensionMessageHandlerFactory() : IExtensionMessageHandlerFactory

‎src/VisualStudio/Core/Test.Next/Services/ServiceHubServicesTests.cs‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,22 @@ namespace Roslyn.VisualStudio.Next.UnitTests.Remote;
3838

3939
[UseExportProvider]
4040
[Trait(Traits.Feature, Traits.Features.RemoteHost)]
41-
public sealed class ServiceHubServicesTests
41+
public sealed partial class ServiceHubServicesTests
4242
{
43-
private static TestWorkspace CreateWorkspace(Type[] additionalParts = null)
44-
=> new(composition: FeaturesTestCompositions.Features.WithTestHostParts(TestHost.OutOfProcess).AddParts(additionalParts));
43+
private static TestWorkspace CreateWorkspace(
44+
Type[] additionalParts = null,
45+
Type[] additionalRemoteParts = null)
46+
{
47+
var workspace = new TestWorkspace(composition: FeaturesTestCompositions.Features.WithTestHostParts(TestHost.OutOfProcess).AddParts(additionalParts));
48+
49+
if (additionalRemoteParts != null)
50+
{
51+
var clientProvider = (InProcRemoteHostClientProvider)workspace.Services.GetService<IRemoteHostClientProvider>();
52+
clientProvider.AdditionalRemoteParts = additionalRemoteParts;
53+
}
54+
55+
return workspace;
56+
}
4557

4658
[Fact]
4759
public async Task TestRemoteHostSynchronize()
@@ -1721,8 +1733,16 @@ private static Solution Populate(Solution solution)
17211733
return solution;
17221734
}
17231735

1724-
private static Solution AddProject(Solution solution, string language, string[] documents, string[] additionalDocuments, ProjectId[] p2pReferences)
1736+
private static Solution AddProject(
1737+
Solution solution,
1738+
string language,
1739+
string[] documents,
1740+
string[] additionalDocuments = null,
1741+
ProjectId[] p2pReferences = null)
17251742
{
1743+
additionalDocuments ??= [];
1744+
p2pReferences ??= [];
1745+
17261746
var projectName = $"Project{solution.ProjectIds.Count}";
17271747
var project = solution.AddProject(projectName, $"{projectName}.dll", language)
17281748
.AddMetadataReference(MetadataReference.CreateFromFile(typeof(object).Assembly.Location))

0 commit comments

Comments
 (0)