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

tracking std out prints #343

Merged
merged 11 commits into from
Jul 23, 2019
Merged

tracking std out prints #343

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
51827f1
tracking std out prints
colombod Jul 23, 2019
2e5a772
transient display ids must be emitted per value
colombod Jul 23, 2019
f44251d
handle last value produce correctly
colombod Jul 23, 2019
117ee6c
remove kernel name from csharpkernel tests
colombod Jul 23, 2019
24e4b9f
mime type not null
colombod Jul 23, 2019
3d4dcdf
Merge branch 'master' into feature/console_writeline
colombod Jul 23, 2019
99be41c
rename methods
colombod Jul 23, 2019
03f2fe1
use const for DefaultMimeType and remove redundant checks
colombod Jul 23, 2019
67aba14
Merge branch 'master' into feature/console_writeline
colombod Jul 23, 2019
69fd2d7
white space
colombod Jul 23, 2019
a5be738
empty or white space string are not valid mimetypes
colombod Jul 23, 2019
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
3 changes: 1 addition & 2 deletions Microsoft.DotNet.Interactive.Jupyter/CompleteRequestHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
using Microsoft.DotNet.Interactive.Commands;
using Microsoft.DotNet.Interactive.Events;
using Microsoft.DotNet.Interactive.Jupyter.Protocol;
using WorkspaceServer.Kernel;

namespace Microsoft.DotNet.Interactive.Jupyter
{
Expand All @@ -31,7 +30,7 @@ public async Task Handle(JupyterRequestContext context)

var command = new RequestCompletion(completeRequest.Code, completeRequest.CursorPosition);

var openRequest = new InflightRequest(context, completeRequest, 0, null);
var openRequest = new InflightRequest(context, completeRequest, 0);

InFlightRequests[command] = openRequest;

Expand Down
23 changes: 17 additions & 6 deletions Microsoft.DotNet.Interactive.Jupyter/ExecuteRequestHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
using Microsoft.DotNet.Interactive.Commands;
using Microsoft.DotNet.Interactive.Events;
using Microsoft.DotNet.Interactive.Jupyter.Protocol;
using WorkspaceServer.Kernel;

namespace Microsoft.DotNet.Interactive.Jupyter
{
Expand All @@ -30,9 +29,8 @@ public async Task Handle(JupyterRequestContext context)
var executionCount = executeRequest.Silent ? _executionCount : Interlocked.Increment(ref _executionCount);

var command = new SubmitCode(executeRequest.Code, "csharp");
var id = Guid.NewGuid();
var transient = new Dictionary<string, object> { { "display_id", id.ToString() } };
var openRequest = new InflightRequest(context, executeRequest, executionCount, transient);

var openRequest = new InflightRequest(context, executeRequest, executionCount);

InFlightRequests[command] = openRequest;

Expand Down Expand Up @@ -79,6 +77,13 @@ public async Task Handle(JupyterRequestContext context)
}
}

private static Dictionary<string, object> CreateTransient()
{
var id = Guid.NewGuid();
var transient = new Dictionary<string, object> {{"display_id", id.ToString()}};
return transient;
}

void OnKernelResultEvent(IKernelEvent value)
{
switch (value)
Expand Down Expand Up @@ -152,10 +157,16 @@ private static void OnValueProduced(

try
{
var transient = CreateTransient();
// executeResult data
var executeResultData = new ExecuteResult(
var executeResultData = valueProduced.IsLastValue
?new ExecuteResult(
openRequest.ExecutionCount,
transient: openRequest.Transient,
transient: transient,
data: valueProduced?.FormattedValues
?.ToDictionary(k => k.MimeType ?? "text/plain", v => v.Value))
: new DisplayData(
transient: transient,
data: valueProduced?.FormattedValues
?.ToDictionary(k => k.MimeType ?? "text/plain", v => v.Value));
Copy link
Contributor

Choose a reason for hiding this comment

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

We should no longer need a fallback here.


Expand Down
6 changes: 1 addition & 5 deletions Microsoft.DotNet.Interactive.Jupyter/RequestHandlerBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
using System.Reactive.Disposables;
using Microsoft.DotNet.Interactive.Commands;
using Microsoft.DotNet.Interactive.Jupyter.Protocol;
using WorkspaceServer.Kernel;

namespace Microsoft.DotNet.Interactive.Jupyter
{
Expand Down Expand Up @@ -41,18 +40,15 @@ public void Dispose()
protected class InflightRequest : IDisposable
{
private readonly CompositeDisposable _disposables = new CompositeDisposable();
public Dictionary<string, object> Transient { get; }
public JupyterRequestContext Context { get; }
public T Request { get; }
public int ExecutionCount { get; }

public InflightRequest(JupyterRequestContext context, T request, int executionCount,
Dictionary<string, object> transient)
public InflightRequest(JupyterRequestContext context, T request, int executionCount)
{
Context = context;
Request = request;
ExecutionCount = executionCount;
Transient = transient;
}

public void AddDisposable(IDisposable disposable)
Expand Down
6 changes: 4 additions & 2 deletions Microsoft.DotNet.Interactive/Events/ValueProduced.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,18 @@ namespace Microsoft.DotNet.Interactive.Events
{
public class ValueProduced : KernelEventBase
{
public ValueProduced(
object value,
public ValueProduced(object value,
SubmitCode submitCode,
bool isLastValue = false,
colombod marked this conversation as resolved.
Show resolved Hide resolved
IReadOnlyCollection<FormattedValue> formattedValues = null) : base(submitCode)
{
Value = value;
IsLastValue = isLastValue;
FormattedValues = formattedValues;
}

public object Value { get; }
public bool IsLastValue { get; }

public IReadOnlyCollection<FormattedValue> FormattedValues { get; }
}
Expand Down
77 changes: 58 additions & 19 deletions WorkspaceServer.Tests/Kernel/CSharpKernelTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public async Task it_returns_the_result_of_a_non_null_expression()
{
var kernel = CreateKernel();

await kernel.SendAsync(new SubmitCode("123", "csharp"));
await kernel.SendAsync(new SubmitCode("123"));

KernelEvents.OfType<ValueProduced>()
.Last()
Expand All @@ -44,9 +44,9 @@ public async Task when_it_throws_exception_after_a_value_was_produced_then_only_
{
var kernel = CreateKernel();

await kernel.SendAsync(new SubmitCode("using System;", "csharp"));
await kernel.SendAsync(new SubmitCode("2 + 2", "csharp"));
await kernel.SendAsync(new SubmitCode("adddddddddd", "csharp"));
await kernel.SendAsync(new SubmitCode("using System;"));
await kernel.SendAsync(new SubmitCode("2 + 2"));
await kernel.SendAsync(new SubmitCode("adddddddddd"));

var (failure, lastCodeSubmissionEvaluationFailedPosition) = KernelEvents
.Select((error, pos) => (error, pos))
Expand Down Expand Up @@ -75,8 +75,8 @@ public async Task it_returns_exceptions_thrown_in_user_code()
{
var kernel = CreateKernel();

await kernel.SendAsync(new SubmitCode("using System;", "csharp"));
await kernel.SendAsync(new SubmitCode("throw new NotImplementedException();", "csharp"));
await kernel.SendAsync(new SubmitCode("using System;"));
await kernel.SendAsync(new SubmitCode("throw new NotImplementedException();"));

KernelEvents.Last()
.Should()
Expand All @@ -92,8 +92,8 @@ public async Task it_returns_diagnostics()
{
var kernel = CreateKernel();

await kernel.SendAsync(new SubmitCode("using System;", "csharp"));
await kernel.SendAsync(new SubmitCode("aaaadd", "csharp"));
await kernel.SendAsync(new SubmitCode("using System;"));
await kernel.SendAsync(new SubmitCode("aaaadd"));

KernelEvents.Last()
.Should()
Expand All @@ -109,9 +109,9 @@ public async Task it_notifies_when_submission_is_complete()
{
var kernel = CreateKernel();

await kernel.SendAsync(new SubmitCode("var a =", "csharp"));
await kernel.SendAsync(new SubmitCode("var a ="));

await kernel.SendAsync(new SubmitCode("12;", "csharp"));
await kernel.SendAsync(new SubmitCode("12;"));

KernelEvents.Should()
.NotContain(e => e is ValueProduced);
Expand All @@ -126,7 +126,7 @@ public async Task it_notifies_when_submission_is_incomplete()
{
var kernel = CreateKernel();

await kernel.SendAsync(new SubmitCode("var a =", "csharp"));
await kernel.SendAsync(new SubmitCode("var a ="));

KernelEvents.Should()
.NotContain(e => e is ValueProduced);
Expand All @@ -141,7 +141,7 @@ public async Task it_returns_the_result_of_a_null_expression()
{
var kernel = CreateKernel();

await kernel.SendAsync(new SubmitCode("null", "csharp"));
await kernel.SendAsync(new SubmitCode("null"));

KernelEvents.OfType<ValueProduced>()
.Last()
Expand All @@ -155,7 +155,7 @@ public async Task it_does_not_return_a_result_for_a_statement()
{
var kernel = CreateKernel();

await kernel.SendAsync(new SubmitCode("var x = 1;", "csharp"));
await kernel.SendAsync(new SubmitCode("var x = 1;"));

KernelEvents
.Should()
Expand All @@ -167,9 +167,9 @@ public async Task it_aggregates_multiple_submissions()
{
var kernel = CreateKernel();

await kernel.SendAsync(new SubmitCode("var x = new List<int>{1,2};", "csharp"));
await kernel.SendAsync(new SubmitCode("x.Add(3);", "csharp"));
await kernel.SendAsync(new SubmitCode("x.Max()", "csharp"));
await kernel.SendAsync(new SubmitCode("var x = new List<int>{1,2};"));
await kernel.SendAsync(new SubmitCode("x.Add(3);"));
await kernel.SendAsync(new SubmitCode("x.Max()"));

KernelEvents.OfType<ValueProduced>()
.Last()
Expand All @@ -178,13 +178,52 @@ public async Task it_aggregates_multiple_submissions()
.Be(3);
}

[Fact]
public async Task it_produces_values_when_executing_Console_output()
{
var kernel = CreateKernel();

var kernelCommand = new SubmitCode(@"
Console.Write(""value one"");
Console.Write(""value two"");
Console.Write(""value three"");");
await kernel.SendAsync(kernelCommand);

KernelEvents.OfType<ValueProduced>()
.Should()
.BeEquivalentTo(
new ValueProduced("value one", kernelCommand, false, new[] { new FormattedValue(null, "value one"), }),
new ValueProduced("value two", kernelCommand, false, new[] { new FormattedValue(null, "value two"), }),
new ValueProduced("value three", kernelCommand, false, new[] { new FormattedValue(null, "value three"), }));
Copy link
Contributor

Choose a reason for hiding this comment

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

Why no mime type?

Copy link
Member Author

Choose a reason for hiding this comment

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

myme types are done later on when formatting on raw value is done

Copy link
Contributor

Choose a reason for hiding this comment

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

I think FormattedValue without a mime type is confusing and probably bug prone. I think this test shows that because of the redundancy -- the formatted values ARE the raw values.

Copy link
Member Author

Choose a reason for hiding this comment

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

null value is generated by the Formatter.MimeTypeFor method

Copy link
Contributor

Choose a reason for hiding this comment

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

That's the right behavior. FormattedValue.MimeType is intentionally read-only because the expectation was that the mime type must be known at the time a FormattedValue is created. It was an oversight that the constructor didn't guard against this.

}

[Fact]
public async Task it_produces_a_final_value_if_the_code_expression_evaluates()
{
var kernel = CreateKernel();

var kernelCommand = new SubmitCode(@"
Console.Write(""value one"");
Console.Write(""value two"");
Console.Write(""value three"");
5", "csharp");
colombod marked this conversation as resolved.
Show resolved Hide resolved
await kernel.SendAsync(kernelCommand);

KernelEvents.OfType<ValueProduced>()
.Should()
.HaveCount(4)
.And
.ContainSingle(e => e.IsLastValue);

}

[Fact(Skip = "requires support for cs8 in roslyn scripting")]
public async Task it_supports_csharp_8()
{
var kernel = CreateKernel();

await kernel.SendAsync(new SubmitCode("var text = \"meow? meow!\";", "csharp"));
await kernel.SendAsync(new SubmitCode("text[^5..^0]", "csharp"));
await kernel.SendAsync(new SubmitCode("var text = \"meow? meow!\";"));
await kernel.SendAsync(new SubmitCode("text[^5..^0]"));

KernelEvents.OfType<ValueProduced>()
.Last()
Expand Down Expand Up @@ -280,7 +319,7 @@ await kernel.SendAsync(
.Should()
.Contain(i => i.DisplayText == "SerializeObject");
}

[Fact]
public async Task The_extend_directive_can_be_used_to_load_a_kernel_extension()
{
Expand Down
Loading