Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
26 changes: 26 additions & 0 deletions SystemInterface/Diagnostics/IProcess.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ public interface IProcess
/// </summary>
IProcessStartInfo StartInfo { get; set; }

/// <summary>
/// Immediately stops the associated process.
/// </summary>
void Kill();

/// <summary>
/// Instructs the ProcessInstance component to wait indefinitely for the associated process to exit.
/// </summary>
Expand Down Expand Up @@ -73,6 +78,27 @@ public interface IProcess
[MonitoringDescription("ProcessStandardOutput"), DesignerSerializationVisibility(DesignerSerializationVisibility.Hidden), Browsable(false)]
IStreamReader StandardOutput { get; }

/// <summary>
/// Gets a new System.Diagnostics.Process component and associates it with the currently active process.
/// </summary>
/// <returns>A new System.Diagnostics.Process component associated with the process resource that is running the calling application.</returns>
Process GetCurrentProcess();
Copy link
Owner

Choose a reason for hiding this comment

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

This should return the IProcess interface to allow proper mocking.

Copy link
Author

Choose a reason for hiding this comment

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

Implementation of this method calls static Process.GetCurrentProcess which returns object of Process class. If we would like to return IProcess we would have to return here ProcessWrap instance or any other class implementing IProcess that would have to be created.
This would make the code a little bit more complicated. Although there are benefits for returning IProcess here, I believe that in majority of cases current implementation should be enough.

If someone would see benefits for himself having IProcess being returned I'm happy to implement it. For our needs it's enough to just return Process object

Copy link
Owner

Choose a reason for hiding this comment

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

The point of this library is test-ability of the code and easy mocking. If the return value cannot be mocked, there is no point in implementing this API in this library.

Copy link
Author

Choose a reason for hiding this comment

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

Changed approach. I introduced IProcessFactory that is responsible for creating IProcess object. So basically all Process static methods are placed there.
Now we are able to return IProcess instead of Process


/// <summary>
/// Returns a new System.Diagnostics.Process component, given the identifier of a process on the local computer.
/// </summary>
/// <param name="processId">The system-unique identifier of a process resource.</param>
/// <returns>A System.Diagnostics.Process component that is associated with the local process resource identified by the processId parameter.</returns>
Process GetProcessById(int processId);
Copy link
Owner

Choose a reason for hiding this comment

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

This should return the IProcess interface to allow proper mocking.


/// <summary>
/// Returns a new System.Diagnostics.Process component, given the identifier of a process on on the network.
/// </summary>
/// <param name="processId">The system-unique identifier of a process resource.</param>
/// <param name="machineName">The name of a computer on the network.</param>
/// <returns>A System.Diagnostics.Process component that is associated with a remote process resource identified by the processId parameter.</returns>
Process GetProcessById(int processId, string machineName);
Copy link
Owner

Choose a reason for hiding this comment

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

This should return the IProcess interface to allow proper mocking.


/*

// Events
Expand Down
30 changes: 28 additions & 2 deletions SystemWrapper/Diagnostics/ProcessWrap.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
using System;
using System.Diagnostics;
using SystemInterface.Diagnostics;
using SystemInterface.IO;
Expand Down Expand Up @@ -56,14 +55,23 @@ public bool Start()
/// <inheritdoc />
public IProcessStartInfo StartInfo
{
get { return this._startInfo ?? (this._startInfo = new ProcessStartInfoWrap(ProcessInstance.StartInfo)); }
get
{
return this._startInfo ?? (this._startInfo = new ProcessStartInfoWrap(ProcessInstance.StartInfo));
}
set
{
this._startInfo = value;
ProcessInstance.StartInfo = _startInfo.ProcessStartInfoInstance;
}
}

/// <inheritdoc />
public void Kill()
{
ProcessInstance.Kill();
}

/// <inheritdoc />
public IStreamReader StandardOutput
{
Expand All @@ -90,5 +98,23 @@ public bool WaitForInputIdle()
{
return ProcessInstance.WaitForInputIdle();
}

/// <inheritdoc />
public Process GetCurrentProcess()
{
return Process.GetCurrentProcess();
}

/// <inheritdoc />
public Process GetProcessById(int processId)
{
return Process.GetProcessById(processId);
}

/// <inheritdoc />
public Process GetProcessById(int processId, string machineName)
{
return Process.GetProcessById(processId, machineName);
}
}
}