From aef44c0fdc1890f869f9c48635e43693aee5b7af Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Thu, 10 Mar 2016 14:33:52 -0500 Subject: [PATCH] [xbuild] TaskLoggingHelper: Implement support for correctly using task resources - make task resources available to TaskLoggingHelper - verify TaskLoggingHelper method args - tests --- .../TestMessageLogger.cs | 13 + mcs/class/Microsoft.Build.Utilities/Makefile | 13 +- .../Microsoft.Build.Utilities/Task.cs | 9 +- .../TaskLoggingHelper.cs | 100 ++++++-- ...Microsoft.Build.Utilities_test.dll.sources | 2 + .../Microsoft.Build.Utilities/Strings.resx | 103 ++++++++ .../TaskLoggingHelperTest.cs | 233 +++++++++++++++++- 7 files changed, 439 insertions(+), 34 deletions(-) create mode 100644 mcs/class/Microsoft.Build.Utilities/Test/Microsoft.Build.Utilities/Strings.resx diff --git a/mcs/class/Microsoft.Build.Tasks/Test/Microsoft.Build.Tasks/TestMessageLogger.cs b/mcs/class/Microsoft.Build.Tasks/Test/Microsoft.Build.Tasks/TestMessageLogger.cs index c6ca1a23901..55b344dceb6 100644 --- a/mcs/class/Microsoft.Build.Tasks/Test/Microsoft.Build.Tasks/TestMessageLogger.cs +++ b/mcs/class/Microsoft.Build.Tasks/Test/Microsoft.Build.Tasks/TestMessageLogger.cs @@ -197,6 +197,19 @@ namespace MonoTests.Microsoft.Build.Tasks return 0; } + public int CheckFullLog (string text) + { + for (int i = 0; i < all_messages.Count; i ++) { + BuildEventArgs arg = all_messages [i]; + if (text == arg.Message) { + all_messages.RemoveAt (i); + return 0; + } + } + + return 1; + } + public void DumpMessages () { foreach (BuildEventArgs arg in all_messages) diff --git a/mcs/class/Microsoft.Build.Utilities/Makefile b/mcs/class/Microsoft.Build.Utilities/Makefile index cef01f61c75..e44bbc2fe80 100644 --- a/mcs/class/Microsoft.Build.Utilities/Makefile +++ b/mcs/class/Microsoft.Build.Utilities/Makefile @@ -16,7 +16,18 @@ LIB_MCS_FLAGS = \ /r:System.Xml.dll \ /r:$(XBUILD_FRAMEWORK) -TEST_MCS_FLAGS = /r:$(XBUILD_ENGINE) /r:$(XBUILD_FRAMEWORK) -r:System.dll -r:System.Core.dll +TEST_RESX_RESOURCES = Test/Microsoft.Build.Utilities/Strings.resources + +TEST_MCS_FLAGS = /r:$(XBUILD_ENGINE) /r:$(XBUILD_FRAMEWORK) -r:System.dll -r:System.Core.dll $(TEST_RESX_RESOURCES:%=-resource:%) include $(XBUILD_DIR)/xbuild_test.make include ../../build/library.make + +EXTRA_DISTFILES = $(TEST_RESX_RESOURCES:.resources=.resx) + +CLEAN_FILES += $(TEST_RESX_RESOURCES) + +$(TEST_RESX_RESOURCES): %.resources: %.resx + $(RESGEN) $< || cp $@.prebuilt $@ + +$(test_lib): $(TEST_RESX_RESOURCES) diff --git a/mcs/class/Microsoft.Build.Utilities/Microsoft.Build.Utilities/Task.cs b/mcs/class/Microsoft.Build.Utilities/Microsoft.Build.Utilities/Task.cs index d768edb5617..2834c874cbe 100644 --- a/mcs/class/Microsoft.Build.Utilities/Microsoft.Build.Utilities/Task.cs +++ b/mcs/class/Microsoft.Build.Utilities/Microsoft.Build.Utilities/Task.cs @@ -38,7 +38,6 @@ namespace Microsoft.Build.Utilities string helpKeywordPrefix; ITaskHost hostObject; TaskLoggingHelper log; - ResourceManager taskResources; protected Task() : this (null, null) @@ -53,7 +52,8 @@ namespace Microsoft.Build.Utilities protected Task(ResourceManager taskResources, string helpKeywordPrefix) { - this.taskResources = taskResources; + log = new TaskLoggingHelper (this); + log.TaskResources = taskResources; this.helpKeywordPrefix = helpKeywordPrefix; } @@ -65,7 +65,6 @@ namespace Microsoft.Build.Utilities } set { buildEngine = value; - log = new TaskLoggingHelper (this); } } @@ -99,10 +98,10 @@ namespace Microsoft.Build.Utilities protected ResourceManager TaskResources { get { - return taskResources; + return log.TaskResources; } set { - taskResources = value; + log.TaskResources = value; } } } diff --git a/mcs/class/Microsoft.Build.Utilities/Microsoft.Build.Utilities/TaskLoggingHelper.cs b/mcs/class/Microsoft.Build.Utilities/Microsoft.Build.Utilities/TaskLoggingHelper.cs index a274dece6b5..4947f51c11d 100644 --- a/mcs/class/Microsoft.Build.Utilities/Microsoft.Build.Utilities/TaskLoggingHelper.cs +++ b/mcs/class/Microsoft.Build.Utilities/Microsoft.Build.Utilities/TaskLoggingHelper.cs @@ -41,11 +41,14 @@ namespace Microsoft.Build.Utilities string helpKeywordPrefix; string taskName; ResourceManager taskResources; + ITask taskInstance; public TaskLoggingHelper (ITask taskInstance) { - if (taskInstance != null) - this.buildEngine = taskInstance.BuildEngine; + if (taskInstance == null) + throw new ArgumentNullException ("taskInstance"); + + this.taskInstance = taskInstance; taskName = null; } @@ -66,8 +69,15 @@ namespace Microsoft.Build.Utilities { if (resourceName == null) throw new ArgumentNullException ("resourceName"); - - return null; + + if (taskResources == null) + throw new InvalidOperationException ("Task did not register any task resources"); + + string resourceString = taskResources.GetString (resourceName); + if (resourceString == null) + throw new ArgumentException ($"No resource string found for resource named {resourceName}"); + + return FormatString (resourceString, args); } [MonoTODO] @@ -99,11 +109,13 @@ namespace Microsoft.Build.Utilities { if (message == null) throw new ArgumentNullException ("message"); + + ThrowInvalidOperationIf (BuildEngine == null, "Task is attempting to log before it was initialized"); BuildErrorEventArgs beea = new BuildErrorEventArgs ( - null, null, buildEngine.ProjectFileOfTaskNode, 0, 0, 0, 0, FormatString (message, messageArgs), + null, null, BuildEngine.ProjectFileOfTaskNode, 0, 0, 0, 0, FormatString (message, messageArgs), helpKeywordPrefix, null); - buildEngine.LogErrorEvent (beea); + BuildEngine.LogErrorEvent (beea); hasLoggedErrors = true; } @@ -117,12 +129,14 @@ namespace Microsoft.Build.Utilities if (message == null) throw new ArgumentNullException ("message"); + ThrowInvalidOperationIf (BuildEngine == null, "Task is attempting to log before it was initialized"); + BuildErrorEventArgs beea = new BuildErrorEventArgs ( subcategory, errorCode, file, lineNumber, columnNumber, endLineNumber, endColumnNumber, FormatString (message, messageArgs), helpKeyword /*it's helpKeyword*/, null /*it's senderName*/); - buildEngine.LogErrorEvent (beea); + BuildEngine.LogErrorEvent (beea); hasLoggedErrors = true; } @@ -144,14 +158,16 @@ namespace Microsoft.Build.Utilities if (exception == null) throw new ArgumentNullException ("exception"); + ThrowInvalidOperationIf (BuildEngine == null, "Task is attempting to log before it was initialized"); + StringBuilder sb = new StringBuilder (); sb.Append (exception.Message); if (showStackTrace == true) sb.Append (exception.StackTrace); BuildErrorEventArgs beea = new BuildErrorEventArgs ( - null, null, buildEngine.ProjectFileOfTaskNode, 0, 0, 0, 0, sb.ToString (), + null, null, BuildEngine.ProjectFileOfTaskNode, 0, 0, 0, 0, sb.ToString (), exception.HelpLink, exception.Source); - buildEngine.LogErrorEvent (beea); + BuildEngine.LogErrorEvent (beea); hasLoggedErrors = true; } @@ -159,7 +175,7 @@ namespace Microsoft.Build.Utilities params object[] messageArgs) { LogErrorFromResources (null, null, null, null, 0, 0, 0, - 0, messageResourceName, null); + 0, messageResourceName, messageArgs); } public void LogErrorFromResources (string subcategoryResourceName, @@ -172,13 +188,22 @@ namespace Microsoft.Build.Utilities string messageResourceName, params object[] messageArgs) { + if (messageResourceName == null) + throw new ArgumentNullException ("messageResourceName"); + + ThrowInvalidOperationIf (BuildEngine == null, "Task is attempting to log before it was initialized"); + + string subcategory = null; + if (!String.IsNullOrEmpty (subcategoryResourceName)) + subcategory = taskResources.GetString (subcategoryResourceName); + BuildErrorEventArgs beea = new BuildErrorEventArgs ( - taskResources.GetString (subcategoryResourceName), + subcategory, errorCode, file, lineNumber, columnNumber, endLineNumber, endColumnNumber, - taskResources.GetString (messageResourceName), + FormatResourceString (messageResourceName, messageArgs), helpKeyword, null ); - buildEngine.LogErrorEvent (beea); + BuildEngine.LogErrorEvent (beea); hasLoggedErrors = true; } @@ -226,16 +251,17 @@ namespace Microsoft.Build.Utilities public void LogMessageFromResources (string messageResourceName, params object[] messageArgs) { - LogMessage (taskResources.GetString (messageResourceName), - messageArgs); + LogMessageFromResources (MessageImportance.Normal, messageResourceName, messageArgs); } public void LogMessageFromResources (MessageImportance importance, string messageResourceName, params object[] messageArgs) { - LogMessage (importance, taskResources.GetString ( - messageResourceName), messageArgs); + if (messageResourceName == null) + throw new ArgumentNullException ("messageResourceName"); + + LogMessage (importance, FormatResourceString (messageResourceName, messageArgs)); } public bool LogMessagesFromFile (string fileName) @@ -280,10 +306,12 @@ namespace Microsoft.Build.Utilities if (lineOfText == null) throw new ArgumentNullException ("lineOfText"); + ThrowInvalidOperationIf (BuildEngine == null, "Task is attempting to log before it was initialized"); + BuildMessageEventArgs bmea = new BuildMessageEventArgs ( lineOfText, helpKeywordPrefix, null, messageImportance); - buildEngine.LogMessageEvent (bmea); + BuildEngine.LogMessageEvent (bmea); return true; } @@ -291,11 +319,13 @@ namespace Microsoft.Build.Utilities public void LogWarning (string message, params object[] messageArgs) { + ThrowInvalidOperationIf (BuildEngine == null, "Task is attempting to log before it was initialized"); + // FIXME: what about all the parameters? BuildWarningEventArgs bwea = new BuildWarningEventArgs ( - null, null, buildEngine.ProjectFileOfTaskNode, 0, 0, 0, 0, FormatString (message, messageArgs), + null, null, BuildEngine.ProjectFileOfTaskNode, 0, 0, 0, 0, FormatString (message, messageArgs), helpKeywordPrefix, null); - buildEngine.LogWarningEvent (bwea); + BuildEngine.LogWarningEvent (bwea); } public void LogWarning (string subcategory, string warningCode, @@ -305,11 +335,13 @@ namespace Microsoft.Build.Utilities string message, params object[] messageArgs) { + ThrowInvalidOperationIf (BuildEngine == null, "Task is attempting to log before it was initialized"); + BuildWarningEventArgs bwea = new BuildWarningEventArgs ( subcategory, warningCode, file, lineNumber, columnNumber, endLineNumber, endColumnNumber, FormatString (message, messageArgs), helpKeyword, null); - buildEngine.LogWarningEvent (bwea); + BuildEngine.LogWarningEvent (bwea); } public void LogWarningFromException (Exception exception) @@ -331,8 +363,10 @@ namespace Microsoft.Build.Utilities public void LogWarningFromResources (string messageResourceName, params object[] messageArgs) { - LogWarning (taskResources.GetString (messageResourceName), - messageArgs); + if (messageResourceName == null) + throw new ArgumentNullException ("messageResourceName"); + + LogWarningFromResources (null, null, null, null, 0, 0, 0, 0, messageResourceName, messageArgs); } public void LogWarningFromResources (string subcategoryResourceName, @@ -346,11 +380,17 @@ namespace Microsoft.Build.Utilities string messageResourceName, params object[] messageArgs) { - LogWarning (taskResources.GetString (subcategoryResourceName), + if (messageResourceName == null) + throw new ArgumentNullException ("messageResourceName"); + + string subcategory = null; + if (!String.IsNullOrEmpty (subcategoryResourceName)) + subcategory = taskResources.GetString (subcategoryResourceName); + + LogWarning (subcategory, warningCode, helpKeyword, file, lineNumber, columnNumber, endLineNumber, endColumnNumber, - taskResources.GetString (messageResourceName), - messageArgs); + FormatResourceString (messageResourceName, messageArgs)); } public void LogWarningWithCodeFromResources (string messageResourceName, @@ -391,9 +431,15 @@ namespace Microsoft.Build.Utilities { } + void ThrowInvalidOperationIf (bool condition, string message) + { + if (condition) + throw new InvalidOperationException (message); + } + protected IBuildEngine BuildEngine { get { - return buildEngine; + return taskInstance?.BuildEngine; } } diff --git a/mcs/class/Microsoft.Build.Utilities/Microsoft.Build.Utilities_test.dll.sources b/mcs/class/Microsoft.Build.Utilities/Microsoft.Build.Utilities_test.dll.sources index 4e2248a2d48..1b4acda3132 100644 --- a/mcs/class/Microsoft.Build.Utilities/Microsoft.Build.Utilities_test.dll.sources +++ b/mcs/class/Microsoft.Build.Utilities/Microsoft.Build.Utilities_test.dll.sources @@ -4,3 +4,5 @@ Microsoft.Build.Utilities/TaskItemTest.cs Microsoft.Build.Utilities/TaskLoggingHelperTest.cs Microsoft.Build.Utilities/ToolLocationHelperTest.cs Microsoft.Build.Utilities/ToolTaskTest.cs +../../Microsoft.Build.Tasks/Test/Microsoft.Build.Tasks/TestMessageLogger.cs +../../Microsoft.Build.Engine/Test/Microsoft.Build.BuildEngine/Consts.cs diff --git a/mcs/class/Microsoft.Build.Utilities/Test/Microsoft.Build.Utilities/Strings.resx b/mcs/class/Microsoft.Build.Utilities/Test/Microsoft.Build.Utilities/Strings.resx new file mode 100644 index 00000000000..8a1975985ab --- /dev/null +++ b/mcs/class/Microsoft.Build.Utilities/Test/Microsoft.Build.Utilities/Strings.resx @@ -0,0 +1,103 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + text/microsoft-resx + + + 1.3 + + + System.Resources.ResXResourceReader, System.Windows.Forms, Version=1.0.5000.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + System.Resources.ResXResourceWriter, System.Windows.Forms, Version=1.0.5000.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + Message from resources with arg '{0}' + + diff --git a/mcs/class/Microsoft.Build.Utilities/Test/Microsoft.Build.Utilities/TaskLoggingHelperTest.cs b/mcs/class/Microsoft.Build.Utilities/Test/Microsoft.Build.Utilities/TaskLoggingHelperTest.cs index 8ba85db6faf..80174bfd454 100644 --- a/mcs/class/Microsoft.Build.Utilities/Test/Microsoft.Build.Utilities/TaskLoggingHelperTest.cs +++ b/mcs/class/Microsoft.Build.Utilities/Test/Microsoft.Build.Utilities/TaskLoggingHelperTest.cs @@ -3,8 +3,10 @@ // // Author: // Marek Sieradzki (marek.sieradzki@gmail.com) +// Ankit Jain (ankit.jain@xamarin.com) // // (C) 2005 Marek Sieradzki +// (C) 2016 Xamarin, Inc. (http://www.xamarin.com) // // Permission is hereby granted, free of charge, to any person obtaining // a copy of this software and associated documentation files (the @@ -27,15 +29,27 @@ using System; using System.Collections; +using System.Reflection; +using System.Resources; using Microsoft.Build.Framework; using Microsoft.Build.Utilities; +using Microsoft.Build.BuildEngine; using NUnit.Framework; +using MonoTests.Microsoft.Build.Tasks; namespace MonoTests.Microsoft.Build.Utilities { class TestTask : Task { + public static Action action = null; + + public TestTask () + : base (new ResourceManager("Strings", typeof(TestTask).GetTypeInfo().Assembly)) + { + } + public override bool Execute () { + action (Log); return true; } } @@ -45,6 +59,11 @@ namespace MonoTests.Microsoft.Build.Utilities { TaskLoggingHelper tlh; TestTask task; + + public TaskLoggingHelperTest () + { + task = new TestTask (); + } [Test] public void TestAssignment () @@ -77,6 +96,218 @@ namespace MonoTests.Microsoft.Build.Utilities { string output; tlh.ExtractMessageCode (null, out output); } + + [Test] + [ExpectedException (typeof (ArgumentNullException))] + public void TestLogErrorFromResourcesNullMessage () + { + tlh = new TaskLoggingHelper (task); + tlh.LogErrorFromResources (null); + } + + [Test] + [ExpectedException (typeof (ArgumentNullException))] + public void TestLogErrorFromResourcesNullMessage2 () + { + tlh = new TaskLoggingHelper (task); + tlh.LogErrorFromResources (null, null, null, null, 0, 0, 0, 0, null); + } + + [Test] + public void TestLogErrorFromResources1 () + { + RunAndCheckTaskLoggingHelper ( + (tlh) => tlh.LogErrorFromResources ("MessageResource1", "foo"), + (l) => Assert.IsTrue (l.CheckFullLog ("Message from resources with arg 'foo'") == 0, "Message not found") + ); + } + + [Test] + public void TestLogErrorFromResourcesNonExistantResourceName () + { + RunAndCheckTaskLoggingHelper ( + (tlh) => tlh.LogErrorFromResources ("NonExistantResourceName", "foo"), + null, + (p, l) => { + Assert.IsFalse (p.Build (), "Build should have failed"); + Assert.IsTrue (l.CheckFullLog ( + "Error executing task TestTask: No resource string found for resource named NonExistantResourceName") == 0, + "Error not found in the log"); + } + ); + } + + + [Test] + public void TestLogErrorFromResourcesNullSubcategoryResourceName () + { + RunAndCheckTaskLoggingHelper ( + (tlh) => tlh.LogErrorFromResources (null, null, null, null, 0, 0, 0, 0, "MessageResource1", "foo"), + (l) => Assert.IsTrue (l.CheckFullLog ("Message from resources with arg 'foo'") == 0, "Message not found") + ); + } + + [Test] + [ExpectedException (typeof (ArgumentNullException))] + public void TestLogWarningFromResourcesNullMessage () + { + tlh = new TaskLoggingHelper (task); + tlh.LogWarningFromResources (null); + } + + [Test] + [ExpectedException (typeof (ArgumentNullException))] + public void TestLogWarningFromResourcesNullMessage2 () + { + tlh = new TaskLoggingHelper (task); + tlh.LogWarningFromResources (null, null, null, null, 0, 0, 0, 0, null); + } + + [Test] + public void TestLogWarningFromResourcesNullSubcategoryResourceName () + { + RunAndCheckTaskLoggingHelper ( + (tlh) => tlh.LogWarningFromResources (null, null, null, null, 0, 0, 0, 0, "MessageResource1", "foo"), + (l) => Assert.IsTrue (l.CheckFullLog ("Message from resources with arg 'foo'") == 0, "Message not found") + ); + } + + [Test] + public void TestLogWarningFromResources1 () + { + RunAndCheckTaskLoggingHelper ( + (tlh) => tlh.LogWarningFromResources ("MessageResource1", "foo"), + (l) => Assert.IsTrue (l.CheckFullLog ("Message from resources with arg 'foo'") == 0, "Message not found") + ); + } + + [Test] + public void TestLogWarningFromResourcesNonExistantResourceName () + { + RunAndCheckTaskLoggingHelper ( + (tlh) => tlh.LogWarningFromResources ("NonExistantResourceName", "foo"), + null, + (p, l) => { + if (p.Build ()) { l.DumpMessages (); Assert.Fail ("Build should have failed"); } + Assert.IsTrue (l.CheckFullLog ( + "Error executing task TestTask: No resource string found for resource named NonExistantResourceName") == 0, + "Error not found in the log"); + } + ); + } + + + [Test] + [ExpectedException (typeof (ArgumentNullException))] + public void TestLogMessageFromResourcesNullMessage () + { + tlh = new TaskLoggingHelper (task); + tlh.LogMessageFromResources (null); + } + + [Test] + [ExpectedException (typeof (ArgumentNullException))] + public void TestLogMessageFromResourcesNullMessage2 () + { + tlh = new TaskLoggingHelper (task); + tlh.LogMessageFromResources (MessageImportance.Low, null); + } + + [Test] + public void TestLogMessageFromResources1 () + { + RunAndCheckTaskLoggingHelper ( + (tlh) => tlh.LogMessageFromResources ("MessageResource1", "foo"), + (l) => Assert.IsTrue (l.CheckFullLog ("Message from resources with arg 'foo'") == 0, "Message not found") + ); + } + + [Test] + public void TestLogMessageFromResourcesNonExistantResourceName () + { + RunAndCheckTaskLoggingHelper ( + (tlh) => tlh.LogMessageFromResources ("NonExistantResourceName", "foo"), + null, + (p, l) => { + if (p.Build ()) { l.DumpMessages (); Assert.Fail ("Build should have failed"); } + l.DumpMessages (); + Assert.IsTrue (l.CheckFullLog ( + "Error executing task TestTask: No resource string found for resource named NonExistantResourceName") == 0, + "Error not found in the log"); + } + ); + } + + [Test] + [ExpectedException (typeof (ArgumentNullException))] + public void TestFormatResourceString1 () + { + tlh = new TaskLoggingHelper (task); + tlh.FormatResourceString (null); + } + + [Test] + [ExpectedException (typeof (InvalidOperationException))] + public void TestFormatResourceString2 () + { + tlh = new TaskLoggingHelper (task); + tlh.FormatResourceString ("MessageResource1"); + } + + [Test] + public void TestFormatResourceString3 () + { + RunAndCheckTaskLoggingHelper ( + (tlh) => tlh.FormatResourceString ("NonExistantResourceName"), + null, + (p, l) => { + if (p.Build ()) { l.DumpMessages (); Assert.Fail ("Build should have failed"); } + l.DumpMessages (); + Assert.IsTrue (l.CheckFullLog ( + "Error executing task TestTask: No resource string found for resource named NonExistantResourceName") == 0, + "Error not found in the log"); + } + ); + } + + [Test] + public void TestFormatResourceString4 () + { + RunAndCheckTaskLoggingHelper ( + (tlh) => Assert.AreEqual ( + tlh.FormatResourceString ("MessageResource1", "foo"), + "Message from resources with arg 'foo'"), + null + ); + } + void RunAndCheckTaskLoggingHelper (Action taskAction, Action loggerAction, Action projectBuildAction = null) + { + string asmLocation = typeof (TaskLoggingHelperTest).Assembly.Location; + string project_xml = @" + + + + + "; + + Engine engine = new Engine (Consts.BinPath); + Project proj = engine.CreateNewProject (); + proj.LoadXml (project_xml); + TestMessageLogger logger = new TestMessageLogger (); + engine.RegisterLogger (logger); + + TestTask.action = taskAction; + + if (projectBuildAction == null) { + if (!proj.Build ("1")) { + logger.DumpMessages (); + Assert.Fail ("Build failed"); + } + } else + projectBuildAction (proj, logger); + + if (loggerAction != null) + loggerAction (logger); + } } - } -- 2.25.1