Skip to content

Fixed logging so it works correctly! #341

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

Merged
merged 7 commits into from
Aug 29, 2020
Merged

Fixed logging so it works correctly! #341

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
bc9c933
Fixed logging so it works correctly!
david-driscoll Aug 29, 2020
6bdf010
timing
david-driscoll Aug 29, 2020
ce82276
bump
david-driscoll Aug 29, 2020
85184b6
tests are still stalling on windows
david-driscoll Aug 29, 2020
ad68c5e
buffer work done test
david-driscoll Aug 29, 2020
b0d7822
Update client tests to skip windows too
david-driscoll Aug 29, 2020
6702b5f
minor fix
david-driscoll Aug 29, 2020
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
1 change: 1 addition & 0 deletions .build/Build.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ public partial class Solution : NukeBuild,

public Target Clean => _ => _.Inherit<ICanClean>(x => x.Clean);
public Target Restore => _ => _.Inherit<ICanRestoreWithDotNetCore>(x => x.CoreRestore);

public Target Test => _ => _.Inherit<ICanTestWithDotNetCore>(x => x.CoreTest);

public Target BuildVersion => _ => _.Inherit<IHaveBuildVersion>(x => x.BuildVersion)
Expand Down
2 changes: 0 additions & 2 deletions src/Dap.Testing/DebugAdapterProtocolTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ Action<DebugAdapterServerOptions> serverOptionsAction
.ConfigureLogging(
x => {
x.SetMinimumLevel(LogLevel.Trace);
x.Services.AddSingleton(TestOptions.ClientLoggerFactory);
}
)
.Services
Expand All @@ -65,7 +64,6 @@ Action<DebugAdapterServerOptions> serverOptionsAction
.ConfigureLogging(
x => {
x.SetMinimumLevel(LogLevel.Trace);
x.Services.AddSingleton(TestOptions.ServerLoggerFactory);
}
)
.Services
Expand Down
2 changes: 1 addition & 1 deletion src/Dap.Testing/DebugAdapterServerTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ protected virtual async Task<IDebugAdapterClient> InitializeClient(Action<DebugA
options
.WithInput(reader)
.WithOutput(writer)
.WithLoggerFactory(TestOptions.ClientLoggerFactory)
.ConfigureLogging(
x => {
x.SetMinimumLevel(LogLevel.Trace);
x.Services.AddSingleton(TestOptions.ClientLoggerFactory);
}
)
.Services
Expand Down
2 changes: 1 addition & 1 deletion src/JsonRpc.Testing/JsonRpcServerTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,14 @@ Action<JsonRpcServerOptions> serverOptionsAction
var clientTask = JsonRpcServer.From(
options => {
options
.WithLoggerFactory(TestOptions.ClientLoggerFactory)
.WithServices(
services => services
.AddTransient(typeof(IPipelineBehavior<,>), typeof(SettlePipeline<,>))
.AddSingleton(ClientEvents as IRequestSettler)
.AddLogging(
x => {
x.SetMinimumLevel(LogLevel.Trace);
x.Services.AddSingleton(TestOptions.ClientLoggerFactory);
}
)
);
Expand Down
2 changes: 2 additions & 0 deletions src/JsonRpc/JsonRpcServerOptionsBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.DependencyInjection.Extensions;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Abstractions;
using Nerdbank.Streams;

namespace OmniSharp.Extensions.JsonRpc
Expand Down Expand Up @@ -86,6 +87,7 @@ public T WithPipe(Pipe pipe)

public T WithLoggerFactory(ILoggerFactory loggerFactory)
{
if (loggerFactory == NullLoggerFactory.Instance) return (T) (object) this;
Services.RemoveAll(typeof(ILoggerFactory));
Services.AddSingleton(loggerFactory);
return (T) (object) this;
Expand Down
12 changes: 6 additions & 6 deletions src/JsonRpc/ResponseRouter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,31 +9,31 @@

namespace OmniSharp.Extensions.JsonRpc
{
public class ResponseRouter : IResponseRouter
internal class ResponseRouter : IResponseRouter
{
internal readonly IOutputHandler OutputHandler;
internal readonly Lazy<IOutputHandler> OutputHandler;
internal readonly ISerializer Serializer;
private readonly IHandlerTypeDescriptorProvider<IHandlerTypeDescriptor> _handlerTypeDescriptorProvider;

internal readonly ConcurrentDictionary<long, (string method, TaskCompletionSource<JToken> pendingTask)> Requests =
new ConcurrentDictionary<long, (string method, TaskCompletionSource<JToken> pendingTask)>();

public ResponseRouter(IOutputHandler outputHandler, ISerializer serializer, IHandlerTypeDescriptorProvider<IHandlerTypeDescriptor> handlerTypeDescriptorProvider)
public ResponseRouter(Lazy<IOutputHandler> outputHandler, ISerializer serializer, IHandlerTypeDescriptorProvider<IHandlerTypeDescriptor> handlerTypeDescriptorProvider)
{
OutputHandler = outputHandler;
Serializer = serializer;
_handlerTypeDescriptorProvider = handlerTypeDescriptorProvider;
}

public void SendNotification(string method) =>
OutputHandler.Send(
OutputHandler.Value.Send(
new OutgoingNotification {
Method = method
}
);

public void SendNotification<T>(string method, T @params) =>
OutputHandler.Send(
OutputHandler.Value.Send(
new OutgoingNotification {
Method = method,
Params = @params
Expand Down Expand Up @@ -81,7 +81,7 @@ public async Task<TResponse> Returning<TResponse>(CancellationToken cancellation

cancellationToken.ThrowIfCancellationRequested();

_router.OutputHandler.Send(
_router.OutputHandler.Value.Send(
new OutgoingRequest {
Method = _method,
Params = _params,
Expand Down
1 change: 0 additions & 1 deletion src/Protocol/LanguageProtocolRpcOptionsBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ public abstract class LanguageProtocolRpcOptionsBase<T> : JsonRpcServerOptionsBa
{
public LanguageProtocolRpcOptionsBase()
{
Services.AddLogging(builder => LoggingBuilderAction?.Invoke(builder));
WithAssemblies(typeof(LanguageProtocolRpcOptionsBase<>).Assembly);
}

Expand Down
7 changes: 4 additions & 3 deletions src/Server/Logging/LanguageServerLogger.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ namespace OmniSharp.Extensions.LanguageServer.Server
{
internal class LanguageServerLogger : ILogger
{
private readonly ILanguageServer _responseRouter;
private readonly ILanguageServerFacade _responseRouter;
private readonly string _categoryName;
private readonly Func<LogLevel> _logLevelGetter;

public LanguageServerLogger(ILanguageServer responseRouter, string categoryName, Func<LogLevel> logLevelGetter)
public LanguageServerLogger(ILanguageServerFacade responseRouter, string categoryName, Func<LogLevel> logLevelGetter)
{
_logLevelGetter = logLevelGetter;
_responseRouter = responseRouter;
Expand Down Expand Up @@ -68,7 +68,8 @@ private bool TryGetMessageType(LogLevel logLevel, out MessageType messageType)
return true;
case LogLevel.Debug:
case LogLevel.Trace:
messageType = MessageType.Info;
// TODO: Integrate with set trace?
messageType = MessageType.Log;
return true;
}

Expand Down
4 changes: 2 additions & 2 deletions src/Server/Logging/LanguageServerLoggerProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ namespace OmniSharp.Extensions.LanguageServer.Server
{
internal class LanguageServerLoggerProvider : ILoggerProvider
{
private readonly ILanguageServer _languageServer;
private readonly ILanguageServerFacade _languageServer;
private readonly LanguageServerLoggerSettings _settings;

public LanguageServerLoggerProvider(ILanguageServer languageServer, LanguageServerLoggerSettings settings)
public LanguageServerLoggerProvider(ILanguageServerFacade languageServer, LanguageServerLoggerSettings settings)
{
_languageServer = languageServer;
_settings = settings;
Expand Down
2 changes: 2 additions & 0 deletions src/Shared/LanguageProtocolServiceCollectionExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ internal static IContainer AddLanguageProtocolInternals<T>(this IContainer conta
throw new ArgumentException("Serializer is missing!", nameof(options));
}

options.Services.AddLogging(builder => options.LoggingBuilderAction?.Invoke(builder));

container = container.AddJsonRpcServerCore(options);
container.RegisterInstanceMany(new LspHandlerTypeDescriptorProvider(options.Assemblies), nonPublicServiceTypes: true);

Expand Down
17 changes: 9 additions & 8 deletions test/Client.Tests/ClientTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using OmniSharp.Extensions.LanguageServer.Protocol.Client.Capabilities;
using OmniSharp.Extensions.LanguageServer.Protocol.Document;
using OmniSharp.Extensions.LanguageServer.Protocol.Models;
using TestingUtils;
using Xunit;
using Xunit.Abstractions;
using Range = OmniSharp.Extensions.LanguageServer.Protocol.Models.Range;
Expand Down Expand Up @@ -37,7 +38,7 @@ public ClientTests(ITestOutputHelper testOutput)
/// <summary>
/// Ensure that the language client can successfully request Hover information.
/// </summary>
[Fact(DisplayName = "Language client can successfully request hover info")]
[FactWithSkipOn(SkipOnPlatform.Windows, DisplayName = "Language client can successfully request hover info")]
public async Task Hover_Success()
{
const int line = 5;
Expand Down Expand Up @@ -110,7 +111,7 @@ public async Task Hover_Success()
/// <summary>
/// Ensure that the language client can successfully request Completions.
/// </summary>
[Fact(DisplayName = "Language client can successfully request completions")]
[FactWithSkipOn(SkipOnPlatform.Windows, DisplayName = "Language client can successfully request completions")]
public async Task Completions_Success()
{
const int line = 5;
Expand Down Expand Up @@ -213,7 +214,7 @@ public async Task Completions_Success()
/// <summary>
/// Ensure that the language client can successfully request SignatureHelp.
/// </summary>
[Fact(DisplayName = "Language client can successfully request signature help")]
[FactWithSkipOn(SkipOnPlatform.Windows, DisplayName = "Language client can successfully request signature help")]
public async Task SignatureHelp_Success()
{
const int line = 5;
Expand Down Expand Up @@ -306,7 +307,7 @@ public async Task SignatureHelp_Success()
/// <summary>
/// Ensure that the language client can successfully request Definition.
/// </summary>
[Fact(DisplayName = "Language client can successfully request definition")]
[FactWithSkipOn(SkipOnPlatform.Windows, DisplayName = "Language client can successfully request definition")]
public async Task Definition_Success()
{
const int line = 5;
Expand Down Expand Up @@ -382,7 +383,7 @@ public async Task Definition_Success()
/// <summary>
/// Ensure that the language client can successfully request DocumentHighlight.
/// </summary>
[Fact(DisplayName = "Language client can successfully request document highlights")]
[FactWithSkipOn(SkipOnPlatform.Windows, DisplayName = "Language client can successfully request document highlights")]
public async Task DocumentHighlights_Success()
{
const int line = 5;
Expand Down Expand Up @@ -447,7 +448,7 @@ public async Task DocumentHighlights_Success()
/// <summary>
/// Ensure that the language client can successfully request DocumentHighlight.
/// </summary>
[Fact(DisplayName = "Language client can successfully request document symbols")]
[FactWithSkipOn(SkipOnPlatform.Windows, DisplayName = "Language client can successfully request document symbols")]
public async Task DocumentSymbols_DocumentSymbol_Success()
{
const int line = 5;
Expand Down Expand Up @@ -532,7 +533,7 @@ public async Task DocumentSymbols_DocumentSymbol_Success()
/// <summary>
/// Ensure that the language client can successfully request FoldingRanges.
/// </summary>
[Fact(DisplayName = "Language client can successfully request document folding ranges")]
[FactWithSkipOn(SkipOnPlatform.Windows, DisplayName = "Language client can successfully request document folding ranges")]
public async Task FoldingRanges_Success()
{
var expectedDocumentPath = AbsoluteDocumentPath;
Expand Down Expand Up @@ -592,7 +593,7 @@ public async Task FoldingRanges_Success()
/// <summary>
/// Ensure that the language client can successfully receive Diagnostics from the server.
/// </summary>
[Fact(DisplayName = "Language client can successfully receive diagnostics")]
[FactWithSkipOn(SkipOnPlatform.Windows, DisplayName = "Language client can successfully receive diagnostics")]
public async Task Diagnostics_Success()
{
var documentPath = AbsoluteDocumentPath;
Expand Down
6 changes: 3 additions & 3 deletions test/Dap.Tests/Integration/RecursiveResolutionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public async Task Server_Cannot_Be_Injected_Into_Handler_During_Creation_Using_I
result.And.ErrorName.Should().Be("UnableToResolveFromRegisteredServices");
}

[Theory]
[TheoryWithSkipOn(Skip = "appears to cause a deadlock")]
[InlineData(Side.Client)]
[InlineData(Side.Server)]
public async Task Server_Facade_Can_Be_Injected_Into_Handler_During_Creation_Using_Registration(Side side)
Expand All @@ -130,7 +130,7 @@ public async Task Server_Facade_Can_Be_Injected_Into_Handler_During_Creation_Usi
await a.Should().NotThrowAsync();
}

[Theory]
[TheoryWithSkipOn(Skip = "appears to cause a deadlock")]
[InlineData(Side.Client)]
[InlineData(Side.Server)]
public async Task Server_Facade_Can_Be_Injected_Into_Handler_During_Creation_Using_Description(Side side)
Expand All @@ -154,7 +154,7 @@ public async Task Server_Facade_Can_Be_Injected_Into_Handler_During_Creation_Usi
await a.Should().NotThrowAsync();
}

[Theory]
[TheoryWithSkipOn(Skip = "appears to cause a deadlock")]
[InlineData(Side.Client)]
[InlineData(Side.Server)]
public async Task Server_Facade_Can_Injected_Into_Handler_During_Creation_Using_Injection(Side side)
Expand Down
9 changes: 5 additions & 4 deletions test/JsonRpc.Tests/ResponseRouterTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Linq;
using System;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using FluentAssertions;
Expand All @@ -18,7 +19,7 @@ public class ResponseRouterTests
public async Task WorksWithResultType()
{
var outputHandler = Substitute.For<IOutputHandler>();
var router = new ResponseRouter(outputHandler, new JsonRpcSerializer(), new HandlerTypeDescriptorProvider(new [] { typeof(HandlerTypeDescriptorProvider).Assembly, typeof(HandlerResolverTests).Assembly }));
var router = new ResponseRouter(new Lazy<IOutputHandler>(() => outputHandler), new JsonRpcSerializer(), new HandlerTypeDescriptorProvider(new [] { typeof(HandlerTypeDescriptorProvider).Assembly, typeof(HandlerResolverTests).Assembly }));

outputHandler
.When(x => x.Send(Arg.Is<object>(x => x.GetType() == typeof(OutgoingRequest))))
Expand All @@ -42,7 +43,7 @@ public async Task WorksWithResultType()
public async Task WorksWithUnitType()
{
var outputHandler = Substitute.For<IOutputHandler>();
var router = new ResponseRouter(outputHandler, new JsonRpcSerializer(), new HandlerTypeDescriptorProvider(new [] { typeof(HandlerTypeDescriptorProvider).Assembly, typeof(HandlerResolverTests).Assembly }));
var router = new ResponseRouter(new Lazy<IOutputHandler>(() => outputHandler), new JsonRpcSerializer(), new HandlerTypeDescriptorProvider(new [] { typeof(HandlerTypeDescriptorProvider).Assembly, typeof(HandlerResolverTests).Assembly }));

outputHandler
.When(x => x.Send(Arg.Is<object>(x => x.GetType() == typeof(OutgoingRequest))))
Expand All @@ -63,7 +64,7 @@ public async Task WorksWithUnitType()
public async Task WorksWithNotification()
{
var outputHandler = Substitute.For<IOutputHandler>();
var router = new ResponseRouter(outputHandler, new JsonRpcSerializer(), new HandlerTypeDescriptorProvider(new [] { typeof(HandlerTypeDescriptorProvider).Assembly, typeof(HandlerResolverTests).Assembly }));
var router = new ResponseRouter(new Lazy<IOutputHandler>(() => outputHandler), new JsonRpcSerializer(), new HandlerTypeDescriptorProvider(new [] { typeof(HandlerTypeDescriptorProvider).Assembly, typeof(HandlerResolverTests).Assembly }));

router.SendNotification(new NotificationParams());

Expand Down
1 change: 1 addition & 0 deletions test/Lsp.Tests/Integration/DynamicRegistrationTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ public async Task Should_Register_Links_Dynamically_While_Server_Is_Running()
)
);

await WaitForRegistrationUpdate(client);
await WaitForRegistrationUpdate(client);
client.RegistrationManager.CurrentRegistrations.Should().Contain(
x =>
Expand Down
Loading