-
Notifications
You must be signed in to change notification settings - Fork 723
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
Allow passing arguments into the test run #1570
Changes from all commits
9e5c690
1524aef
7cb5c3d
911e498
1f2ed08
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,94 @@ | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Text; | ||
|
||
namespace NUnit.Framework | ||
{ | ||
/// <summary> | ||
/// TestParameters class holds any named parameters supplied to the test run | ||
/// </summary> | ||
public class TestParameters | ||
{ | ||
private readonly Dictionary<string, string> _parameters = new Dictionary<string, string>(); | ||
|
||
/// <summary> | ||
/// Gets the number of test parameters | ||
/// </summary> | ||
public int Count | ||
{ | ||
get { return _parameters.Count; } | ||
} | ||
|
||
/// <summary> | ||
/// Gets a collection of the test parameter names | ||
/// </summary> | ||
public ICollection<string> Names | ||
{ | ||
get { return _parameters.Keys; } | ||
} | ||
|
||
/// <summary> | ||
/// Gets a flag indicating whether a parameter with the specified name exists.N | ||
/// </summary> | ||
/// <param name="name">Name of the parameter</param> | ||
/// <returns>True if it exists, otherwise false</returns> | ||
public bool Exists(string name) | ||
{ | ||
return _parameters.ContainsKey(name); | ||
} | ||
|
||
/// <summary> | ||
/// Indexer provides access to the internal dictionary | ||
/// </summary> | ||
/// <param name="name">Name of the parameter</param> | ||
/// <returns>Value of the parameter or null if not present</returns> | ||
public string this[string name] | ||
{ | ||
get { return Get(name); } | ||
} | ||
|
||
/// <summary> | ||
/// Get method is a simple alternative to the indexer | ||
/// </summary> | ||
/// <param name="name">Name of the paramter</param> | ||
/// <returns>Value of the parameter or null if not present</returns> | ||
public string Get(string name) | ||
{ | ||
return Exists(name) ? _parameters[name] : null; | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is very inefficient. Should be: string value; return value; There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I used Exists for clarity, but it's basically as if I wrote... if (_parameters.ContainsKey(name) return _parameters[name]; This is a pattern we use all over NUnit. If memory serves, it's because TryGetValue doesn't (or maybe didn't) exist on some platforms. As far as I have always understood, it's a little bit inefficient, but not enough to make me want to prematurely optimize it, especially at the expense of clarity. The various TryXxx functions always seem very hard to read. Do you have an estimate of how inefficient it is in a typical use? For that matter, since it's new, do we have any idea what typical use will be? I'm thinking of some small number of users having maybe three or four parameters so maybe that biases my approach. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using exists and then getting the value requires two completely independent lookups in the dictionary. Trygetvalue only does a single lookup. I never use the pattern you used. As far as I know every platform has trygetvalue for the generic dictionary class. ---Sent from Boxer | http://getboxer.com On June 17, 2016 at 9:08:04 AM GMT+8, CharliePoole notifications@github.com wrote:In src/NUnitFramework/framework/TestParameters.cs: > + /// Value of the parameter or null if not present > + public string this[string name] > + { > + get { return Get(name); } > + } > + > + ///
|
||
/// <summary> | ||
/// Get the value of a parameter or a default string | ||
/// </summary> | ||
/// <param name="name">Name of the parameter</param> | ||
/// <param name="defaultValue">Default value of the parameter</param> | ||
/// <returns>Value of the parameter or default value if not present</returns> | ||
public string Get(string name, string defaultValue) | ||
{ | ||
return Get(name) ?? defaultValue; | ||
} | ||
|
||
/// <summary> | ||
/// Get the value of a parameter or return a default | ||
/// </summary> | ||
/// <typeparam name="T">The return Type</typeparam> | ||
/// <param name="name">Name of the parameter</param> | ||
/// <param name="defaultValue">Default value of the parameter</param> | ||
/// <returns>Value of the parameter or default value if not present</returns> | ||
public T Get<T>(string name, T defaultValue) | ||
{ | ||
string val = Get(name); | ||
return val != null ? (T)Convert.ChangeType(val, typeof(T), null) : defaultValue; | ||
} | ||
|
||
/// <summary> | ||
/// Adds a parameter to the list | ||
/// </summary> | ||
/// <param name="name">Name of the parameter</param> | ||
/// <param name="value">Value of the parameter</param> | ||
internal void Add(string name, string value) | ||
{ | ||
_parameters[name] = value; | ||
} | ||
} | ||
} |
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.
Based on #1571, should we be using semi-colons, or is there an easy workaround to that issue?
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.
Let me try it out when I get home to my Linux machine. We have had the result option for a long time and nobody has brought up the semicolon before.
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.
I don't see a problem. I commented on #1571