Skip to content

Commit

Permalink
value kernel should not produce reference values (#2740)
Browse files Browse the repository at this point in the history
fix


fix
  • Loading branch information
colombod authored Feb 16, 2023
1 parent c8a0c6d commit bfc84b4
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 22 deletions.
26 changes: 7 additions & 19 deletions src/Microsoft.DotNet.Interactive.Tests/KeyValueStoreKernelTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public async Task SubmitCode_is_not_valid_without_a_value_name()
}

[Fact]
public async Task SubmitCode_stores_code_as_a_string()
public async Task SubmitCode_stores_code_as_a_formatted_value()
{
using var kernel = CreateKernel();

Expand All @@ -54,10 +54,7 @@ await kernel.SubmitCodeAsync(

var (success, valueProduced) = await keyValueStoreKernel.TryRequestValueAsync("hi");

valueProduced.Value
.Should()
.BeOfType<string>()
.Which
valueProduced.FormattedValue.Value
.Should()
.Be(storedValue);
}
Expand Down Expand Up @@ -184,10 +181,7 @@ public async Task It_can_import_file_contents_as_strings(string code)

var (success, valueProduced) = await keyValueStoreKernel.TryRequestValueAsync("hi");

valueProduced.Value
.Should()
.BeOfType<string>()
.Which
valueProduced.FormattedValue.Value
.Should()
.Be(fileContents);
}
Expand All @@ -203,10 +197,7 @@ public async Task It_can_import_URL_contents_as_strings()

var (success, valueProduced) = await keyValueStoreKernel.TryRequestValueAsync("hi");

valueProduced.Value
.Should()
.BeOfType<string>()
.Which
valueProduced.FormattedValue.Value
.Should()
.Contain("<html");
}
Expand All @@ -229,10 +220,7 @@ public async Task It_can_store_user_input()

var (success, valueProduced) = await keyValueStoreKernel.TryRequestValueAsync("hi");

valueProduced.Value
.Should()
.BeOfType<string>()
.Which
valueProduced.FormattedValue.Value
.Should()
.Be("hello!");
}
Expand Down Expand Up @@ -302,7 +290,7 @@ public async Task Share_into_the_value_kernel_is_not_supported_and_stores_the_di
var (success, valueProduced) = await valueKernel.TryRequestValueAsync("x");
success.Should().BeTrue();

valueProduced.Value.Should().Be("#!share --from fsharp f");
valueProduced.FormattedValue.Value.Should().Be("#!share --from fsharp f");
}

[Fact]
Expand Down Expand Up @@ -416,7 +404,7 @@ await kernel.SubmitCodeAsync($@"
.Should()
.BeTrue();

valueProduced.Value
valueProduced.FormattedValue.Value
.Should()
.Be("// previous content");
}
Expand Down
39 changes: 39 additions & 0 deletions src/Microsoft.DotNet.Interactive.Tests/SetMagicCommandTests.cs
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
// Copyright (c) .NET Foundation and contributors. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System;
using System.Collections.Generic;
using System.Text.Json;
using System.Threading.Tasks;
using FluentAssertions;
using FluentAssertions.Equivalency;
using FluentAssertions.Execution;
using Microsoft.DotNet.Interactive.Commands;
using Microsoft.DotNet.Interactive.CSharp;
Expand Down Expand Up @@ -99,6 +102,42 @@ await composite.SendAsync(new SubmitCode($@"
valueProduced.Value.Should().BeEquivalentTo("456");
}

[Fact]
public async Task honors_mimetype_from_value_kernel()
{
var csharpKernel = CreateKernel(Language.CSharp);
var valueKernel = new KeyValueStoreKernel().UseValueSharing();

using var composite = new CompositeKernel
{
csharpKernel,
valueKernel
};
var jsonFragment = @"{
""a"" : 123
}";
await composite.SendAsync(new SubmitCode(@$"#!value --name data --mime-type application/json
{jsonFragment}
"));

var result = await composite.SendAsync(new SubmitCode($@"#!set --name x --value @{valueKernel.Name}:data ", targetKernelName: csharpKernel.Name));

result.Events.Should().NotContainErrors();
var (succeeded, valueProduced) = await csharpKernel.TryRequestValueAsync("x");

using var _ = new AssertionScope();

var expected = JsonDocument.Parse(jsonFragment);
succeeded.Should().BeTrue();
valueProduced.Value.Should()
.BeOfType<JsonDocument>()
.Which
.Should()
.BeEquivalentTo(expected, JsonEquivalenceConfig);
}

public EquivalencyAssertionOptions<JsonDocument> JsonEquivalenceConfig(EquivalencyAssertionOptions<JsonDocument> opt) => opt.ComparingByMembers<JsonElement>();


[Fact]
public async Task set_requires_from_option()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ public async Task CSharpKernel_can_prompt_for_input_from_JavaScript_via_a_ProxyK
.Should()
.BeTrue();

valueProduced.Value.Should().Be("hello!");
valueProduced.FormattedValue.Value.Should().Be("hello!");
}

[Fact]
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.DotNet.Interactive/KeyValueStoreKernel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ Task IKernelCommandHandler<RequestValue>.HandleAsync(RequestValue command, Kerne
if (_values.TryGetValue(command.Name, out var value))
{
context.Publish(new ValueProduced(
value.Value,
null,
command.Name,
value,
command));
Expand Down
7 changes: 6 additions & 1 deletion src/Microsoft.DotNet.Interactive/Parsing/SubmissionParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using System.Linq;
using System.Reactive.Linq;
using System.Text.Json;
using System.Text.Json.Nodes;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Text;
using Microsoft.DotNet.Interactive.Commands;
Expand Down Expand Up @@ -478,12 +479,16 @@ private bool InterpolateValueFromKernel(
JsonValueKind.False => false,
JsonValueKind.String => jsonDoc.Deserialize<string>(),
JsonValueKind.Number => jsonDoc.Deserialize<double>(),

JsonValueKind.Object => stringValue,
_ => null
};

interpolatedValue = jsonValue?.ToString();
}
else if (valueProduced.FormattedValue.MimeType == "text/plain")
{
interpolatedValue = valueProduced.FormattedValue.Value;
}
else
{
errorMessage = result.Events.OfType<CommandFailed>().Last().Message;
Expand Down

0 comments on commit bfc84b4

Please sign in to comment.