Applying code review comments
authorMaryam Ariyan <maryam.ariyan@microsoft.com>
Mon, 11 Sep 2017 20:43:38 +0000 (16:43 -0400)
committerMaryam Ariyan <maryam.ariyan@microsoft.com>
Tue, 12 Sep 2017 00:37:31 +0000 (20:37 -0400)
fixes dotnet/corefx#22299 and dotnet/corefx#19956

Commit migrated from https://github.com/dotnet/corefx/commit/1a22357d316cd39bd4cf163009480ac7689a8ddb

src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs
src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Linux.cs
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs
src/libraries/System.Diagnostics.Process/tests/Configurations.props
src/libraries/System.Diagnostics.Process/tests/ProcessTests.Linux.cs [deleted file]
src/libraries/System.Diagnostics.Process/tests/ProcessTests.OSX.cs [deleted file]
src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs
src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj

index 73723bf..be94dbe 100644 (file)
@@ -12,18 +12,18 @@ internal static partial class Interop
 {
     internal static partial class Sys
     {
-        internal static unsafe void ForkAndExecProcess(
+        internal static unsafe int ForkAndExecProcess(
             string filename, string[] argv, string[] envp, string cwd,
             bool redirectStdin, bool redirectStdout, bool redirectStderr,
-            out int lpChildPid, out int stdinFd, out int stdoutFd, out int stderrFd, out int res, bool shouldThrow = true)
+            out int lpChildPid, out int stdinFd, out int stdoutFd, out int stderrFd, bool shouldThrow = true)
         {
             byte** argvPtr = null, envpPtr = null;
-            res = -1;
+            int result = -1;
             try
             {
                 AllocNullTerminatedArray(argv, ref argvPtr);
                 AllocNullTerminatedArray(envp, ref envpPtr);
-                int result = ForkAndExecProcess(
+                result = ForkAndExecProcess(
                     filename, argvPtr, envpPtr, cwd,
                     redirectStdin ? 1 : 0, redirectStdout ? 1 : 0, redirectStderr ? 1 :0,
                     out lpChildPid, out stdinFd, out stdoutFd, out stderrFd);
@@ -42,13 +42,13 @@ internal static partial class Interop
                     if (shouldThrow)
                         throw new Win32Exception();
                 }
-                res = result;
             }
             finally
             {
                 FreeArray(envpPtr, envp.Length);
                 FreeArray(argvPtr, argv.Length);
             }
+            return result;
         }
 
         [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_ForkAndExecProcess", SetLastError = true)]
index 71dc826..b7ff33e 100644 (file)
   <data name="CantRedirectStreams" xml:space="preserve">
     <value>The Process object must have the UseShellExecute property set to false in order to redirect IO streams.</value>
   </data>
+  <data name="DirectoryNotValidAsInput" xml:space="preserve">
+    <value>The FileName property should not be a directory.</value>
+  </data>
   <data name="PendingAsyncOperation" xml:space="preserve">
     <value>An async read operation has already been started on the stream.</value>
   </data>
index 9e0955d..d46cda8 100644 (file)
@@ -60,7 +60,7 @@ namespace System.Diagnostics
         /// <summary>Gets execution path</summary>
         private string GetPathToOpenFile()
         {
-            string[] allowedProgramsToRun = { "xdg-open", "gnome-open", s_kfmclient };
+            string[] allowedProgramsToRun = { "xdg-open", "gnome-open", "kfmclient" };
             foreach (var program in allowedProgramsToRun)
             {
                 string pathToProgram = FindProgramInPath(program);
index 2ff16ad..987a827 100644 (file)
@@ -16,8 +16,6 @@ namespace System.Diagnostics
 {
     public partial class Process : IDisposable
     {
-        private const string s_kfmclient = "kfmclient";
-
         private static readonly UTF8Encoding s_utf8NoBom =
             new UTF8Encoding(encoderShouldEmitUTF8Identifier: false);
 
@@ -47,7 +45,7 @@ namespace System.Diagnostics
 
         [CLSCompliant(false)]
         public static Process Start(string fileName, string arguments, string userName, SecureString password, string domain)
-        { 
+        {
             throw new PlatformNotSupportedException(SR.ProcessStartIdentityNotSupported);
         }
 
@@ -165,7 +163,7 @@ namespace System.Diagnostics
                 }
 
                 Debug.Assert(pri >= -20 && pri <= 20);
-                return 
+                return
                     pri < -15 ? ProcessPriorityClass.RealTime :
                     pri < -10 ? ProcessPriorityClass.High :
                     pri < -5 ? ProcessPriorityClass.AboveNormal :
@@ -221,13 +219,22 @@ namespace System.Diagnostics
             return new SafeProcessHandle(_processId);
         }
 
-        /// <summary>Starts the process using the supplied start info.</summary>
+        /// <summary>
+        /// Starts the process using the supplied start info. 
+        /// Even with UseShellExecute option, we try first running fileName just in case the caller is giving executable which we should run
+        /// Then if we couldn't run it we'll try the shell tools to launch it(e.g. "open fileName")
+        /// </summary>
         /// <param name="startInfo">The start info with which to start the process.</param>
         private bool StartCore(ProcessStartInfo startInfo)
         {
             string filename;
             string[] argv;
 
+            if (Directory.Exists(startInfo.FileName))
+            {
+                throw new Win32Exception(SR.DirectoryNotValidAsInput);
+            }
+
             if (startInfo.UseShellExecute)
             {
                 if (startInfo.RedirectStandardInput || startInfo.RedirectStandardOutput || startInfo.RedirectStandardError)
@@ -240,7 +247,7 @@ namespace System.Diagnostics
             string[] envp = CreateEnvp(startInfo);
             string cwd = !string.IsNullOrWhiteSpace(startInfo.WorkingDirectory) ? startInfo.WorkingDirectory : null;
 
-            filename = ResolvePath(startInfo.FileName);
+            filename = ResolvePath(startInfo.FileName); 
             if (!string.IsNullOrEmpty(filename))
             {
                 argv = ParseArgv(startInfo);
@@ -250,11 +257,11 @@ namespace System.Diagnostics
                 // descriptors, and execve to execute the requested process.  The shim implementation
                 // is used to fork/execve as executing managed code in a forked process is not safe (only
                 // the calling thread will transfer, thread IDs aren't stable across the fork, etc.)
-                Interop.Sys.ForkAndExecProcess(
+                result = Interop.Sys.ForkAndExecProcess(
                     filename, argv, envp, cwd,
                     startInfo.RedirectStandardInput, startInfo.RedirectStandardOutput, startInfo.RedirectStandardError,
                     out childPid,
-                    out stdinFd, out stdoutFd, out stderrFd, out result, shouldThrow: !startInfo.UseShellExecute);
+                    out stdinFd, out stdoutFd, out stderrFd, shouldThrow: !startInfo.UseShellExecute);
             }
 
             if (result != 0)
@@ -268,13 +275,15 @@ namespace System.Diagnostics
 
                 // this time, set the filename as default program to open file/url
                 filename = GetPathToOpenFile();
-                argv = GetArgsToOpenFile(startInfo);
+                argv = ParseArgv(startInfo, GetPathToOpenFile());
+
+                result = Interop.Sys.ForkAndExecProcess(
+                    filename, argv, envp, cwd,
+                    startInfo.RedirectStandardInput, startInfo.RedirectStandardOutput, startInfo.RedirectStandardError,
+                    out childPid,
+                    out stdinFd, out stdoutFd, out stderrFd);
 
-                Interop.Sys.ForkAndExecProcess(
-                filename, argv, envp, cwd,
-                startInfo.RedirectStandardInput, startInfo.RedirectStandardOutput, startInfo.RedirectStandardError,
-                out childPid,
-                out stdinFd, out stdoutFd, out stderrFd, out result);
+                Debug.Assert(result == 0);
             }
 
             // Store the child's information into this Process object.
@@ -308,17 +317,6 @@ namespace System.Diagnostics
             return true;
         }
 
-        /// <summary>Gets arguments.</summary>
-        /// <param name="startInfo">The start info with which to start the process.</param>
-        private string[] GetArgsToOpenFile(ProcessStartInfo startInfo)
-        {
-            if (!startInfo.UseShellExecute)
-            {
-                return ParseArgv(startInfo);
-            }
-            return ParseArgv(startInfo, GetPathToOpenFile());
-        }
-
         // -----------------------------
         // ---- PAL layer ends here ----
         // -----------------------------
@@ -329,7 +327,7 @@ namespace System.Diagnostics
         /// <summary>Size to use for redirect streams and stream readers/writers.</summary>
         private const int StreamBufferSize = 4096;
 
-        
+
         /// <summary>Converts the filename and arguments information from a ProcessStartInfo into an argv array.</summary>
         /// <param name="psi">The ProcessStartInfo.</param>
         /// <param name="alternativePath">alternative resolved path to use as first argument</param>
@@ -346,7 +344,7 @@ namespace System.Diagnostics
             if (!string.IsNullOrEmpty(alternativePath))
             {
                 argvList.Add(alternativePath);
-                if (alternativePath.Contains(s_kfmclient))
+                if (alternativePath.Contains("kfmclient"))
                 {
                     argvList.Add("openURL"); // kfmclient needs OpenURL
                 }
@@ -370,9 +368,9 @@ namespace System.Diagnostics
             return envp;
         }
 
-        /// <summary>Resolves a path to the filename passed to ProcessStartInfo.</summary>
+        /// <summary>Resolves a path to the filename passed to ProcessStartInfo. </summary>
         /// <param name="filename">The filename.</param>
-        /// <returns>The resolved path.</returns>
+        /// <returns>The resolved path. It can return null in case of URLs.</returns>
         private static string ResolvePath(string filename)
         {
             // Follow the same resolution that Windows uses with CreateProcess:
@@ -482,7 +480,7 @@ namespace System.Diagnostics
         {
             Debug.Assert(fd >= 0);
             return new FileStream(
-                new SafeFileHandle((IntPtr)fd, ownsHandle: true), 
+                new SafeFileHandle((IntPtr)fd, ownsHandle: true),
                 access, StreamBufferSize, isAsync: false);
         }
 
index 7087e3f..f60b31a 100644 (file)
@@ -4,9 +4,8 @@
     <BuildConfigurations>
       netstandard-Windows_NT;
       netstandard-Unix;
-      netstandard-OSX;
       uap-Windows_NT;
       uapaot-Windows_NT;
     </BuildConfigurations>
   </PropertyGroup>
-</Project>
+</Project>
\ No newline at end of file
diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Linux.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Linux.cs
deleted file mode 100644 (file)
index 10314f3..0000000
+++ /dev/null
@@ -1,116 +0,0 @@
-// Licensed to the .NET Foundation under one or more agreements.
-// The .NET Foundation licenses this file to you under the MIT license.
-// See the LICENSE file in the project root for more information.
-
-using System.Collections.Generic;
-using System.ComponentModel;
-using System.IO;
-using System.Linq;
-using System.Runtime.InteropServices;
-using System.Threading;
-using System.Security;
-using Xunit;
-using Xunit.NetCore.Extensions;
-
-namespace System.Diagnostics.Tests
-{
-    public partial class ProcessTests : ProcessTestBase
-    {
-        private const string s_xdg_open = "xdg-open";
-
-        [Fact]
-        // [OuterLoop("Opens program")]
-        public void ProcessStart_UseShellExecuteTrue_OpenFile_ThrowsIfNoDefaultProgramInstalledSucceedsOtherwise()
-        {
-            string fileToOpen = GetTestFilePath() + ".txt";
-            File.WriteAllText(fileToOpen, $"{nameof(ProcessStart_UseShellExecuteTrue_OpenFile_ThrowsIfNoDefaultProgramInstalledSucceedsOtherwise)}");
-
-            string[] allowedProgramsToRun = { s_xdg_open, "gnome-open", "kfmclient" };
-            foreach (var program in allowedProgramsToRun)
-            {
-                if (IsProgramInstalled(program))
-                {
-                    var startInfo = new ProcessStartInfo { UseShellExecute = true, FileName = fileToOpen };
-                    using (var px = Process.Start(startInfo))
-                    {
-                        Assert.NotNull(px);
-                        Console.WriteLine($"{nameof(ProcessStart_UseShellExecuteTrue_OpenFile_ThrowsIfNoDefaultProgramInstalledSucceedsOtherwise)}(): {program} was used to open file on this machine. ProcessName: {px.ProcessName}");
-                        Assert.Equal(program, px.ProcessName);
-                        px.Kill();
-                        px.WaitForExit();
-                        Assert.True(px.HasExited);
-                        Assert.Equal(137, px.ExitCode); // 137 means the process was killed
-                    }
-                    return;
-                }
-            }
-
-            Win32Exception e = Assert.Throws<Win32Exception>(() => Process.Start(new ProcessStartInfo { UseShellExecute = true, FileName = fileToOpen }));
-        }
-
-        [Theory, InlineData("nano")]
-        // [OuterLoop("Opens program")]
-        public void ProcessStart_OpenFile_UsesSpecifiedProgram(string programToOpenWith)
-        {
-            string fileToOpen = GetTestFilePath() + ".txt";
-            File.WriteAllText(fileToOpen, $"{nameof(ProcessStart_OpenFile_UsesSpecifiedProgram)}");
-            using (var px = Process.Start(programToOpenWith, fileToOpen))
-            {
-                Assert.Equal(programToOpenWith, px.ProcessName);
-                px.Kill();
-                px.WaitForExit();
-                Assert.True(px.HasExited);
-                Assert.Equal(137, px.ExitCode); // 137 means the process was killed
-            }
-        }
-
-        [Fact]
-        // [OuterLoop("test should succeed when xdg-open is installed. Otherwise we write to console")]
-        public void ProcessStart_UseShellExecuteTrue_OpenMissingFile_XdgOpenReturnsExitCode2()
-        {
-            if (IsProgramInstalled(s_xdg_open))
-            {
-                string fileToOpen = Path.Combine(Environment.CurrentDirectory, "_no_such_file.TXT");
-                using (var p = Process.Start(new ProcessStartInfo { UseShellExecute = true, FileName = fileToOpen }))
-                {
-                    if (p != null)
-                    {
-                        Assert.Equal(s_xdg_open, p.ProcessName);
-                        p.WaitForExit();
-                        Assert.True(p.HasExited);
-                        Assert.Equal(2, p.ExitCode);
-                    }
-                }
-            }
-            else
-            {
-                Console.WriteLine($"{nameof(ProcessStart_UseShellExecuteTrue_OpenMissingFile_XdgOpenReturnsExitCode2)}(): {s_xdg_open} is not installed on this machine.");
-            }
-        }
-
-        /// <summary>
-        /// Checks if the program is installed
-        /// </summary>
-        /// <param name="program"></param>
-        /// <returns></returns>
-        private bool IsProgramInstalled(string program)
-        {
-            string path;
-            string pathEnvVar = Environment.GetEnvironmentVariable("PATH");
-            if (pathEnvVar != null)
-            {
-                var pathParser = new StringParser(pathEnvVar, ':', skipEmpty: true);
-                while (pathParser.MoveNext())
-                {
-                    string subPath = pathParser.ExtractCurrent();
-                    path = Path.Combine(subPath, program);
-                    if (File.Exists(path))
-                    {
-                        return true;
-                    }
-                }
-            }
-            return false;
-        }
-    }
-}
diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.OSX.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.OSX.cs
deleted file mode 100644 (file)
index 1f1fd89..0000000
+++ /dev/null
@@ -1,76 +0,0 @@
-// Licensed to the .NET Foundation under one or more agreements.
-// The .NET Foundation licenses this file to you under the MIT license.
-// See the LICENSE file in the project root for more information.
-
-using System.Collections.Generic;
-using System.ComponentModel;
-using System.IO;
-using System.Linq;
-using System.Runtime.InteropServices;
-using System.Threading;
-using System.Security;
-using Xunit;
-using Xunit.NetCore.Extensions;
-
-namespace System.Diagnostics.Tests
-{
-    public partial class ProcessTests : ProcessTestBase
-    {
-        [Theory, InlineData("/usr/bin/open"), InlineData("/usr/bin/nano")]
-        // [OuterLoop("Opens program")]
-        public void ProcessStart_OpenFile_UsesSpecifiedProgram(string programToOpenWith)
-        {
-            string fileToOpen = GetTestFilePath() + ".txt";
-            File.WriteAllText(fileToOpen, $"{nameof(ProcessStart_OpenFile_UsesSpecifiedProgram)}");
-            using (var px = Process.Start(programToOpenWith, fileToOpen))
-            {
-                Assert.False(px.HasExited);
-                px.WaitForExit();
-                Assert.True(px.HasExited);
-                Assert.Equal(0, px.ExitCode); // Exit Code 0 from open means success
-            }
-        }
-        
-        [Theory, InlineData("Safari"), InlineData("\"Google Chrome\"")]
-        // [OuterLoop("Opens browser")]
-        public void ProcessStart_OpenUrl_UsesSpecifiedApplication(string applicationToOpenWith)
-        {
-            using (var px = Process.Start("/usr/bin/open", "https://github.com/dotnet/corefx -a " + applicationToOpenWith))
-            {
-                Assert.False(px.HasExited);
-                px.WaitForExit();
-                Assert.True(px.HasExited);
-                Assert.Equal(0, px.ExitCode); // Exit Code 0 from open means success
-            }
-        }
-
-        [Theory, InlineData("-a Safari"), InlineData("-a \"Google Chrome\"")]
-        // [OuterLoop("Opens browser")]
-        public void ProcessStart_UseShellExecuteTrue_OpenUrl_SuccessfullyReadsArgument(string arguments)
-        {
-            var startInfo = new ProcessStartInfo { UseShellExecute = true, FileName = "https://github.com/dotnet/corefx", Arguments = arguments };
-            using (var px = Process.Start(startInfo))
-            {
-                if (px != null)
-                {
-                    // px.Kill(); // uncommenting this changes exit code to 137, meaning process was killed
-                    px.WaitForExit();
-                    Assert.True(px.HasExited);
-                    Assert.Equal(0, px.ExitCode);
-                }
-            }
-        }
-
-        [Fact]
-        // [OuterLoop("Shows that /usr/bin/open fails to open missing file")]
-        public void ProcessStart_UseShellExecuteTrue_TryOpenFileThatDoesntExist_ReturnsExitCode1()
-        {
-            string file = Path.Combine(Environment.CurrentDirectory, "_no_such_file.TXT");
-            using (var p = Process.Start(new ProcessStartInfo { UseShellExecute = true, FileName = file }))
-            {
-                Assert.True(p.WaitForExit(WaitInMS));
-                Assert.Equal(1, p.ExitCode); // Exit Code 1 from open means something went wrong
-            }
-        }
-    }
-}
index b1a30eb..55982aa 100644 (file)
@@ -16,6 +16,8 @@ namespace System.Diagnostics.Tests
 {
     public partial class ProcessTests : ProcessTestBase
     {
+        private const string s_xdg_open = "xdg-open";
+
         [Fact]
         private void TestWindowApisUnix()
         {
@@ -61,33 +63,151 @@ namespace System.Diagnostics.Tests
             Assert.Equal(1, p.Id);
         }
 
-        [Theory, InlineData(false)]
-        public void ProcessStart_TryExitCommandAsFileName_ThrowsWin32Exception(bool useShellExecute)
+        [Fact]
+        public void ProcessStart_TryExitCommandAsFileName_ThrowsWin32Exception()
         {
-            Win32Exception e = Assert.Throws<Win32Exception>(() => Process.Start(new ProcessStartInfo { UseShellExecute = useShellExecute, FileName = "exit", Arguments = "42" }));
+            Win32Exception e = Assert.Throws<Win32Exception>(() => Process.Start(new ProcessStartInfo { UseShellExecute = false, FileName = "exit", Arguments = "42" }));
         }
 
         [Fact]
-        // [OuterLoop("Opens application")]
-        public void ProcessStart_UseShellExecuteTrue_OpenNano_OpensNano()
+        public void ProcessStart_UseShellExecuteFalse_FilenameIsUrl_ThrowsWin32Exception()
+        {
+            Win32Exception e = Assert.Throws<Win32Exception>(() => Process.Start(new ProcessStartInfo { UseShellExecute = false, FileName = "https://www.github.com/corefx" }));
+        }
+
+        [Theory, InlineData(false), InlineData(true)]
+        public void ProcessStart_TryOpenFolder_ThrowsWin32Exception(bool useShellExecute)
         {
-            string appToOpen = "nano";
-            var startInfo = new ProcessStartInfo { UseShellExecute = true, FileName = appToOpen };
+            Win32Exception e = Assert.Throws<Win32Exception>(() => Process.Start(new ProcessStartInfo { UseShellExecute = useShellExecute, FileName = Environment.CurrentDirectory }));
+        }
+
+        [Fact, PlatformSpecific(TestPlatforms.Linux)]
+        // [OuterLoop("Opens program")]
+        public void ProcessStart_UseShellExecuteTrue_OpenFile_ThrowsIfNoDefaultProgramInstalledSucceedsOtherwise()
+        {
+            string fileToOpen = GetTestFilePath() + ".txt";
+            File.WriteAllText(fileToOpen, $"{nameof(ProcessStart_UseShellExecuteTrue_OpenFile_ThrowsIfNoDefaultProgramInstalledSucceedsOtherwise)}");
+
+            string[] allowedProgramsToRun = { s_xdg_open, "gnome-open", "kfmclient" };
+            foreach (var program in allowedProgramsToRun)
+            {
+                if (IsProgramInstalled(program))
+                {
+                    var startInfo = new ProcessStartInfo { UseShellExecute = true, FileName = fileToOpen };
+                    using (var px = Process.Start(startInfo))
+                    {
+                        Assert.NotNull(px);
+                        Console.WriteLine($"{nameof(ProcessStart_UseShellExecuteTrue_OpenFile_ThrowsIfNoDefaultProgramInstalledSucceedsOtherwise)}(): {program} was used to open file on this machine. ProcessName: {px.ProcessName}");
+                        Assert.Equal(program, px.ProcessName);
+                        px.Kill();
+                        px.WaitForExit();
+                        Assert.True(px.HasExited);
+                        Assert.Equal(137, px.ExitCode); // 137 means the process was killed
+                    }
+                    return;
+                }
+            }
+
+            Win32Exception e = Assert.Throws<Win32Exception>(() => Process.Start(new ProcessStartInfo { UseShellExecute = true, FileName = fileToOpen }));
+        }
+
+        [Theory, InlineData("nano")]
+        [PlatformSpecific(TestPlatforms.Linux)]
+        // [OuterLoop("Opens program")]
+        public void ProcessStart_OpenFileOnLinux_UsesSpecifiedProgram(string programToOpenWith)
+        {
+            string fileToOpen = GetTestFilePath() + ".txt";
+            File.WriteAllText(fileToOpen, $"{nameof(ProcessStart_OpenFileOnLinux_UsesSpecifiedProgram)}");
+            using (var px = Process.Start(programToOpenWith, fileToOpen))
+            {
+                Assert.Equal(programToOpenWith, px.ProcessName);
+                px.Kill();
+                px.WaitForExit();
+                Assert.True(px.HasExited);
+                Assert.Equal(137, px.ExitCode); // 137 means the process was killed
+            }
+        }
+
+        [Fact, PlatformSpecific(TestPlatforms.Linux)]
+        // [OuterLoop("Opens program")]
+        public void ProcessStart_UseShellExecuteTrue_OpenMissingFile_XdgOpenReturnsExitCode2()
+        {
+            if (IsProgramInstalled(s_xdg_open))
+            {
+                string fileToOpen = Path.Combine(Environment.CurrentDirectory, "_no_such_file.TXT");
+                using (var p = Process.Start(new ProcessStartInfo { UseShellExecute = true, FileName = fileToOpen }))
+                {
+                    Assert.NotNull(p);
+                    Assert.Equal(s_xdg_open, p.ProcessName);
+                    p.WaitForExit();
+                    Assert.True(p.HasExited);
+                    Assert.Equal(2, p.ExitCode);
+                }
+            }
+            else
+            {
+                Console.WriteLine($"{nameof(ProcessStart_UseShellExecuteTrue_OpenMissingFile_XdgOpenReturnsExitCode2)}(): {s_xdg_open} is not installed on this machine.");
+            }
+        }
+
+        [Theory, InlineData("/usr/bin/open"), InlineData("/usr/bin/nano")]
+        [PlatformSpecific(TestPlatforms.OSX)]
+        // [OuterLoop("Opens program")]
+        public void ProcessStart_OpenFileOnOsx_UsesSpecifiedProgram(string programToOpenWith)
+        {
+            string fileToOpen = GetTestFilePath() + ".txt";
+            File.WriteAllText(fileToOpen, $"{nameof(ProcessStart_OpenFileOnOsx_UsesSpecifiedProgram)}");
+            using (var px = Process.Start(programToOpenWith, fileToOpen))
+            {
+                Console.WriteLine($"in OSX, {nameof(programToOpenWith)} is {programToOpenWith}, while {nameof(px.ProcessName)} is {px.ProcessName}.");
+                // Assert.Equal(programToOpenWith, px.ProcessName); // on OSX, process name is dotnet for some reason
+                px.Kill();
+                px.WaitForExit();
+                Assert.True(px.HasExited);
+                Assert.Equal(137, px.ExitCode); // 137 means the process was killed
+            }
+        }
+
+        [Theory, InlineData("Safari"), InlineData("\"Google Chrome\"")]
+        [PlatformSpecific(TestPlatforms.OSX)]
+        // [OuterLoop("Opens browser")]
+        public void ProcessStart_OpenUrl_UsesSpecifiedApplication(string applicationToOpenWith)
+        {
+            using (var px = Process.Start("/usr/bin/open", "https://github.com/dotnet/corefx -a " + applicationToOpenWith))
+            {
+                Assert.False(px.HasExited);
+                px.WaitForExit();
+                Assert.True(px.HasExited);
+                Assert.Equal(0, px.ExitCode); // Exit Code 0 from open means success
+            }
+        }
+
+        [Theory, InlineData("-a Safari"), InlineData("-a \"Google Chrome\"")]
+        [PlatformSpecific(TestPlatforms.OSX)]
+        // [OuterLoop("Opens browser")]
+        public void ProcessStart_UseShellExecuteTrue_OpenUrl_SuccessfullyReadsArgument(string arguments)
+        {
+            var startInfo = new ProcessStartInfo { UseShellExecute = true, FileName = "https://github.com/dotnet/corefx", Arguments = arguments };
             using (var px = Process.Start(startInfo))
             {
                 Assert.NotNull(px);
-                Console.WriteLine($"{nameof(ProcessStart_UseShellExecuteTrue_OpenNano_OpensNano)}(): Process {px.ProcessName} opened {appToOpen}");
-                px.Kill();
+                // px.Kill(); // uncommenting this changes exit code to 137, meaning process was killed
                 px.WaitForExit();
                 Assert.True(px.HasExited);
-                Assert.Equal(137, px.ExitCode);
+                Assert.Equal(0, px.ExitCode);
             }
         }
 
-        [Theory(Skip= "Needs fix for #22299: Should throw"), InlineData(true), InlineData(false)]
-        public void ProcessStart_TryOpenFolder_ThrowsWin32Exception(bool useShellExecute)
+        [Fact, PlatformSpecific(TestPlatforms.OSX)]
+        // [OuterLoop("Opens program")]
+        public void ProcessStart_UseShellExecuteTrue_TryOpenFileThatDoesntExist_ReturnsExitCode1()
         {
-            Win32Exception e = Assert.Throws<Win32Exception>(() => Process.Start(new ProcessStartInfo { UseShellExecute = useShellExecute, FileName = Environment.CurrentDirectory }));
+            string file = Path.Combine(Environment.CurrentDirectory, "_no_such_file.TXT");
+            using (var p = Process.Start(new ProcessStartInfo { UseShellExecute = true, FileName = file }))
+            {
+                Assert.True(p.WaitForExit(WaitInMS));
+                Assert.Equal(1, p.ExitCode); // Exit Code 1 from open means something went wrong
+            }
         }
 
         [Fact]
@@ -167,5 +287,30 @@ namespace System.Diagnostics.Tests
 
         [DllImport("libc")]
         private static extern int chmod(string path, int mode);
+
+        /// <summary>
+        /// Checks if the program is installed
+        /// </summary>
+        /// <param name="program"></param>
+        /// <returns></returns>
+        private bool IsProgramInstalled(string program)
+        {
+            string path;
+            string pathEnvVar = Environment.GetEnvironmentVariable("PATH");
+            if (pathEnvVar != null)
+            {
+                var pathParser = new StringParser(pathEnvVar, ':', skipEmpty: true);
+                while (pathParser.MoveNext())
+                {
+                    string subPath = pathParser.ExtractCurrent();
+                    path = Path.Combine(subPath, program);
+                    if (File.Exists(path))
+                    {
+                        return true;
+                    }
+                }
+            }
+            return false;
+        }
     }
 }
index 895840a..84eb2b4 100644 (file)
@@ -7,8 +7,6 @@
     <DefineConstants Condition="'$(TargetsWindows)' == 'true'">$(DefineConstants);TargetsWindows</DefineConstants>
   </PropertyGroup>
   <!-- Default configurations to help VS understand the configurations -->
-  <PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'netstandard-OSX-Debug|AnyCPU'" />
-  <PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'netstandard-OSX-Release|AnyCPU'" />
   <PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'netstandard-Unix-Debug|AnyCPU'" />
   <PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'netstandard-Unix-Release|AnyCPU'" />
   <PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'netstandard-Windows_NT-Debug|AnyCPU'" />
@@ -37,8 +35,6 @@
     <Compile Condition="'$(TargetGroup)' != 'uap'" Include="ProcessTestBase.NonUap.cs" />
     <Compile Condition="'$(TargetGroup)' == 'uap'" Include="ProcessTestBase.Uap.cs" />
     <Compile Include="ProcessTests.cs" />
-    <Compile Include="ProcessTests.Linux.cs" Condition="'$(TargetsLinux)' == 'true'" />
-    <Compile Include="ProcessTests.OSX.cs" Condition="'$(TargetsOSX)' == 'true'" />
     <Compile Include="ProcessTests.Unix.cs" Condition="'$(TargetsWindows)' != 'true'" />
     <Compile Include="ProcessThreadTests.cs" />
     <Compile Include="ProcessWaitingTests.cs" />