Skip to content

Don't write diagnostics to stdOut #686

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 3 commits into from
Sep 26, 2022
Merged

Don't write diagnostics to stdOut #686

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cd3e6e7
Write diagnostics to stdErr and added test
seesharper Sep 23, 2022
fb85674
Fixed exitcode
seesharper Sep 23, 2022
7fc6a33
Use PascalCasing for ProcessResult properties
seesharper Sep 23, 2022
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
15 changes: 11 additions & 4 deletions src/Dotnet.Script.Core/ScriptConsole.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,22 +36,29 @@ public virtual void WriteHighlighted(string value)
Console.ResetColor();
}

public virtual void WriteWarning(string value)
{
Console.ForegroundColor = ConsoleColor.Yellow;
Error.WriteLine(value.TrimEnd(Environment.NewLine.ToCharArray()));
Console.ResetColor();
}

public virtual void WriteNormal(string value)
{
Out.WriteLine(value.TrimEnd(Environment.NewLine.ToCharArray()));
}

public virtual void WriteDiagnostics(Diagnostic[] warningDiagnostics, Diagnostic[] errorDiagnostics)
public virtual void WriteDiagnostics(Diagnostic[] warningDiagnostics, Diagnostic[] errorDiagnostics)
{
if (warningDiagnostics != null)
if (warningDiagnostics != null)
{
foreach (var warning in warningDiagnostics)
{
WriteHighlighted(warning.ToString());
WriteWarning(warning.ToString());
}
}

if (errorDiagnostics != null)
if (errorDiagnostics != null)
{
foreach (var error in errorDiagnostics)
{
Expand Down
2 changes: 1 addition & 1 deletion src/Dotnet.Script.Core/ScriptEmitter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public virtual ScriptEmitResult Emit<TReturn, THost>(ScriptContext context, stri
var compilationContext = _scriptCompiler.CreateCompilationContext<TReturn, THost>(context);
foreach (var warning in compilationContext.Warnings)
{
_scriptConsole.WriteHighlighted(warning.ToString());
_scriptConsole.WriteWarning(warning.ToString());
}

if (compilationContext.Errors.Any())
Expand Down
12 changes: 7 additions & 5 deletions src/Dotnet.Script.Shared.Tests/ProcessHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ namespace Dotnet.Script.Shared.Tests
{
public static class ProcessHelper
{
public static (string output, int exitcode) RunAndCaptureOutput(string fileName, string arguments, string workingDirectory = null)
public static ProcessResult RunAndCaptureOutput(string fileName, string arguments, string workingDirectory = null)
{
var startInfo = new ProcessStartInfo(fileName, arguments)
{
Expand All @@ -29,15 +29,17 @@ public static (string output, int exitcode) RunAndCaptureOutput(string fileName,
catch
{
Console.WriteLine($"Failed to launch '{fileName}' with args, '{arguments}'");
return (null, -1);
return new ProcessResult(null, -1, null, null);
}

var output = process.StandardOutput.ReadToEnd();
output += process.StandardError.ReadToEnd();
var standardOut = process.StandardOutput.ReadToEnd().Trim();
var standardError = process.StandardError.ReadToEnd().Trim();

var output = standardOut + standardError;

process.WaitForExit();

return (output.Trim(), process.ExitCode);
return new ProcessResult(output, process.ExitCode, standardOut, standardError);
}
}
}
24 changes: 24 additions & 0 deletions src/Dotnet.Script.Shared.Tests/ProcessResult.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
namespace Dotnet.Script.Shared.Tests
{
public class ProcessResult
{
public ProcessResult(string output, int exitCode, string standardOut, string standardError)
{
this.Output = output;
this.ExitCode = exitCode;
this.StandardOut = standardOut;
this.StandardError = standardError;
}

public string Output { get; }
public int ExitCode { get; }
public string StandardOut { get; }
public string StandardError { get; }

public void Deconstruct(out string output, out int exitCode)
{
output = this.Output;
exitCode = this.ExitCode;
}
}
}
6 changes: 3 additions & 3 deletions src/Dotnet.Script.Tests/ExecutionCacheTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,8 @@ public void ShouldCacheScriptsFromSameFolderIndividually()
private (string output, string hash) Execute(string pathToScript)
{
var result = ScriptTestRunner.Default.Execute(pathToScript);
testOutputHelper.WriteLine(result.output);
Assert.Equal(0, result.exitCode);
testOutputHelper.WriteLine(result.Output);
Assert.Equal(0, result.ExitCode);
string pathToExecutionCache = GetPathToExecutionCache(pathToScript);
var pathToCacheFile = Path.Combine(pathToExecutionCache, "script.sha256");
string cachedhash = null;
Expand All @@ -146,7 +146,7 @@ public void ShouldCacheScriptsFromSameFolderIndividually()
cachedhash = File.ReadAllText(pathToCacheFile);
}

return (result.output, cachedhash);
return (result.Output, cachedhash);
}

private static string GetPathToExecutionCache(string pathToScript)
Expand Down
8 changes: 4 additions & 4 deletions src/Dotnet.Script.Tests/PackageSourceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ public void ShouldHandleSpecifyingPackageSource()
var fixture = "ScriptPackage/WithNoNuGetConfig";
var pathToScriptPackages = ScriptPackagesFixture.GetPathToPackagesFolder();
var result = ScriptTestRunner.Default.ExecuteFixture(fixture, $"--no-cache -s \"{pathToScriptPackages}\"");
Assert.Contains("Hello", result.output);
Assert.Equal(0, result.exitCode);
Assert.Contains("Hello", result.Output);
Assert.Equal(0, result.ExitCode);
}

[Fact]
Expand All @@ -28,8 +28,8 @@ public void ShouldHandleSpecifyingPackageSourceWhenEvaluatingCode()
var pathToScriptPackages = ScriptPackagesFixture.GetPathToPackagesFolder();
var code = @"#load \""nuget:ScriptPackageWithMainCsx,1.0.0\"" SayHello();";
var result = ScriptTestRunner.Default.Execute($"--no-cache -s \"{pathToScriptPackages}\" eval \"{code}\"");
Assert.Contains("Hello", result.output);
Assert.Equal(0, result.exitCode);
Assert.Contains("Hello", result.Output);
Assert.Equal(0, result.ExitCode);
}
}
}
16 changes: 12 additions & 4 deletions src/Dotnet.Script.Tests/ScriptExecutionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,14 @@ public void ShouldReturnExitCodeOneWhenScriptFailsToCompile()
Assert.Equal(1, exitCode);
}

[Fact]
public void ShouldWriteCompilerWarningsToStandardError()
{
var result = ScriptTestRunner.Default.ExecuteFixture(fixture: "CompilationWarning", "--no-cache");
Assert.True(string.IsNullOrWhiteSpace(result.StandardOut));
Assert.Contains("CS1998", result.StandardError, StringComparison.OrdinalIgnoreCase);
}

[Fact]
public void ShouldHandleIssue129()
{
Expand Down Expand Up @@ -246,7 +254,7 @@ public void ShouldSupportInlineNugetReferencesWithTrailingSemicoloninEvaluatedCo
public void ShouldExecuteRemoteScript(string url, string output)
{
var result = ScriptTestRunner.Default.Execute(url);
Assert.Contains(output, result.output);
Assert.Contains(output, result.Output);
}

[Fact]
Expand Down Expand Up @@ -329,19 +337,19 @@ public void ShouldThrowMeaningfulErrorMessageWhenDependencyIsNotFound()

// Run once to ensure that it is cached.
var result = ScriptTestRunner.Default.Execute(pathToScript);
Assert.Contains("42", result.output);
Assert.Contains("42", result.Output);

// Remove the package from the global NuGet cache
TestPathUtils.RemovePackageFromGlobalNugetCache("SampleLibrary");

//ScriptTestRunner.Default.ExecuteInProcess(pathToScript);

result = ScriptTestRunner.Default.Execute(pathToScript);
Assert.Contains("Try executing/publishing the script", result.output);
Assert.Contains("Try executing/publishing the script", result.Output);

// Run again with the '--no-cache' option to assert that the advice actually worked.
result = ScriptTestRunner.Default.Execute($"{pathToScript} --no-cache");
Assert.Contains("42", result.output);
Assert.Contains("42", result.Output);
}

[Fact]
Expand Down
4 changes: 2 additions & 2 deletions src/Dotnet.Script.Tests/ScriptPackagesTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public void ShouldThrowMeaningfulExceptionWhenScriptPackageIsMissing()

// Run once to ensure that it is cached.
var result = ScriptTestRunner.Default.Execute($"{pathToScriptFile} -s {pathToScriptPackages}");
Assert.StartsWith("Hello from netstandard2.0", result.output);
Assert.StartsWith("Hello from netstandard2.0", result.Output);

// Remove the package from the global NuGet cache
TestPathUtils.RemovePackageFromGlobalNugetCache("ScriptPackageWithMainCsx");
Expand All @@ -48,7 +48,7 @@ public void ShouldThrowMeaningfulExceptionWhenScriptPackageIsMissing()
File.WriteAllText(pathToScriptFile, code.ToString());

result = ScriptTestRunner.Default.Execute($"{pathToScriptFile} -s {pathToScriptPackages}");
Assert.Contains("Try executing/publishing the script", result.output);
Assert.Contains("Try executing/publishing the script", result.Output);
}

[Fact]
Expand Down
12 changes: 6 additions & 6 deletions src/Dotnet.Script.Tests/ScriptPublisherTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ public void SimplePublishDllTest()
var dllPath = Path.Combine("publish", "main.dll");
var dllRunResult = ScriptTestRunner.Default.Execute($"exec {dllPath}", workspaceFolder.Path);

Assert.Equal(0, dllRunResult.exitCode);
Assert.Equal(0, dllRunResult.ExitCode);
}

[Fact]
Expand All @@ -143,7 +143,7 @@ public void SimplePublishDllFromCurrentDirectoryTest()

var dllRunResult = ScriptTestRunner.Default.Execute($"exec {dllPath}", workspaceFolder.Path);

Assert.Equal(0, dllRunResult.exitCode);
Assert.Equal(0, dllRunResult.ExitCode);
}

[Fact]
Expand All @@ -160,7 +160,7 @@ public void SimplePublishDllToOtherFolderTest()
var dllPath = Path.Combine(publishFolder.Path, "main.dll");
var dllRunResult = ScriptTestRunner.Default.Execute($"exec {dllPath}", publishFolder.Path);

Assert.Equal(0, dllRunResult.exitCode);
Assert.Equal(0, dllRunResult.ExitCode);
}

[Fact]
Expand All @@ -178,7 +178,7 @@ public void CustomDllNameTest()
var dllPath = Path.Combine(workspaceFolder.Path, "publish", assemblyName);
var dllRunResult = ScriptTestRunner.Default.Execute($"exec {dllPath}", workspaceFolder.Path);

Assert.Equal(0, dllRunResult.exitCode);
Assert.Equal(0, dllRunResult.ExitCode);
}

[Fact]
Expand Down Expand Up @@ -211,8 +211,8 @@ public void DllWithArgsTests()
var dllPath = Path.Combine(workspaceFolder.Path, "publish", "main.dll");
var dllRunResult = ScriptTestRunner.Default.Execute($"exec {dllPath} -- w o r l d", workspaceFolder.Path);

Assert.Equal(0, dllRunResult.exitCode);
Assert.Contains("Hello world", dllRunResult.output);
Assert.Equal(0, dllRunResult.ExitCode);
Assert.Contains("Hello world", dllRunResult.Output);
}

[Fact]
Expand Down
2 changes: 1 addition & 1 deletion src/Dotnet.Script.Tests/ScriptRunnerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ private static ScriptRunner CreateScriptRunner()
{
var logFactory = TestOutputHelper.CreateTestLogFactory();
var scriptCompiler = new ScriptCompiler(logFactory, false);

return new ScriptRunner(scriptCompiler, logFactory, ScriptConsole.Default);
}
}
Expand Down
10 changes: 5 additions & 5 deletions src/Dotnet.Script.Tests/ScriptTestRunner.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ private ScriptTestRunner()
_scriptEnvironment = ScriptEnvironment.Default;
}

public (string output, int exitCode) Execute(string arguments, string workingDirectory = null)
public ProcessResult Execute(string arguments, string workingDirectory = null)
{
var result = ProcessHelper.RunAndCaptureOutput("dotnet", GetDotnetScriptArguments(arguments), workingDirectory);
return result;
Expand All @@ -35,14 +35,14 @@ public int ExecuteInProcess(string arguments = null)
return Program.Main(arguments?.Split(" ") ?? Array.Empty<string>());
}

public (string output, int exitCode) ExecuteFixture(string fixture, string arguments = null, string workingDirectory = null)
public ProcessResult ExecuteFixture(string fixture, string arguments = null, string workingDirectory = null)
{
var pathToFixture = TestPathUtils.GetPathToTestFixture(fixture);
var result = ProcessHelper.RunAndCaptureOutput("dotnet", GetDotnetScriptArguments($"\"{pathToFixture}\" {arguments}"), workingDirectory);
return result;
}

public (string output, int exitcode) ExecuteWithScriptPackage(string fixture, string arguments = null, string workingDirectory = null)
public ProcessResult ExecuteWithScriptPackage(string fixture, string arguments = null, string workingDirectory = null)
{
var pathToScriptPackageFixtures = TestPathUtils.GetPathToTestFixtureFolder("ScriptPackage");
var pathToFixture = Path.Combine(pathToScriptPackageFixtures, fixture, $"{fixture}.csx");
Expand All @@ -67,13 +67,13 @@ public static int ExecuteCodeInProcess(string code, string arguments)
return Program.Main(allArguments.ToArray());
}

public (string output, int exitCode) ExecuteCode(string code)
public ProcessResult ExecuteCode(string code)
{
var result = ProcessHelper.RunAndCaptureOutput("dotnet", GetDotnetScriptArguments($"eval \"{code}\""));
return result;
}

public (string output, int exitCode) ExecuteCodeInReleaseMode(string code)
public ProcessResult ExecuteCodeInReleaseMode(string code)
{
var result = ProcessHelper.RunAndCaptureOutput("dotnet", GetDotnetScriptArguments($"-c release eval \"{code}\""));
return result;
Expand Down
3 changes: 3 additions & 0 deletions src/Dotnet.Script.Tests/TestFixtures/CompilationWarning/CompilationWarning.csx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
public async Task Foo()
{
}