diff --git a/src/Runner.Worker/ActionCommandManager.cs b/src/Runner.Worker/ActionCommandManager.cs index 5f0a7438c..42230588b 100644 --- a/src/Runner.Worker/ActionCommandManager.cs +++ b/src/Runner.Worker/ActionCommandManager.cs @@ -270,13 +270,10 @@ namespace GitHub.Runner.Worker if (string.Equals(blocked, envName, StringComparison.OrdinalIgnoreCase)) { // Log Telemetry and let user know they shouldn't do this - var issue = new Issue() - { - Type = IssueType.Error, - Message = $"Can't update {blocked} environment variable using ::set-env:: command." - }; - issue.Data[Constants.Runner.InternalTelemetryIssueDataKey] = $"{Constants.Runner.UnsupportedCommand}_{envName}"; - context.AddIssue(issue, writeToLog: true); + var message = $"Can't update {blocked} environment variable using ::set-env:: command."; + var metadata = new IssueMetadata(Constants.Runner.InternalTelemetryIssueDataKey, $"{Constants.Runner.UnsupportedCommand}_{envName}"); + var issue = context.CreateIssue(IssueType.Error, message, metadata, true); + context.AddIssue(issue); return; } @@ -309,13 +306,10 @@ namespace GitHub.Runner.Worker { if (context.Global.Variables.GetBoolean("DistributedTask.DeprecateStepOutputCommands") ?? false) { - var issue = new Issue() - { - Type = IssueType.Warning, - Message = String.Format(Constants.Runner.UnsupportedCommandMessage, this.Command) - }; - issue.Data[Constants.Runner.InternalTelemetryIssueDataKey] = Constants.Runner.UnsupportedCommand; - context.AddIssue(issue, writeToLog: true); + var message = string.Format(Constants.Runner.UnsupportedCommandMessage, this.Command); + var metadata = new IssueMetadata(Constants.Runner.InternalTelemetryIssueDataKey, Constants.Runner.UnsupportedCommand); + var issue = context.CreateIssue(IssueType.Warning, message, metadata, true); + context.AddIssue(issue); } if (!command.Properties.TryGetValue(SetOutputCommandProperties.Name, out string outputName) || string.IsNullOrEmpty(outputName)) @@ -344,13 +338,10 @@ namespace GitHub.Runner.Worker { if (context.Global.Variables.GetBoolean("DistributedTask.DeprecateStepOutputCommands") ?? false) { - var issue = new Issue() - { - Type = IssueType.Warning, - Message = String.Format(Constants.Runner.UnsupportedCommandMessage, this.Command) - }; - issue.Data[Constants.Runner.InternalTelemetryIssueDataKey] = Constants.Runner.UnsupportedCommand; - context.AddIssue(issue, writeToLog: true); + var message = string.Format(Constants.Runner.UnsupportedCommandMessage, this.Command); + var metadata = new IssueMetadata(Constants.Runner.InternalTelemetryIssueDataKey, Constants.Runner.UnsupportedCommand); + var issue = context.CreateIssue(IssueType.Warning, message, metadata, true); + context.AddIssue(issue); } if (!command.Properties.TryGetValue(SaveStateCommandProperties.Name, out string stateName) || string.IsNullOrEmpty(stateName)) @@ -618,16 +609,11 @@ namespace GitHub.Runner.Worker context.Debug("Enhanced Annotations not enabled on the server. The 'title', 'end_line', and 'end_column' fields are unsupported."); } - Issue issue = new() - { - Category = "General", - Type = this.Type, - Message = command.Data - }; + var issueCategory = "General"; if (!string.IsNullOrEmpty(file)) { - issue.Category = "Code"; + issueCategory = "Code"; if (container != null) { @@ -658,15 +644,13 @@ namespace GitHub.Runner.Worker } } - foreach (var property in command.Properties) - { - if (!string.Equals(property.Key, Constants.Runner.InternalTelemetryIssueDataKey, StringComparison.OrdinalIgnoreCase)) - { - issue.Data[property.Key] = property.Value; - } - } + string keyToExclude = Constants.Runner.InternalTelemetryIssueDataKey; + var filteredDictionaryEntries = command.Properties + .Where(kvp => !string.Equals(kvp.Key, keyToExclude, StringComparison.OrdinalIgnoreCase)); - context.AddIssue(issue, writeToLog: true); + var metadata = new IssueMetadata(issueCategory, false, filteredDictionaryEntries); + var issue = context.CreateIssue(this.Type, command.Data, metadata, true); + context.AddIssue(issue); } public static void ValidateLinesAndColumns(ActionCommand command, IExecutionContext context) diff --git a/src/Runner.Worker/ExecutionContext.cs b/src/Runner.Worker/ExecutionContext.cs index e9e7dbf4f..ec59b029d 100644 --- a/src/Runner.Worker/ExecutionContext.cs +++ b/src/Runner.Worker/ExecutionContext.cs @@ -91,7 +91,8 @@ namespace GitHub.Runner.Worker void SetGitHubContext(string name, string value); void SetOutput(string name, string value, out string reference); void SetTimeout(TimeSpan? timeout); - void AddIssue(Issue issue, bool writeToLog); + IReadOnlyIssue CreateIssue(IssueType issueType, string rawMessage, IssueMetadata metadata, bool writeToLog); + void AddIssue(IReadOnlyIssue issue); void Progress(int percentage, string currentOperation = null); void UpdateDetailTimelineRecord(TimelineRecord record); @@ -125,7 +126,7 @@ namespace GitHub.Runner.Worker private readonly TimelineRecord _record = new(); private readonly Dictionary _detailRecords = new(); - private readonly List _embeddedIssueCollector; + private readonly List _embeddedIssueCollector; private readonly object _loggerLock = new(); private readonly object _matchersLock = new(); private readonly ExecutionContext _parentExecutionContext; @@ -447,7 +448,7 @@ namespace GitHub.Runner.Worker { foreach (var issue in _embeddedIssueCollector) { - AddIssue(issue, writeToLog: false); + AddIssue(issue); } } @@ -579,23 +580,29 @@ namespace GitHub.Runner.Worker } // This is not thread safe, the caller needs to take lock before calling issue() - public void AddIssue(Issue issue, bool writeToLog) + public IReadOnlyIssue CreateIssue(IssueType issueType, string rawMessage, IssueMetadata metadata, bool writeToLog) { - ArgUtil.NotNull(issue, nameof(issue)); + string refinedMessage = PrimitiveExtensions.TrimExcess(HostContext.SecretMasker.MaskSecrets(rawMessage), _maxIssueMessageLength); - string refinedMessage = PrimitiveExtensions.TrimExcess(HostContext.SecretMasker.MaskSecrets(issue.Message), _maxIssueMessageLength); - issue.Message = refinedMessage; + var result = new Issue() { + Type = issueType, + Category = metadata?.Category, + IsInfrastructureIssue = metadata?.IsInfrastructureIssue ?? false, + Message = refinedMessage, + }; + + result.Data.AddRangeIfRangeNotNull(metadata?.Data); // It's important to keep track of the step number (key:stepNumber) and the line number (key:logFileLineNumber) of every issue that gets logged. // Actions UI from the run summary page use both values to easily link to an exact locations in logs where annotations originate from. if (_record.Order != null) { - issue.Data["stepNumber"] = _record.Order.ToString(); + result.Data["stepNumber"] = _record.Order.ToString(); } string wellKnownTag = null; Int32? previousCountForIssueType = null; - switch (issue.Type) + switch (issueType) { case IssueType.Error: wellKnownTag = WellKnownTags.Error; @@ -616,14 +623,23 @@ namespace GitHub.Runner.Worker if (writeToLog && !string.IsNullOrEmpty(refinedMessage)) { long logLineNumber = Write(wellKnownTag, refinedMessage); - issue.Data["logFileLineNumber"] = logLineNumber.ToString(); + result.Data["logFileLineNumber"] = logLineNumber.ToString(); } if (previousCountForIssueType.GetValueOrDefault(0) < _maxIssueCount) { - _record.Issues.Add(issue); + _record.Issues.Add(result); } } + return result; + } + + + // This is not thread safe, the caller needs to take lock before calling issue() + public void AddIssue(IReadOnlyIssue issue) + { + ArgUtil.NotNull(issue, nameof(issue)); + // Embedded ExecutionContexts (a.k.a. Composite actions) should never upload a timeline record to the server. // Instead, we store processed issues on a shared (psuedo-inherited) list (belonging to the closest // non-embedded ancestor ExecutionContext) so that they can be processed when that ancestor completes. @@ -1169,22 +1185,23 @@ namespace GitHub.Runner.Worker // Do not add a format string overload. See comment on ExecutionContext.Write(). public static void Error(this IExecutionContext context, string message) { - var issue = new Issue() { Type = IssueType.Error, Message = message }; - context.AddIssue(issue, writeToLog: true); + var issue = context.CreateIssue(IssueType.Error, message, null, true); + context.AddIssue(issue); } // Do not add a format string overload. See comment on ExecutionContext.Write(). public static void InfrastructureError(this IExecutionContext context, string message) { - var issue = new Issue() { Type = IssueType.Error, Message = message, IsInfrastructureIssue = true }; - context.AddIssue(issue, writeToLog: true); + var metadata = new IssueMetadata(null, true, Enumerable.Empty>()); + var issue = context.CreateIssue(IssueType.Error, message, metadata, true); + context.AddIssue(issue); } // Do not add a format string overload. See comment on ExecutionContext.Write(). public static void Warning(this IExecutionContext context, string message) { - var issue = new Issue() { Type = IssueType.Warning, Message = message }; - context.AddIssue(issue, writeToLog: true); + var issue = context.CreateIssue(IssueType.Warning, message, null, true); + context.AddIssue(issue); } // Do not add a format string overload. See comment on ExecutionContext.Write(). diff --git a/src/Runner.Worker/Handlers/OutputManager.cs b/src/Runner.Worker/Handlers/OutputManager.cs index dc683422f..3f5301e15 100644 --- a/src/Runner.Worker/Handlers/OutputManager.cs +++ b/src/Runner.Worker/Handlers/OutputManager.cs @@ -4,6 +4,7 @@ using System.Globalization; using System.IO; using System.Linq; using System.Text.RegularExpressions; +using GitHub.DistributedTask.WebApi; using GitHub.Runner.Common; using GitHub.Runner.Sdk; using GitHub.Runner.Worker.Container; @@ -141,7 +142,7 @@ namespace GitHub.Runner.Worker.Handlers if (issue != null) { // Log issue - _executionContext.AddIssue(issue, writeToLog: true); + _executionContext.AddIssue(issue); return; } } @@ -193,7 +194,7 @@ namespace GitHub.Runner.Worker.Handlers } } - private DTWebApi.Issue ConvertToIssue(IssueMatch match) + private DTWebApi.IReadOnlyIssue ConvertToIssue(IssueMatch match) { // Validate the message if (string.IsNullOrWhiteSpace(match.Message)) @@ -222,18 +223,14 @@ namespace GitHub.Runner.Worker.Handlers return null; } - var issue = new DTWebApi.Issue - { - Message = match.Message, - Type = issueType, - }; + var issueData = new Dictionary(); // Line if (!string.IsNullOrEmpty(match.Line)) { if (int.TryParse(match.Line, NumberStyles.None, CultureInfo.InvariantCulture, out var line)) { - issue.Data["line"] = line.ToString(CultureInfo.InvariantCulture); + issueData["line"] = line.ToString(CultureInfo.InvariantCulture); } else { @@ -246,7 +243,7 @@ namespace GitHub.Runner.Worker.Handlers { if (int.TryParse(match.Column, NumberStyles.None, CultureInfo.InvariantCulture, out var column)) { - issue.Data["col"] = column.ToString(CultureInfo.InvariantCulture); + issueData["col"] = column.ToString(CultureInfo.InvariantCulture); } else { @@ -257,7 +254,7 @@ namespace GitHub.Runner.Worker.Handlers // Code if (!string.IsNullOrWhiteSpace(match.Code)) { - issue.Data["code"] = match.Code.Trim(); + issueData["code"] = match.Code.Trim(); } // File @@ -309,7 +306,7 @@ namespace GitHub.Runner.Worker.Handlers var relativePath = file.Substring(repositoryPath.Length).TrimStart(Path.DirectorySeparatorChar); // Prefer `/` on all platforms - issue.Data["file"] = relativePath.Replace(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); + issueData["file"] = relativePath.Replace(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); } else { @@ -324,9 +321,11 @@ namespace GitHub.Runner.Worker.Handlers } catch (Exception ex) { - _executionContext.Debug($"Dropping file value '{match.File}' and fromPath value '{match.FromPath}'. Exception during validation: {ex.ToString()}"); + _executionContext.Debug($"Dropping file value '{match.File}' and fromPath value '{match.FromPath}'. Exception during validation: {ex}"); } + var metadata = new IssueMetadata(null, false, issueData); + var issue = _executionContext.CreateIssue(issueType, match.Message, metadata, true); return issue; } diff --git a/src/Runner.Worker/JobExtension.cs b/src/Runner.Worker/JobExtension.cs index ec91510e1..52f039d31 100644 --- a/src/Runner.Worker/JobExtension.cs +++ b/src/Runner.Worker/JobExtension.cs @@ -660,9 +660,10 @@ namespace GitHub.Runner.Worker var freeSpaceInMB = driveInfo.AvailableFreeSpace / 1024 / 1024; if (freeSpaceInMB < lowDiskSpaceThreshold) { - var issue = new Issue() { Type = IssueType.Warning, Message = $"You are running out of disk space. The runner will stop working when the machine runs out of disk space. Free space left: {freeSpaceInMB} MB" }; - issue.Data[Constants.Runner.InternalTelemetryIssueDataKey] = Constants.Runner.LowDiskSpace; - context.AddIssue(issue, writeToLog: true); + var message = $"You are running out of disk space. The runner will stop working when the machine runs out of disk space. Free space left: {freeSpaceInMB} MB"; + var metadata = new IssueMetadata(Constants.Runner.InternalTelemetryIssueDataKey, Constants.Runner.LowDiskSpace); + var issue = context.CreateIssue(IssueType.Warning, message, metadata, true); + context.AddIssue(issue); return; } diff --git a/src/Runner.Worker/JobRunner.cs b/src/Runner.Worker/JobRunner.cs index fca6c72e9..b5a1bcc25 100644 --- a/src/Runner.Worker/JobRunner.cs +++ b/src/Runner.Worker/JobRunner.cs @@ -84,8 +84,8 @@ namespace GitHub.Runner.Worker default: throw new ArgumentException(HostContext.RunnerShutdownReason.ToString(), nameof(HostContext.RunnerShutdownReason)); } - var issue = new Issue() { Type = IssueType.Error, Message = errorMessage }; - jobContext.AddIssue(issue, writeToLog: true); + var issue = jobContext.CreateIssue(IssueType.Error, errorMessage, null, true); + jobContext.AddIssue(issue); }); // Validate directory permissions. diff --git a/src/Sdk/Common/Common/Utility/PrimitiveExtensions.cs b/src/Sdk/Common/Common/Utility/PrimitiveExtensions.cs index aae382489..4b00293c7 100644 --- a/src/Sdk/Common/Common/Utility/PrimitiveExtensions.cs +++ b/src/Sdk/Common/Common/Utility/PrimitiveExtensions.cs @@ -31,9 +31,9 @@ namespace GitHub.Services.Common { string result = text; - if (!string.IsNullOrEmpty(text) && text.Length > maxLength) + if (!string.IsNullOrEmpty(result) && result.Length > maxLength) { - result = text.Substring(0, maxLength); + result = result.Substring(0, maxLength); } return result; diff --git a/src/Sdk/DTWebApi/WebApi/Issue.cs b/src/Sdk/DTWebApi/WebApi/Issue.cs index af4d2f850..d68815402 100644 --- a/src/Sdk/DTWebApi/WebApi/Issue.cs +++ b/src/Sdk/DTWebApi/WebApi/Issue.cs @@ -4,24 +4,55 @@ using System.Runtime.Serialization; namespace GitHub.DistributedTask.WebApi { + + public interface IReadOnlyIssue + { + IssueType Type { get; } + string Category { get; } + string Message { get; } + bool? IsInfrastructureIssue { get; } + string this[string key] { get; } + } + + public class IssueMetadata + { + + public IssueMetadata(string key, string value) + : this(null, false, new []{ KeyValuePair.Create(key, value) }) + { + } + + public IssueMetadata(string category, bool infrastructureIssue, IEnumerable> data) + { + this.Category = category; + this.IsInfrastructureIssue = infrastructureIssue; + this.Data = data; + } + + + public readonly string Category; + public readonly bool IsInfrastructureIssue; + public readonly IEnumerable> Data; + } + [DataContract] - public class Issue + public class Issue : IReadOnlyIssue { public Issue() { } - private Issue(Issue issueToBeCloned) + private Issue(Issue original) { - this.Type = issueToBeCloned.Type; - this.Category = issueToBeCloned.Category; - this.Message = issueToBeCloned.Message; - this.IsInfrastructureIssue = issueToBeCloned.IsInfrastructureIssue; + this.Type = original.Type; + this.Category = original.Category; + this.Message = original.Message; + this.IsInfrastructureIssue = original.IsInfrastructureIssue; - if (issueToBeCloned.m_data != null) + if (original.m_data != null) { - foreach (var item in issueToBeCloned.m_data) + foreach (var item in original.m_data) { this.Data.Add(item); } @@ -68,6 +99,16 @@ namespace GitHub.DistributedTask.WebApi } } + public string this[string key] + { + get + { + m_data.TryGetValue(key, out string result); + return result; + } + } + + public Issue Clone() { return new Issue(this); diff --git a/src/Test/L0/Worker/ActionCommandManagerL0.cs b/src/Test/L0/Worker/ActionCommandManagerL0.cs index cef826fd9..a94413cb8 100644 --- a/src/Test/L0/Worker/ActionCommandManagerL0.cs +++ b/src/Test/L0/Worker/ActionCommandManagerL0.cs @@ -6,6 +6,7 @@ using GitHub.DistributedTask.Pipelines.ContextData; using GitHub.DistributedTask.WebApi; using GitHub.Runner.Worker; using GitHub.Runner.Worker.Container; +using GitHub.Services.Common; using Moq; using Xunit; using Pipelines = GitHub.DistributedTask.Pipelines; @@ -32,10 +33,10 @@ namespace GitHub.Runner.Common.Tests.Worker hc.GetTrace().Info($"{tag} {line}"); return 1; }); - _ec.Setup(x => x.AddIssue(It.IsAny(), It.IsAny())) - .Callback((Issue issue, string message) => + _ec.Setup(x => x.AddIssue(It.IsAny())) + .Callback((IReadOnlyIssue issue) => { - hc.GetTrace().Info($"{issue.Type} {issue.Message} {message ?? string.Empty}"); + hc.GetTrace().Info($"{issue.Type} {issue.Message}"); }); _commandManager.EnablePluginInternalCommand(); @@ -59,10 +60,10 @@ namespace GitHub.Runner.Common.Tests.Worker hc.GetTrace().Info($"{tag} {line}"); return 1; }); - _ec.Setup(x => x.AddIssue(It.IsAny(), It.IsAny())) - .Callback((Issue issue, string message) => + _ec.Setup(x => x.AddIssue(It.IsAny())) + .Callback((IReadOnlyIssue issue) => { - hc.GetTrace().Info($"{issue.Type} {issue.Message} {message ?? string.Empty}"); + hc.GetTrace().Info($"{issue.Type} {issue.Message}"); }); _commandManager.EnablePluginInternalCommand(); @@ -92,10 +93,24 @@ namespace GitHub.Runner.Common.Tests.Worker return 1; }); - _ec.Setup(x => x.AddIssue(It.IsAny(), It.IsAny())) - .Callback((Issue issue, string message) => + _ec.Setup(x => x.CreateIssue(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((IssueType type, string message, IssueMetadata metadata, bool writeToLog) => + { + var result = new Issue() + { + Type = type, + Message = message, + Category = metadata?.Category, + IsInfrastructureIssue = metadata?.IsInfrastructureIssue ?? false + }; + result.Data.AddRangeIfRangeNotNull(metadata?.Data); + return result; + }); + + _ec.Setup(x => x.AddIssue(It.IsAny())) + .Callback((IReadOnlyIssue issue) => { - hc.GetTrace().Info($"{issue.Type} {issue.Message} {message ?? string.Empty}"); + hc.GetTrace().Info($"{issue.Type} {issue.Message}"); }); _ec.Object.Global.EnvironmentVariables = new Dictionary(); diff --git a/src/Test/L0/Worker/ActionManagerL0.cs b/src/Test/L0/Worker/ActionManagerL0.cs index 07cf9b487..1d14bf5ed 100644 --- a/src/Test/L0/Worker/ActionManagerL0.cs +++ b/src/Test/L0/Worker/ActionManagerL0.cs @@ -1,7 +1,6 @@ using System; using System.Collections.Generic; using System.IO; -using System.IO.Compression; using System.Net; using System.Net.Http; using System.Runtime.CompilerServices; @@ -14,6 +13,7 @@ using GitHub.DistributedTask.WebApi; using GitHub.Runner.Sdk; using GitHub.Runner.Worker; using GitHub.Runner.Worker.Container; +using GitHub.Services.Common; using Moq; using Moq.Protected; using Xunit; @@ -2147,7 +2147,20 @@ runs: _ec.Object.Global.FileTable = new List(); _ec.Object.Global.Plan = new TaskOrchestrationPlanReference(); _ec.Setup(x => x.Write(It.IsAny(), It.IsAny())).Callback((string tag, string message) => { _hc.GetTrace().Info($"[{tag}]{message}"); }); - _ec.Setup(x => x.AddIssue(It.IsAny(), It.IsAny())).Callback((Issue issue, string message) => { _hc.GetTrace().Info($"[{issue.Type}]{issue.Message ?? message}"); }); + _ec.Setup(x => x.CreateIssue(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((IssueType type, string message, IssueMetadata metadata, bool writeToLog) => + { + var result = new Issue() + { + Type = type, + Message = message, + Category = metadata?.Category, + IsInfrastructureIssue = metadata?.IsInfrastructureIssue ?? false + }; + result.Data.AddRangeIfRangeNotNull(metadata?.Data); + return result; + }); + _ec.Setup(x => x.AddIssue(It.IsAny())).Callback((IReadOnlyIssue issue) => { _hc.GetTrace().Info($"[{issue.Type}]{issue.Message}"); }); _ec.Setup(x => x.GetGitHubContext("workspace")).Returns(Path.Combine(_workFolder, "actions", "actions")); _dockerManager = new Mock(); diff --git a/src/Test/L0/Worker/ActionManifestManagerL0.cs b/src/Test/L0/Worker/ActionManifestManagerL0.cs index b6e3f855b..aaec5f772 100644 --- a/src/Test/L0/Worker/ActionManifestManagerL0.cs +++ b/src/Test/L0/Worker/ActionManifestManagerL0.cs @@ -4,10 +4,10 @@ using GitHub.DistributedTask.Pipelines.ContextData; using GitHub.DistributedTask.WebApi; using GitHub.Runner.Worker; using GitHub.Runner.Worker.Expressions; +using GitHub.Services.Common; using Moq; using System; using System.Collections.Generic; -using System.Diagnostics; using System.IO; using System.Runtime.CompilerServices; using System.Threading; @@ -670,7 +670,7 @@ namespace GitHub.Runner.Common.Tests.Worker { Teardown(); } - } + } [Fact] [Trait("Level", "L0")] @@ -715,7 +715,7 @@ namespace GitHub.Runner.Common.Tests.Worker //Assert var err = Assert.Throws(() => actionManifest.Load(_ec.Object, action_path)); Assert.Contains($"Fail to load {action_path}", err.Message); - _ec.Verify(x => x.AddIssue(It.Is(s => s.Message.Contains("Missing 'using' value. 'using' requires 'composite', 'docker', 'node12' or 'node16'.")), It.IsAny()), Times.Once); + _ec.Verify(x => x.AddIssue(It.Is(s => s.Message.Contains("Missing 'using' value. 'using' requires 'composite', 'docker', 'node12' or 'node16'."))), Times.Once); } finally { @@ -860,7 +860,22 @@ namespace GitHub.Runner.Common.Tests.Worker _ec.Setup(x => x.ExpressionValues).Returns(new DictionaryContextData()); _ec.Setup(x => x.ExpressionFunctions).Returns(new List()); _ec.Setup(x => x.Write(It.IsAny(), It.IsAny())).Callback((string tag, string message) => { _hc.GetTrace().Info($"{tag}{message}"); }); - _ec.Setup(x => x.AddIssue(It.IsAny(), It.IsAny())).Callback((Issue issue, string message) => { _hc.GetTrace().Info($"[{issue.Type}]{issue.Message ?? message}"); }); + + _ec.Setup(x => x.CreateIssue(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((IssueType type, string message, IssueMetadata metadata, bool writeToLog) => + { + var result = new Issue() + { + Type = type, + Message = message, + Category = metadata?.Category, + IsInfrastructureIssue = metadata?.IsInfrastructureIssue ?? false + }; + result.Data.AddRangeIfRangeNotNull(metadata?.Data); + return result; + }); + + _ec.Setup(x => x.AddIssue(It.IsAny())).Callback((IReadOnlyIssue issue) => { _hc.GetTrace().Info($"[{issue.Type}]{issue.Message}"); }); } private void Teardown() diff --git a/src/Test/L0/Worker/ActionRunnerL0.cs b/src/Test/L0/Worker/ActionRunnerL0.cs index 31d074365..f56f85536 100644 --- a/src/Test/L0/Worker/ActionRunnerL0.cs +++ b/src/Test/L0/Worker/ActionRunnerL0.cs @@ -3,20 +3,15 @@ using GitHub.DistributedTask.ObjectTemplating.Tokens; using GitHub.DistributedTask.Pipelines; using GitHub.DistributedTask.Pipelines.ContextData; using GitHub.DistributedTask.WebApi; -using GitHub.Runner.Common.Util; using GitHub.Runner.Worker; -using GitHub.Runner.Worker.Container; using GitHub.Runner.Worker.Handlers; +using GitHub.Services.Common; using Moq; using Newtonsoft.Json.Linq; using System; using System.Collections.Generic; -using System.IO; -using System.IO.Compression; -using System.Reflection; using System.Runtime.CompilerServices; using System.Threading; -using System.Threading.Tasks; using Xunit; using Pipelines = GitHub.DistributedTask.Pipelines; @@ -330,7 +325,7 @@ namespace GitHub.Runner.Common.Tests.Worker Assert.Equal("invalid1", finialInputs["invalid1"]); Assert.Equal("invalid2", finialInputs["invalid2"]); - _ec.Verify(x => x.AddIssue(It.Is(s => s.Message.Contains("Unexpected input(s) 'invalid1', 'invalid2'")), It.IsAny()), Times.Once); + _ec.Verify(x => x.AddIssue(It.Is(s => s.Message.Contains("Unexpected input(s) 'invalid1', 'invalid2'"))), Times.Once); } [Fact] @@ -449,7 +444,21 @@ namespace GitHub.Runner.Common.Tests.Worker _ec.Setup(x => x.CancellationToken).Returns(_ecTokenSource.Token); _ec.Object.Global.Variables = new Variables(_hc, new Dictionary()); _ec.Setup(x => x.Write(It.IsAny(), It.IsAny())).Callback((string tag, string message) => { _hc.GetTrace().Info($"[{tag}]{message}"); }); - _ec.Setup(x => x.AddIssue(It.IsAny(), It.IsAny())).Callback((Issue issue, string message) => { _hc.GetTrace().Info($"[{issue.Type}]{issue.Message ?? message}"); }); + _ec.Setup(x => x.CreateIssue(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((IssueType type, string message, IssueMetadata metadata, bool writeToLog) => + { + var result = new Issue() + { + Type = type, + Message = message, + Category = metadata?.Category, + IsInfrastructureIssue = metadata?.IsInfrastructureIssue ?? false + }; + result.Data.AddRangeIfRangeNotNull(metadata?.Data); + return result; + }); + + _ec.Setup(x => x.AddIssue(It.IsAny())).Callback((IReadOnlyIssue issue) => { _hc.GetTrace().Info($"[{issue.Type}]{issue.Message}"); }); _hc.SetSingleton(_actionManager.Object); _hc.SetSingleton(_handlerFactory.Object); diff --git a/src/Test/L0/Worker/CreateStepSummaryCommandL0.cs b/src/Test/L0/Worker/CreateStepSummaryCommandL0.cs index 67631ed4b..69b2cbdda 100644 --- a/src/Test/L0/Worker/CreateStepSummaryCommandL0.cs +++ b/src/Test/L0/Worker/CreateStepSummaryCommandL0.cs @@ -11,6 +11,7 @@ using Xunit; using DTWebApi = GitHub.DistributedTask.WebApi; using GitHub.DistributedTask.WebApi; using Pipelines = GitHub.DistributedTask.Pipelines; +using GitHub.Services.Common; namespace GitHub.Runner.Common.Tests.Worker { @@ -19,7 +20,7 @@ namespace GitHub.Runner.Common.Tests.Worker private Mock _executionContext; private Mock _jobServerQueue; private ExecutionContext _jobExecutionContext; - private List> _issues; + private List _issues; private Variables _variables; private string _rootDirectory; private CreateStepSummaryCommand _createStepCommand; @@ -186,7 +187,7 @@ namespace GitHub.Runner.Common.Tests.Worker { var hostContext = new TestHostContext(this, name); - _issues = new List>(); + _issues = new List(); // Setup a job request TaskOrchestrationPlanReference plan = new(); @@ -247,13 +248,26 @@ namespace GitHub.Runner.Common.Tests.Worker WriteDebug = true, Variables = _variables, }); - _executionContext.Setup(x => x.AddIssue(It.IsAny(), It.IsAny())) - .Callback((DTWebApi.Issue issue, string logMessage) => + _executionContext.Setup(x => x.AddIssue(It.IsAny())) + .Callback((DTWebApi.IReadOnlyIssue issue) => { - _issues.Add(new Tuple(issue, logMessage)); - var message = !string.IsNullOrEmpty(logMessage) ? logMessage : issue.Message; - _trace.Info($"Issue '{issue.Type}': {message}"); + _issues.Add(issue); + _trace.Info($"Issue '{issue.Type}': {issue.Message}"); }); + _executionContext.Setup(x => x.CreateIssue(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((IssueType type, string message, IssueMetadata metadata, bool writeToLog) => + { + var result = new Issue() + { + Type = type, + Message = message, + Category = metadata?.Category, + IsInfrastructureIssue = metadata?.IsInfrastructureIssue ?? false + }; + result.Data.AddRangeIfRangeNotNull(metadata?.Data); + return result; + }); + _executionContext.Setup(x => x.Write(It.IsAny(), It.IsAny())) .Callback((string tag, string message) => { diff --git a/src/Test/L0/Worker/ExecutionContextL0.cs b/src/Test/L0/Worker/ExecutionContextL0.cs index 3aef7fb58..91adbfb18 100644 --- a/src/Test/L0/Worker/ExecutionContextL0.cs +++ b/src/Test/L0/Worker/ExecutionContextL0.cs @@ -52,36 +52,36 @@ namespace GitHub.Runner.Common.Tests.Worker // Act. ec.InitializeJob(jobRequest, CancellationToken.None); - ec.AddIssue(new Issue() { Type = IssueType.Error, Message = "error" }); - ec.AddIssue(new Issue() { Type = IssueType.Error, Message = "error" }); - ec.AddIssue(new Issue() { Type = IssueType.Error, Message = "error" }); - ec.AddIssue(new Issue() { Type = IssueType.Error, Message = "error" }); - ec.AddIssue(new Issue() { Type = IssueType.Error, Message = "error" }); - ec.AddIssue(new Issue() { Type = IssueType.Error, Message = "error" }); - ec.AddIssue(new Issue() { Type = IssueType.Error, Message = "error" }); - ec.AddIssue(new Issue() { Type = IssueType.Error, Message = "error" }); - ec.AddIssue(new Issue() { Type = IssueType.Error, Message = "error" }); - ec.AddIssue(new Issue() { Type = IssueType.Error, Message = "error" }); - ec.AddIssue(new Issue() { Type = IssueType.Error, Message = "error" }); - ec.AddIssue(new Issue() { Type = IssueType.Error, Message = "error" }); - ec.AddIssue(new Issue() { Type = IssueType.Error, Message = "error" }); - ec.AddIssue(new Issue() { Type = IssueType.Error, Message = "error" }); - ec.AddIssue(new Issue() { Type = IssueType.Error, Message = "error" }); + ec.AddIssue(ec.CreateIssue(IssueType.Error, "error", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Error, "error", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Error, "error", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Error, "error", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Error, "error", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Error, "error", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Error, "error", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Error, "error", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Error, "error", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Error, "error", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Error, "error", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Error, "error", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Error, "error", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Error, "error", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Error, "error", null, true)); - ec.AddIssue(new Issue() { Type = IssueType.Warning, Message = "warning" }); - ec.AddIssue(new Issue() { Type = IssueType.Warning, Message = "warning" }); - ec.AddIssue(new Issue() { Type = IssueType.Warning, Message = "warning" }); - ec.AddIssue(new Issue() { Type = IssueType.Warning, Message = "warning" }); - ec.AddIssue(new Issue() { Type = IssueType.Warning, Message = "warning" }); - ec.AddIssue(new Issue() { Type = IssueType.Warning, Message = "warning" }); - ec.AddIssue(new Issue() { Type = IssueType.Warning, Message = "warning" }); - ec.AddIssue(new Issue() { Type = IssueType.Warning, Message = "warning" }); - ec.AddIssue(new Issue() { Type = IssueType.Warning, Message = "warning" }); - ec.AddIssue(new Issue() { Type = IssueType.Warning, Message = "warning" }); - ec.AddIssue(new Issue() { Type = IssueType.Warning, Message = "warning" }); - ec.AddIssue(new Issue() { Type = IssueType.Warning, Message = "warning" }); - ec.AddIssue(new Issue() { Type = IssueType.Warning, Message = "warning" }); - ec.AddIssue(new Issue() { Type = IssueType.Warning, Message = "warning" }); + ec.AddIssue(ec.CreateIssue(IssueType.Warning, "warning", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Warning, "warning", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Warning, "warning", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Warning, "warning", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Warning, "warning", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Warning, "warning", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Warning, "warning", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Warning, "warning", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Warning, "warning", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Warning, "warning", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Warning, "warning", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Warning, "warning", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Warning, "warning", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Warning, "warning", null, true)); ec.Complete(); @@ -190,9 +190,9 @@ namespace GitHub.Runner.Common.Tests.Worker bigMessage += "a"; } - ec.AddIssue(new Issue() { Type = IssueType.Error, Message = bigMessage }); - ec.AddIssue(new Issue() { Type = IssueType.Warning, Message = bigMessage }); - ec.AddIssue(new Issue() { Type = IssueType.Notice, Message = bigMessage }); + ec.AddIssue(ec.CreateIssue(IssueType.Error, bigMessage, null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Warning, bigMessage, null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Notice, bigMessage, null, true)); ec.Complete(); @@ -242,9 +242,9 @@ namespace GitHub.Runner.Common.Tests.Worker var embeddedStep = ec.CreateChild(Guid.NewGuid(), "action_1_pre", "action_1_pre", null, null, ActionRunStage.Main, isEmbedded: true); embeddedStep.Start(); - embeddedStep.AddIssue(new Issue() { Type = IssueType.Error, Message = "error annotation that should have step and line number information" }); - embeddedStep.AddIssue(new Issue() { Type = IssueType.Warning, Message = "warning annotation that should have step and line number information" }); - embeddedStep.AddIssue(new Issue() { Type = IssueType.Notice, Message = "notice annotation that should have step and line number information" }); + embeddedStep.AddIssue(embeddedStep.CreateIssue(IssueType.Error, "error annotation that should have step and line number information", null, true)); + embeddedStep.AddIssue(embeddedStep.CreateIssue(IssueType.Warning, "warning annotation that should have step and line number information", null, true)); + embeddedStep.AddIssue(embeddedStep.CreateIssue(IssueType.Notice, "notice annotation that should have step and line number information", null, true)); jobServerQueue.Verify(x => x.QueueTimelineRecordUpdate(It.IsAny(), It.Is(t => t.Issues.Where(i => i.Data.ContainsKey("stepNumber") && i.Data.ContainsKey("logFileLineNumber") && i.Type == IssueType.Error).Count() == 1)), Times.AtLeastOnce); jobServerQueue.Verify(x => x.QueueTimelineRecordUpdate(It.IsAny(), It.Is(t => t.Issues.Where(i => i.Data.ContainsKey("stepNumber") && i.Data.ContainsKey("logFileLineNumber") && i.Type == IssueType.Warning).Count() == 1)), Times.AtLeastOnce); @@ -626,12 +626,12 @@ namespace GitHub.Runner.Common.Tests.Worker ec.StepTelemetry.StepId = Guid.NewGuid(); ec.StepTelemetry.Stage = "main"; - ec.AddIssue(new Issue() { Type = IssueType.Error, Message = "error" }); - ec.AddIssue(new Issue() { Type = IssueType.Warning, Message = "warning" }); - ec.AddIssue(new Issue() { Type = IssueType.Notice, Message = "notice" }); - ec.AddIssue(new Issue() { Type = IssueType.Error, Message = "error" }); - ec.AddIssue(new Issue() { Type = IssueType.Warning, Message = "warning" }); - ec.AddIssue(new Issue() { Type = IssueType.Notice, Message = "notice" }); + ec.AddIssue(ec.CreateIssue(IssueType.Error, "error", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Warning, "warning", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Notice, "notice", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Error, "error", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Warning, "warning", null, true)); + ec.AddIssue(ec.CreateIssue(IssueType.Notice, "notice", null, true)); ec.Complete(); @@ -692,9 +692,9 @@ namespace GitHub.Runner.Common.Tests.Worker embeddedStep.StepTelemetry.Action = "actions/checkout"; embeddedStep.StepTelemetry.Ref = "v2"; - embeddedStep.AddIssue(new Issue() { Type = IssueType.Error, Message = "error" }); - embeddedStep.AddIssue(new Issue() { Type = IssueType.Warning, Message = "warning" }); - embeddedStep.AddIssue(new Issue() { Type = IssueType.Notice, Message = "notice" }); + embeddedStep.AddIssue(ec.CreateIssue(IssueType.Error, "error", null, true)); + embeddedStep.AddIssue(ec.CreateIssue(IssueType.Warning, "warning", null, true)); + embeddedStep.AddIssue(ec.CreateIssue(IssueType.Notice, "notice", null, true)); embeddedStep.PublishStepTelemetry(); @@ -870,7 +870,7 @@ namespace GitHub.Runner.Common.Tests.Worker inputVarsContext["VARIABLE_2"] = new StringContextData("value2"); jobRequest.ContextData["vars"] = inputVarsContext; - // Arrange: Setup the paging logger. + // Arrange: Setup the paging logger. var pagingLogger1 = new Mock(); var jobServerQueue = new Mock(); hc.EnqueueInstance(pagingLogger1.Object); @@ -884,7 +884,7 @@ namespace GitHub.Runner.Common.Tests.Worker var expected = new DictionaryContextData(); expected["VARIABLE_1"] = new StringContextData("value1"); expected["VARIABLE_2"] = new StringContextData("value1"); - + Assert.True(ExpressionValuesAssertEqual(expected, jobContext.ExpressionValues["vars"] as DictionaryContextData)); } } @@ -915,7 +915,7 @@ namespace GitHub.Runner.Common.Tests.Worker inputVarsContext[Constants.Variables.Actions.RunnerDebug] = new StringContextData("true"); jobRequest.ContextData["vars"] = inputVarsContext; - // Arrange: Setup the paging logger. + // Arrange: Setup the paging logger. var pagingLogger1 = new Mock(); var jobServerQueue = new Mock(); hc.EnqueueInstance(pagingLogger1.Object); @@ -926,7 +926,7 @@ namespace GitHub.Runner.Common.Tests.Worker jobContext.InitializeJob(jobRequest, CancellationToken.None); - + Assert.Equal("true", jobContext.Global.Variables.Get(Constants.Variables.Actions.StepDebug)); Assert.Equal("true", jobContext.Global.Variables.Get(Constants.Variables.Actions.RunnerDebug)); } @@ -961,7 +961,7 @@ namespace GitHub.Runner.Common.Tests.Worker jobRequest.Variables[Constants.Variables.Actions.StepDebug] = "false"; jobRequest.Variables[Constants.Variables.Actions.RunnerDebug] = "false"; - // Arrange: Setup the paging logger. + // Arrange: Setup the paging logger. var pagingLogger1 = new Mock(); var jobServerQueue = new Mock(); hc.EnqueueInstance(pagingLogger1.Object); @@ -972,7 +972,7 @@ namespace GitHub.Runner.Common.Tests.Worker jobContext.InitializeJob(jobRequest, CancellationToken.None); - + Assert.Equal("false", jobContext.Global.Variables.Get(Constants.Variables.Actions.StepDebug)); Assert.Equal("false", jobContext.Global.Variables.Get(Constants.Variables.Actions.RunnerDebug)); } diff --git a/src/Test/L0/Worker/OutputManagerL0.cs b/src/Test/L0/Worker/OutputManagerL0.cs index e0364e9f0..e511bd73f 100644 --- a/src/Test/L0/Worker/OutputManagerL0.cs +++ b/src/Test/L0/Worker/OutputManagerL0.cs @@ -9,6 +9,7 @@ using GitHub.Runner.Sdk; using GitHub.Runner.Worker; using GitHub.Runner.Worker.Container; using GitHub.Runner.Worker.Handlers; +using GitHub.Services.Common; using Moq; using Xunit; using DTWebApi = GitHub.DistributedTask.WebApi; @@ -21,7 +22,7 @@ namespace GitHub.Runner.Common.Tests.Worker private Mock _commandManager; private Variables _variables; private OnMatcherChanged _onMatcherChanged; - private List> _issues; + private List _issues; private List _messages; private List _commands; private OutputManager _outputManager; @@ -82,10 +83,10 @@ namespace GitHub.Runner.Common.Tests.Worker Process("ERROR: message 4"); Process("NOT GOOD: message 5"); Assert.Equal(4, _issues.Count); - Assert.Equal("message 1", _issues[0].Item1.Message); - Assert.Equal("message 2", _issues[1].Item1.Message); - Assert.Equal("message 3", _issues[2].Item1.Message); - Assert.Equal("message 5", _issues[3].Item1.Message); + Assert.Equal("message 1", _issues[0].Message); + Assert.Equal("message 2", _issues[1].Message); + Assert.Equal("message 3", _issues[2].Message); + Assert.Equal("message 5", _issues[3].Message); Assert.Equal(0, _commands.Count); Assert.Equal(1, _messages.Count); Assert.Equal("ERROR: message 4", _messages[0]); @@ -148,11 +149,11 @@ namespace GitHub.Runner.Common.Tests.Worker Process("ERROR: message 4"); Process("NOT GOOD: message 5"); Assert.Equal(5, _issues.Count); - Assert.Equal("message 1", _issues[0].Item1.Message); - Assert.Equal("message 2", _issues[1].Item1.Message); - Assert.Equal("message 3", _issues[2].Item1.Message); - Assert.Equal("message 4", _issues[3].Item1.Message); - Assert.Equal("message 5", _issues[4].Item1.Message); + Assert.Equal("message 1", _issues[0].Message); + Assert.Equal("message 2", _issues[1].Message); + Assert.Equal("message 3", _issues[2].Message); + Assert.Equal("message 4", _issues[3].Message); + Assert.Equal("message 5", _issues[4].Message); Assert.Equal(0, _commands.Count); Assert.Equal(0, _messages.Count); } @@ -188,10 +189,10 @@ namespace GitHub.Runner.Common.Tests.Worker Process("BAD: real bad"); Process(": not working"); Assert.Equal(2, _issues.Count); - Assert.Equal("real bad", _issues[0].Item1.Message); - Assert.Equal("BAD", _issues[0].Item1.Data["code"]); - Assert.Equal("not working", _issues[1].Item1.Message); - Assert.False(_issues[1].Item1.Data.ContainsKey("code")); + Assert.Equal("real bad", _issues[0].Message); + Assert.Equal("BAD", _issues[0]["code"]); + Assert.Equal("not working", _issues[1].Message); + Assert.Null(_issues[1]["code"]); Assert.Equal(0, _commands.Count); Assert.Equal(0, _messages.Count); } @@ -238,11 +239,11 @@ namespace GitHub.Runner.Common.Tests.Worker Process("Error: real bad"); Process("regular message 2"); Assert.Equal(5, _issues.Count); - Assert.Equal("it broke", _issues[0].Item1.Message); - Assert.Equal("oh no", _issues[1].Item1.Message); - Assert.Equal("not good", _issues[2].Item1.Message); - Assert.Equal("it broke again", _issues[3].Item1.Message); - Assert.Equal("real bad", _issues[4].Item1.Message); + Assert.Equal("it broke", _issues[0].Message); + Assert.Equal("oh no", _issues[1].Message); + Assert.Equal("not good", _issues[2].Message); + Assert.Equal("it broke again", _issues[3].Message); + Assert.Equal("real bad", _issues[4].Message); Assert.Equal(0, _commands.Count); Assert.Equal(4, _messages.Count); Assert.Equal("Start: hello", _messages[0]); @@ -293,8 +294,8 @@ namespace GitHub.Runner.Common.Tests.Worker Process("ERROR: it is broken"); Process("NOT GOOD: that did not work"); Assert.Equal(2, _issues.Count); - Assert.Equal("it is broken", _issues[0].Item1.Message); - Assert.Equal("that did not work", _issues[1].Item1.Message); + Assert.Equal("it is broken", _issues[0].Message); + Assert.Equal("that did not work", _issues[1].Message); Assert.Equal(0, _commands.Count); Assert.Equal(0, _messages.Count); } @@ -332,15 +333,15 @@ namespace GitHub.Runner.Common.Tests.Worker Process("(12,thirty-four): it is broken"); Process("(twelve,34): not working"); Assert.Equal(3, _issues.Count); - Assert.Equal("real bad", _issues[0].Item1.Message); - Assert.Equal("12", _issues[0].Item1.Data["line"]); - Assert.Equal("34", _issues[0].Item1.Data["col"]); - Assert.Equal("it is broken", _issues[1].Item1.Message); - Assert.Equal("12", _issues[1].Item1.Data["line"]); - Assert.False(_issues[1].Item1.Data.ContainsKey("col")); - Assert.Equal("not working", _issues[2].Item1.Message); - Assert.False(_issues[2].Item1.Data.ContainsKey("line")); - Assert.Equal("34", _issues[2].Item1.Data["col"]); + Assert.Equal("real bad", _issues[0].Message); + Assert.Equal("12", _issues[0]["line"]); + Assert.Equal("34", _issues[0]["col"]); + Assert.Equal("it is broken", _issues[1].Message); + Assert.Equal("12", _issues[1]["line"]); + Assert.Null(_issues[1]["col"]); + Assert.Equal("not working", _issues[2].Message); + Assert.Null(_issues[2]["line"]); + Assert.Equal("34", _issues[2]["col"]); Assert.Equal(0, _commands.Count); Assert.Equal(2, _messages.Count); Assert.Equal("##[debug]Unable to parse column number 'thirty-four'", _messages[0]); @@ -373,8 +374,8 @@ namespace GitHub.Runner.Common.Tests.Worker Process("this line is a command too ##[some-command]even though it contains ERROR: not working again"); Process("##[not-command]this line is an ERROR: it is broken again"); Assert.Equal(2, _issues.Count); - Assert.Equal("it is broken", _issues[0].Item1.Message); - Assert.Equal("it is broken again", _issues[1].Item1.Message); + Assert.Equal("it is broken", _issues[0].Message); + Assert.Equal("it is broken again", _issues[1].Message); Assert.Equal(2, _commands.Count); Assert.Equal("##[some-command]this line is a command even though it contains ERROR: not working", _commands[0]); Assert.Equal("this line is a command too ##[some-command]even though it contains ERROR: not working again", _commands[1]); @@ -404,8 +405,7 @@ namespace GitHub.Runner.Common.Tests.Worker }); Process("the error: \033[31mred, \033[1;31mbright red, \033[mreset"); Assert.Equal(1, _issues.Count); - Assert.Equal("red, bright red, reset", _issues[0].Item1.Message); - Assert.Equal("the error: red, bright red, reset", _issues[0].Item2); + Assert.Equal("red, bright red, reset", _issues[0].Message); Assert.Equal(0, _commands.Count); Assert.Equal(0, _messages.Count); } @@ -455,9 +455,9 @@ namespace GitHub.Runner.Common.Tests.Worker Process("ERROR: message 3"); Process("NOT GOOD: message 4"); Assert.Equal(3, _issues.Count); - Assert.Equal("message 1", _issues[0].Item1.Message); - Assert.Equal("message 2", _issues[1].Item1.Message); - Assert.Equal("message 4", _issues[2].Item1.Message); + Assert.Equal("message 1", _issues[0].Message); + Assert.Equal("message 2", _issues[1].Message); + Assert.Equal("message 4", _issues[2].Message); Assert.Equal(0, _commands.Count); Assert.Equal(1, _messages.Count); Assert.Equal("ERROR: message 3", _messages[0]); @@ -517,8 +517,8 @@ namespace GitHub.Runner.Common.Tests.Worker Process("Matches both line 1: hello again"); Process("oh no, another error"); Assert.Equal(2, _issues.Count); - Assert.Equal("it broke", _issues[0].Item1.Message); - Assert.Equal("oh no, another error", _issues[1].Item1.Message); + Assert.Equal("it broke", _issues[0].Message); + Assert.Equal("oh no, another error", _issues[1].Message); Assert.Equal(0, _commands.Count); Assert.Equal(4, _messages.Count); Assert.Equal("Matches both line 1: hello", _messages[0]); @@ -573,14 +573,14 @@ namespace GitHub.Runner.Common.Tests.Worker Process(": not working"); Process("ERROR! uh oh"); Assert.Equal(4, _issues.Count); - Assert.Equal("real bad", _issues[0].Item1.Message); - Assert.Equal(DTWebApi.IssueType.Error, _issues[0].Item1.Type); - Assert.Equal("not great", _issues[1].Item1.Message); - Assert.Equal(DTWebApi.IssueType.Warning, _issues[1].Item1.Type); - Assert.Equal("not working", _issues[2].Item1.Message); - Assert.Equal(DTWebApi.IssueType.Error, _issues[2].Item1.Type); - Assert.Equal("uh oh", _issues[3].Item1.Message); - Assert.Equal(DTWebApi.IssueType.Error, _issues[3].Item1.Type); + Assert.Equal("real bad", _issues[0].Message); + Assert.Equal(DTWebApi.IssueType.Error, _issues[0].Type); + Assert.Equal("not great", _issues[1].Message); + Assert.Equal(DTWebApi.IssueType.Warning, _issues[1].Type); + Assert.Equal("not working", _issues[2].Message); + Assert.Equal(DTWebApi.IssueType.Error, _issues[2].Type); + Assert.Equal("uh oh", _issues[3].Message); + Assert.Equal(DTWebApi.IssueType.Error, _issues[3].Type); Assert.Equal(0, _commands.Count); Assert.Equal(2, _messages.Count); Assert.StartsWith("##[debug]Skipped", _messages[0]); @@ -633,9 +633,9 @@ namespace GitHub.Runner.Common.Tests.Worker Process("jane.doe@contoso.com"); Process("ERR: this error"); Assert.Equal(3, _issues.Count); - Assert.Equal("john.doe@contoso.com", _issues[0].Item1.Message); - Assert.Contains("Removing issue matcher 'email'", _issues[1].Item1.Message); - Assert.Equal("this error", _issues[2].Item1.Message); + Assert.Equal("john.doe@contoso.com", _issues[0].Message); + Assert.Contains("Removing issue matcher 'email'", _issues[1].Message); + Assert.Equal("this error", _issues[2].Message); Assert.Equal(0, _commands.Count); Assert.Equal(2, _messages.Where(x => x.StartsWith("##[debug]Timeout processing issue matcher")).Count()); Assert.Equal(1, _messages.Where(x => x.Equals("t@t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.t.c%20")).Count()); @@ -725,32 +725,32 @@ namespace GitHub.Runner.Common.Tests.Worker Assert.Equal(9, _issues.Count); - Assert.Equal("some error 1", _issues[0].Item1.Message); - Assert.False(_issues[0].Item1.Data.ContainsKey("file")); + Assert.Equal("some error 1", _issues[0].Message); + Assert.Null(_issues[0]["file"]); - Assert.Equal("some error 2", _issues[1].Item1.Message); - Assert.Equal(file_workflowRepository.Substring(workflowRepository.Length + 1).Replace(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar), _issues[1].Item1.Data["file"]); + Assert.Equal("some error 2", _issues[1].Message); + Assert.Equal(file_workflowRepository.Substring(workflowRepository.Length + 1).Replace(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar), _issues[1]["file"]); - Assert.Equal("some error 3", _issues[2].Item1.Message); - Assert.Equal(file_workflowRepository.Substring(workflowRepository.Length + 1).Replace(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar), _issues[2].Item1.Data["file"]); + Assert.Equal("some error 3", _issues[2].Message); + Assert.Equal(file_workflowRepository.Substring(workflowRepository.Length + 1).Replace(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar), _issues[2]["file"]); - Assert.Equal("some error 4", _issues[3].Item1.Message); - Assert.Equal(file_workflowRepository_nestedDirectory.Substring(workflowRepository.Length + 1).Replace(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar), _issues[3].Item1.Data["file"]); + Assert.Equal("some error 4", _issues[3].Message); + Assert.Equal(file_workflowRepository_nestedDirectory.Substring(workflowRepository.Length + 1).Replace(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar), _issues[3]["file"]); - Assert.Equal("some error 5", _issues[4].Item1.Message); - Assert.False(_issues[4].Item1.Data.ContainsKey("file")); + Assert.Equal("some error 5", _issues[4].Message); + Assert.Null(_issues[4]["file"]); - Assert.Equal("some error 6", _issues[5].Item1.Message); - Assert.False(_issues[5].Item1.Data.ContainsKey("file")); + Assert.Equal("some error 6", _issues[5].Message); + Assert.Null(_issues[5]["file"]); - Assert.Equal("some error 7", _issues[6].Item1.Message); - Assert.False(_issues[6].Item1.Data.ContainsKey("file")); + Assert.Equal("some error 7", _issues[6].Message); + Assert.Null(_issues[6]["file"]); - Assert.Equal("some error 8", _issues[7].Item1.Message); - Assert.Equal(file_nestedWorkflowRepository.Substring(nestedWorkflowRepository.Length + 1).Replace(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar), _issues[7].Item1.Data["file"]); + Assert.Equal("some error 8", _issues[7].Message); + Assert.Equal(file_nestedWorkflowRepository.Substring(nestedWorkflowRepository.Length + 1).Replace(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar), _issues[7]["file"]); - Assert.Equal("some error 9", _issues[8].Item1.Message); - Assert.Equal(file_workflowRepositoryUsingSsh.Substring(workflowRepositoryUsingSsh.Length + 1).Replace(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar), _issues[8].Item1.Data["file"]); + Assert.Equal("some error 9", _issues[8].Message); + Assert.Equal(file_workflowRepositoryUsingSsh.Substring(workflowRepositoryUsingSsh.Length + 1).Replace(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar), _issues[8]["file"]); } Environment.SetEnvironmentVariable("RUNNER_TEST_GET_REPOSITORY_PATH_FAILSAFE", ""); @@ -810,11 +810,11 @@ namespace GitHub.Runner.Common.Tests.Worker Assert.Equal(2, _issues.Count); - Assert.Equal("some error 1", _issues[0].Item1.Message); - Assert.Equal("some-file.txt", _issues[0].Item1.Data["file"]); + Assert.Equal("some error 1", _issues[0].Message); + Assert.Equal("some-file.txt", _issues[0]["file"]); - Assert.Equal("some error 2", _issues[1].Item1.Message); - Assert.Equal("some-file.txt", _issues[1].Item1.Data["file"]); + Assert.Equal("some error 2", _issues[1].Message); + Assert.Equal("some-file.txt", _issues[1]["file"]); } } @@ -871,11 +871,11 @@ namespace GitHub.Runner.Common.Tests.Worker Assert.Equal(2, _issues.Count); - Assert.Equal("some error 1", _issues[0].Item1.Message); - Assert.Equal("some-file.txt", _issues[0].Item1.Data["file"]); + Assert.Equal("some error 1", _issues[0].Message); + Assert.Equal("some-file.txt", _issues[0]["file"]); - Assert.Equal("some error 2", _issues[1].Item1.Message); - Assert.Equal("some-file.txt", _issues[1].Item1.Data["file"]); + Assert.Equal("some error 2", _issues[1].Message); + Assert.Equal("some-file.txt", _issues[1]["file"]); } } #endif @@ -929,8 +929,8 @@ namespace GitHub.Runner.Common.Tests.Worker // Process Process("some-directory/some-file.txt: some error [workflow-repo/some-project/some-project.proj]"); Assert.Equal(1, _issues.Count); - Assert.Equal("some error", _issues[0].Item1.Message); - Assert.Equal("some-project/some-directory/some-file.txt", _issues[0].Item1.Data["file"]); + Assert.Equal("some error", _issues[0].Message); + Assert.Equal("some-project/some-directory/some-file.txt", _issues[0]["file"]); Assert.Equal(0, _commands.Count); Assert.Equal(0, _messages.Count); } @@ -958,7 +958,7 @@ namespace GitHub.Runner.Common.Tests.Worker matchers?.Validate(); _onMatcherChanged = null; - _issues = new List>(); + _issues = new List(); _messages = new List(); _commands = new List(); @@ -983,10 +983,24 @@ namespace GitHub.Runner.Common.Tests.Worker { _onMatcherChanged = handler; }); - _executionContext.Setup(x => x.AddIssue(It.IsAny(), It.IsAny())) - .Callback((DTWebApi.Issue issue, string logMessage) => + _executionContext.Setup(x => x.CreateIssue(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((DTWebApi.IssueType type, string message, DTWebApi.IssueMetadata metadata, bool writeToLog) => { - _issues.Add(new Tuple(issue, logMessage)); + var result = new DTWebApi.Issue() + { + Type = type, + Message = message, + Category = metadata?.Category, + IsInfrastructureIssue = metadata?.IsInfrastructureIssue ?? false + }; + result.Data.AddRangeIfRangeNotNull(metadata?.Data); + return result; + }); + + _executionContext.Setup(x => x.AddIssue(It.IsAny())) + .Callback((DTWebApi.IReadOnlyIssue issue) => + { + _issues.Add(issue); }); _executionContext.Setup(x => x.Write(It.IsAny(), It.IsAny())) .Callback((string tag, string message) => diff --git a/src/Test/L0/Worker/SaveStateFileCommandL0.cs b/src/Test/L0/Worker/SaveStateFileCommandL0.cs index 45296c709..01503f2b2 100644 --- a/src/Test/L0/Worker/SaveStateFileCommandL0.cs +++ b/src/Test/L0/Worker/SaveStateFileCommandL0.cs @@ -21,7 +21,7 @@ namespace GitHub.Runner.Common.Tests.Worker public sealed class SaveStateFileCommandL0 { private Mock _executionContext; - private List> _issues; + private List _issues; private string _rootDirectory; private SaveStateFileCommand _saveStateFileCommand; private Dictionary _intraActionState; @@ -390,7 +390,7 @@ namespace GitHub.Runner.Common.Tests.Worker private TestHostContext Setup([CallerMemberName] string name = "") { - _issues = new List>(); + _issues = new List(); _intraActionState = new Dictionary(); var hostContext = new TestHostContext(this, name); @@ -413,12 +413,11 @@ namespace GitHub.Runner.Common.Tests.Worker EnvironmentVariables = new Dictionary(VarUtil.EnvironmentVariableKeyComparer), WriteDebug = true, }); - _executionContext.Setup(x => x.AddIssue(It.IsAny(), It.IsAny())) - .Callback((DTWebApi.Issue issue, string logMessage) => + _executionContext.Setup(x => x.AddIssue(It.IsAny())) + .Callback((DTWebApi.IReadOnlyIssue issue) => { - _issues.Add(new Tuple(issue, logMessage)); - var message = !string.IsNullOrEmpty(logMessage) ? logMessage : issue.Message; - _trace.Info($"Issue '{issue.Type}': {message}"); + _issues.Add(issue); + _trace.Info($"Issue '{issue.Type}': {issue.Message}"); }); _executionContext.Setup(x => x.Write(It.IsAny(), It.IsAny())) .Callback((string tag, string message) => diff --git a/src/Test/L0/Worker/SetEnvFileCommandL0.cs b/src/Test/L0/Worker/SetEnvFileCommandL0.cs index 41f8499d5..a49b89655 100644 --- a/src/Test/L0/Worker/SetEnvFileCommandL0.cs +++ b/src/Test/L0/Worker/SetEnvFileCommandL0.cs @@ -1,17 +1,11 @@ using System; using System.Collections.Generic; -using System.Globalization; using System.IO; -using System.Linq; using System.Text; -using System.Threading; -using System.Threading.Tasks; using System.Runtime.CompilerServices; using GitHub.Runner.Common.Util; using GitHub.Runner.Sdk; using GitHub.Runner.Worker; -using GitHub.Runner.Worker.Container; -using GitHub.Runner.Worker.Handlers; using Moq; using Xunit; using DTWebApi = GitHub.DistributedTask.WebApi; @@ -21,7 +15,7 @@ namespace GitHub.Runner.Common.Tests.Worker public sealed class SetEnvFileCommandL0 { private Mock _executionContext; - private List> _issues; + private List _issues; private string _rootDirectory; private SetEnvFileCommand _setEnvFileCommand; private ITraceWriter _trace; @@ -389,7 +383,7 @@ namespace GitHub.Runner.Common.Tests.Worker private TestHostContext Setup([CallerMemberName] string name = "") { - _issues = new List>(); + _issues = new List(); var hostContext = new TestHostContext(this, name); @@ -411,12 +405,11 @@ namespace GitHub.Runner.Common.Tests.Worker EnvironmentVariables = new Dictionary(VarUtil.EnvironmentVariableKeyComparer), WriteDebug = true, }); - _executionContext.Setup(x => x.AddIssue(It.IsAny(), It.IsAny())) - .Callback((DTWebApi.Issue issue, string logMessage) => + _executionContext.Setup(x => x.AddIssue(It.IsAny())) + .Callback((DTWebApi.IReadOnlyIssue issue) => { - _issues.Add(new Tuple(issue, logMessage)); - var message = !string.IsNullOrEmpty(logMessage) ? logMessage : issue.Message; - _trace.Info($"Issue '{issue.Type}': {message}"); + _issues.Add(issue); + _trace.Info($"Issue '{issue.Type}': {issue.Message}"); }); _executionContext.Setup(x => x.Write(It.IsAny(), It.IsAny())) .Callback((string tag, string message) => diff --git a/src/Test/L0/Worker/SetOutputFileCommandL0.cs b/src/Test/L0/Worker/SetOutputFileCommandL0.cs index 8af9695db..0a9fea55f 100644 --- a/src/Test/L0/Worker/SetOutputFileCommandL0.cs +++ b/src/Test/L0/Worker/SetOutputFileCommandL0.cs @@ -1,17 +1,11 @@ using System; using System.Collections.Generic; -using System.Globalization; using System.IO; -using System.Linq; using System.Text; -using System.Threading; -using System.Threading.Tasks; using System.Runtime.CompilerServices; using GitHub.Runner.Common.Util; using GitHub.Runner.Sdk; using GitHub.Runner.Worker; -using GitHub.Runner.Worker.Container; -using GitHub.Runner.Worker.Handlers; using Moq; using Xunit; using DTWebApi = GitHub.DistributedTask.WebApi; @@ -21,7 +15,7 @@ namespace GitHub.Runner.Common.Tests.Worker public sealed class SetOutputFileCommandL0 { private Mock _executionContext; - private List> _issues; + private List _issues; private Dictionary _outputs; private string _rootDirectory; private SetOutputFileCommand _setOutputFileCommand; @@ -390,7 +384,7 @@ namespace GitHub.Runner.Common.Tests.Worker private TestHostContext Setup([CallerMemberName] string name = "") { - _issues = new List>(); + _issues = new List(); _outputs = new Dictionary(); var hostContext = new TestHostContext(this, name); @@ -413,12 +407,11 @@ namespace GitHub.Runner.Common.Tests.Worker EnvironmentVariables = new Dictionary(VarUtil.EnvironmentVariableKeyComparer), WriteDebug = true, }); - _executionContext.Setup(x => x.AddIssue(It.IsAny(), It.IsAny())) - .Callback((DTWebApi.Issue issue, string logMessage) => + _executionContext.Setup(x => x.AddIssue(It.IsAny())) + .Callback((DTWebApi.IReadOnlyIssue issue) => { - _issues.Add(new Tuple(issue, logMessage)); - var message = !string.IsNullOrEmpty(logMessage) ? logMessage : issue.Message; - _trace.Info($"Issue '{issue.Type}': {message}"); + _issues.Add(issue); + _trace.Info($"Issue '{issue.Type}': {issue.Message}"); }); _executionContext.Setup(x => x.Write(It.IsAny(), It.IsAny())) .Callback((string tag, string message) =>