Skip to content
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

Add enhancement: trx logger can take logfile parameter #282

Merged
merged 15 commits into from
Dec 26, 2016
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -385,8 +385,7 @@ private Dictionary<string, string> UpdateLoggerParamters(Dictionary<string, stri
}

// Add default logger parameters...
// todo Read Output Directory from RunSettings
loggerParams[DefaultLoggerParameterNames.TestRunDirectory] = null;
loggerParams[DefaultLoggerParameterNames.TestRunDirectory] = this.GetResultsDirectory(RunSettingsManager.Instance.ActiveRunSettings);
return loggerParams;
}

Expand Down
117 changes: 74 additions & 43 deletions src/Microsoft.TestPlatform.Extensions.TrxLogger/TrxLogger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ namespace Microsoft.VisualStudio.TestPlatform.Extensions.TrxLogger
/// </summary>
[FriendlyName(TrxLogger.FriendlyName)]
[ExtensionUri(TrxLogger.ExtensionUri)]
internal class TrxLogger : ITestLogger
internal class TrxLogger : ITestLoggerWithParameters
{
#region Constants

Expand All @@ -47,14 +47,21 @@ internal class TrxLogger : ITestLogger
/// </summary>
public const string DataCollectorUriPrefix = "dataCollector://";

/// <summary>
/// Log file parameter key
/// </summary>
public const string LogFileParameterKey = "logfile";

#endregion

#region Fields


Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: Remove extra lines.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed.


/// <summary>
/// Cache the TRX filename
/// Cache the TRX file path
/// </summary>
private static string trxFileName;
private static string trxFilePath;

private TrxLoggerObjectModel.TestRun testRun;
private List<TrxLoggerObjectModel.UnitTestResult> results;
Expand All @@ -75,6 +82,11 @@ internal class TrxLogger : ITestLogger

private DateTime testRunStartTime;

/// <summary>
/// Parameters dictionary for logger. Ex: {"logfile":"c:pathto/logfile.trx"}.
/// </summary>
private Dictionary<string, string> parametersDictionary;

#endregion

/// <summary>
Expand All @@ -89,33 +101,45 @@ public static string TrxFileDirectory

#region ITestLogger

/// <summary>
/// Initializes the Test Logger.
/// </summary>
/// <param name="events">Events that can be registered for.</param>
/// <param name="testRunDirectory">Test Run Directory</param>
public void Initialize(TestLoggerEvents events, string testRunDirectory)
/// <inheritdoc/>
public void Initialize(TestLoggerEvents events, string testResultsDirPath)
Copy link
Contributor

Choose a reason for hiding this comment

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

When will this method get called?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is overloaded method, which calls it.

{
if (events == null)
{
throw new ArgumentNullException(nameof(events));
}

if (string.IsNullOrEmpty(testRunDirectory))
if (string.IsNullOrEmpty(testResultsDirPath))
{
throw new ArgumentNullException(nameof(testRunDirectory));
throw new ArgumentNullException(nameof(testResultsDirPath));
}

// Register for the events.
events.TestRunMessage += this.TestMessageHandler;
events.TestResult += this.TestResultHandler;
events.TestRunComplete += this.TestRunCompleteHandler;

TrxFileDirectory = testRunDirectory;
TrxFileDirectory = testResultsDirPath;

this.InitializeInternal();
}

/// <inheritdoc/>
public void Initialize(TestLoggerEvents events, Dictionary<string, string> parameters)
{
if (parameters == null)
{
throw new ArgumentNullException(nameof(parameters));
}

if (parameters.Count == 0)
{
throw new ArgumentException("No default parameters added", nameof(parameters));
}

this.parametersDictionary = parameters;
this.Initialize(events, this.parametersDictionary[DefaultLoggerParameterNames.TestRunDirectory]);
}
#endregion

#region ForTesting
Expand Down Expand Up @@ -358,19 +382,9 @@ internal void TestRunCompleteHandler(object sender, TestRunCompleteEventArgs e)

helper.SaveObject(runSummary, rootElement, "ResultSummary", parameters);

if (Directory.Exists(TrxFileDirectory) == false)
{
Directory.CreateDirectory(TrxFileDirectory);
}

if (string.IsNullOrEmpty(trxFileName))
{
// save the xml to file in testResultsFolder
// [RunDeploymentRootDirectory] Replace white space with underscore from trx file name to make it command line friendly
trxFileName = this.GetTrxFileName(TrxFileDirectory, this.testRun.RunConfiguration.RunDeploymentRootDirectory.Replace(' ', '_'));
}

this.PopulateTrxFile(trxFileName, rootElement);
//Save results to Trx file
this.DeriveTrxFilePath();
this.PopulateTrxFile(trxFilePath, rootElement);
}
}

Expand Down Expand Up @@ -398,24 +412,6 @@ internal virtual void PopulateTrxFile(string trxFileName, XmlElement rootElement
}
}

/// <summary>
/// Get full path to trx file
/// </summary>
/// <param name="baseDirectory">
/// The base Directory.
/// </param>
/// <param name="trxFileName">
/// The trx File Name.
/// </param>
/// <returns>
/// trx file name.
/// </returns>
[SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1650:ElementDocumentationMustBeSpelledCorrectly", Justification = "Reviewed. Suppression is OK here.")]
private string GetTrxFileName(string baseDirectory, string trxFileName)
{
return FileHelper.GetNextIterationFileName(baseDirectory, trxFileName + ".trx", false);
}

// Initializes trx logger cache.
private void InitializeInternal()
{
Expand Down Expand Up @@ -454,6 +450,41 @@ private void HandleSkippedTest(ObjectModel.TestResult rsTestResult)
this.AddRunLevelInformationalMessage(message);
}

private void DeriveTrxFilePath()
{
if (this.parametersDictionary != null && this.parametersDictionary.ContainsKey(LogFileParameterKey) && !string.IsNullOrWhiteSpace(this.parametersDictionary[LogFileParameterKey]))
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider using Dictionary.TryGetValue. Use the value obtained for further checks instead of multiple dictionary lookups.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

make sense, made changes.

{
this.SetTrxFilePathFromParameters();
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we inline these methods. Some common code like creating a results directory could get combined.

}
else
{
this.SetDefaultTrxFilePath();
}
}

private void SetTrxFilePathFromParameters()
{
TrxLogger.trxFilePath = Path.GetFullPath(this.parametersDictionary[LogFileParameterKey]);
var trxFileDirPath = new FileInfo(TrxLogger.trxFilePath).DirectoryName;
if (Directory.Exists(trxFileDirPath) == false)
{
Directory.CreateDirectory(trxFileDirPath);
}
}

private void SetDefaultTrxFilePath()
{
// save the trx file to testResultsDirectory
if (Directory.Exists(TrxFileDirectory) == false)
Copy link
Contributor

Choose a reason for hiding this comment

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

It is confusing to use TrxFileDirectory in one case and derive the directory in the other.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed it to testResultsDirPath.

{
Directory.CreateDirectory(TrxFileDirectory);
}

// [RunDeploymentRootDirectory] Replace white space with underscore from trx file name to make it command line friendly
var defaultTrxFileName = this.testRun.RunConfiguration.RunDeploymentRootDirectory.Replace(' ', '_') + ".trx";
TrxLogger.trxFilePath = FileHelper.GetNextIterationFileName(TrxFileDirectory, defaultTrxFileName, false);
}

#endregion
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ private void AddLoggerByUri(string argument, Dictionary<string, string> paramete
throw new CommandLineException(e.Message, e);
}
}

#endregion
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,16 @@ public void AddLoggerShouldNotThrowExceptionIfUriIsNonExistent()
});
}

[TestMethod]
public void AddLoggerShouldAddDefaultLoggerParameterForTestLoggerWithParameters()
{
ValidLoggerWithParameters.Reset();
TestLoggerManager.Instance.AddLogger(new Uri("test-logger-with-parameter://logger"), new Dictionary<string, string>());
Assert.IsNotNull(ValidLoggerWithParameters.parameters, "parameters not getting passed");
Assert.IsTrue(ValidLoggerWithParameters.parameters.ContainsKey(DefaultLoggerParameterNames.TestRunDirectory), $"{DefaultLoggerParameterNames.TestRunDirectory} not added to parameters");
Assert.IsFalse(string.IsNullOrWhiteSpace(ValidLoggerWithParameters.parameters[DefaultLoggerParameterNames.TestRunDirectory]), $"parameter {DefaultLoggerParameterNames.TestRunDirectory} should not be null, empty or whitespace");
Copy link
Contributor

Choose a reason for hiding this comment

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

indent

}

[TestMethod]
public void DisposeShouldNotThrowExceptionIfCalledMultipleTimes()
{
Expand Down Expand Up @@ -393,6 +403,27 @@ private void TestMessageHandler(object sender, TestRunMessageEventArgs e)

}

[ExtensionUri("test-logger-with-parameter://logger")]
[FriendlyName("TestLoggerWithParameterExtension")]
private class ValidLoggerWithParameters : ITestLoggerWithParameters
{
public static Dictionary<string, string> parameters;
public void Initialize(TestLoggerEvents events, string testRunDirectory)
{

}

public void Initialize(TestLoggerEvents events, Dictionary<string, string> parameters)
{
ValidLoggerWithParameters.parameters = parameters;
}

public static void Reset()
{
ValidLoggerWithParameters.parameters = null;
}
}

internal class DummyTestLoggerManager : TestLoggerManager
{
public DummyTestLoggerManager()
Expand Down
Loading