-
Notifications
You must be signed in to change notification settings - Fork 516
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.
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
[dotnet] Added tests, fixed bugs #17979
Conversation
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few very small things, but otherwise LGTM :)
Directory.CreateDirectory (path); | ||
return path; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: reformatter may grab this :)
if (workingDirectory != null) | ||
info.WorkingDirectory = workingDirectory; | ||
|
||
if (output == null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (output == null) | |
if (output is null) |
info.RedirectStandardInput = false; | ||
info.RedirectStandardOutput = true; | ||
info.RedirectStandardError = true; | ||
if (workingDirectory != null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (workingDirectory != null) | |
if (workingDirectory is not null) |
if (output == null) | ||
output = new StringBuilder (); | ||
|
||
if (env != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (env != null) { | |
if (env is not null) { |
|
||
if (env != null) { | ||
foreach (var kvp in env) { | ||
if (kvp.Value == null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (kvp.Value == null) { | |
if (kvp.Value is null) { |
static string GetTempDirectory () | ||
{ | ||
var unique = Interlocked.Increment (ref counter).ToString (); | ||
var path = Path.Combine (root, unique); | ||
Directory.CreateDirectory (path); | ||
return path; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use Cache.CreateTemporaryDirectory instead:
xamarin-macios/tests/mtouch/Cache.cs
Line 47 in ada4d2c
public static string CreateTemporaryDirectory (string? name = null) |
public static string Run (string path, string args, string? workingDirectory = null, bool verbose = false) | ||
{ | ||
var output = new StringBuilder (); | ||
var exitCode = RunCommand (path, args, output: output, verbose: verbose, workingDirectory: workingDirectory); | ||
if (exitCode != 0) | ||
throw new Exception ($"Failed to execute (exit code {exitCode}): {path} {string.Join (" ", args)}\n{output.ToString ()}"); | ||
return output.ToString (); | ||
} | ||
|
||
static void ReadStream (Stream stream, StringBuilder sb, ManualResetEvent completed) | ||
{ | ||
var encoding = Encoding.UTF8; | ||
var decoder = encoding.GetDecoder (); | ||
var buffer = new byte [1024]; | ||
var characters = new char [encoding.GetMaxCharCount (buffer.Length)]; | ||
|
||
AsyncCallback? callback = null; | ||
callback = new AsyncCallback ((IAsyncResult ar) => { | ||
var read = stream.EndRead (ar); | ||
|
||
var chars = decoder.GetChars (buffer, 0, read, characters, 0); | ||
lock (sb) | ||
sb.Append (characters, 0, chars); | ||
|
||
if (read > 0) { | ||
stream.BeginRead (buffer, 0, buffer.Length, callback, null); | ||
} else { | ||
completed.Set (); | ||
} | ||
}); | ||
stream.BeginRead (buffer, 0, buffer.Length, callback, null); | ||
} | ||
|
||
public static int RunCommand (string path, string args, IDictionary<string, string>? env = null, StringBuilder? output = null, bool verbose = false, string? workingDirectory = null) | ||
{ | ||
var info = new ProcessStartInfo (path, args); | ||
info.UseShellExecute = false; | ||
info.RedirectStandardInput = false; | ||
info.RedirectStandardOutput = true; | ||
info.RedirectStandardError = true; | ||
if (workingDirectory is not null) | ||
info.WorkingDirectory = workingDirectory; | ||
|
||
if (output is null) | ||
output = new StringBuilder (); | ||
|
||
if (env is not null) { | ||
foreach (var kvp in env) { | ||
if (kvp.Value is null) { | ||
if (info.EnvironmentVariables.ContainsKey (kvp.Key)) | ||
info.EnvironmentVariables.Remove (kvp.Key); | ||
} else { | ||
info.EnvironmentVariables [kvp.Key] = kvp.Value; | ||
} | ||
} | ||
} | ||
|
||
if (verbose) { | ||
var envOut = new StringBuilder (); | ||
foreach (var k in info.EnvironmentVariables.Keys) { | ||
if (k is string key) { | ||
var value = info.EnvironmentVariables [key]; | ||
envOut.AppendLine ($"export {key}={value}"); | ||
} | ||
} | ||
envOut.AppendLine ($"{path} {args}"); | ||
Console.Write (envOut.ToString ()); | ||
Console.WriteLine ("{0} {1}", path, args); | ||
} | ||
|
||
using (var p = Process.Start (info)) { | ||
var error_output = new StringBuilder (); | ||
var stdout_completed = new ManualResetEvent (false); | ||
var stderr_completed = new ManualResetEvent (false); | ||
|
||
ReadStream (p.StandardOutput.BaseStream, output, stdout_completed); | ||
ReadStream (p.StandardError.BaseStream, error_output, stderr_completed); | ||
|
||
p.WaitForExit (); | ||
|
||
stderr_completed.WaitOne (TimeSpan.FromMinutes (1)); | ||
stdout_completed.WaitOne (TimeSpan.FromMinutes (1)); | ||
|
||
if (verbose) { | ||
if (output.Length > 0) | ||
Console.WriteLine (output); | ||
if (error_output.Length > 0) | ||
Console.WriteLine (error_output); | ||
if (p.ExitCode != 0) | ||
Console.Error.WriteLine ($"Process exited with code {p.ExitCode}"); | ||
} | ||
if (p.ExitCode != 0 && error_output.Length > 0) | ||
output.Append (error_output); | ||
return p.ExitCode; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the Execution
class in tools/common/Execution.cs instead:
xamarin-macios/tools/common/Execution.cs
Lines 185 to 190 in ada4d2c
public static Task<Execution> RunAsync (string filename, IList<string> arguments, Dictionary<string, string?>? environment = null, bool mergeOutput = false, string? workingDirectory = null, TextWriter? log = null, TimeSpan? timeout = null, CancellationToken? cancellationToken = null) | |
{ | |
var standardOutput = new StringWriter (); | |
var standardError = mergeOutput ? standardOutput : new StringWriter (); | |
return RunAsync (filename, arguments, environment, standardOutput, standardError, log, workingDirectory, timeout, cancellationToken); | |
} |
static void DeleteDirectoryAndContents (string path) | ||
{ | ||
var directories = new Stack<string> (); | ||
directories.Push (path); | ||
while (directories.Count > 0) { | ||
var currDirectory = directories.Pop (); | ||
PushAll (directories, Directory.EnumerateDirectories (currDirectory)); | ||
foreach (var file in Directory.EnumerateFiles (currDirectory)) | ||
File.Delete (file); | ||
Directory.Delete (currDirectory); | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is unnecessary if you use Cache.CreateTemporaryDirectory
(as suggested below)
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
using var writer = new StreamWriter (inputpath); | ||
writer.Write (code); | ||
writer.Flush (); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is somewhat simpler:
using var writer = new StreamWriter (inputpath); | |
writer.Write (code); | |
writer.Flush (); | |
File.WriteAllText (inputPath, code); |
Stunt testing!
What happens is this: we write a string of C# to a file. This C# acts as a mock of the ObjCRuntime namespace it also includes a one or more classes that act internally like NSObject descendants at least as far as the
class_ptr
field is concerned.The test compiles this code, runs the rewriter, then executes the final code collecting the output.
The tests include the following:
ClassHandle
now returns a different valueThe code to run a process and collect its output was taken from the binding-tools-for-swift tests.
All tests pass.