diff --git a/src/Runner.Common/JobServerQueue.cs b/src/Runner.Common/JobServerQueue.cs index d84614c15c3..dd8907ebeef 100644 --- a/src/Runner.Common/JobServerQueue.cs +++ b/src/Runner.Common/JobServerQueue.cs @@ -299,7 +299,7 @@ private async Task ProcessWebConsoleLinesQueueAsync(bool runOnce = false) { try { - // Give at most 60s for each request. + // Give at most 60s for each request. using (var timeoutTokenSource = new CancellationTokenSource(TimeSpan.FromSeconds(60))) { await _jobServer.AppendTimelineRecordFeedAsync(_scopeIdentifier, _hubName, _planId, _jobTimelineId, _jobTimelineRecordId, stepRecordId, batch.Select(logLine => logLine.Line).ToList(), batch[0].LineNumber, timeoutTokenSource.Token); @@ -600,8 +600,7 @@ private List MergeTimelineRecords(List timelineR { foreach (var issue in record.Issues) { - String source; - issue.Data.TryGetValue("sourcepath", out source); + string source = issue["sourcepath"]; Trace.Verbose($" Issue: c={issue.Category}, t={issue.Type}, s={source ?? string.Empty}, m={issue.Message}"); } } diff --git a/src/Runner.Listener/JobDispatcher.cs b/src/Runner.Listener/JobDispatcher.cs index aaa83b80c78..12a8b26d7f7 100644 --- a/src/Runner.Listener/JobDispatcher.cs +++ b/src/Runner.Listener/JobDispatcher.cs @@ -33,7 +33,7 @@ public interface IJobDispatcher : IRunnerService // This implementation of IJobDispatcher is not thread safe. // It is based on the fact that the current design of the runner is a dequeue // and processes one message from the message queue at a time. - // In addition, it only executes one job every time, + // In addition, it only executes one job every time, // and the server will not send another job while this one is still running. public sealed class JobDispatcher : RunnerService, IJobDispatcher { @@ -426,7 +426,7 @@ private async Task RunAsync(Pipelines.AgentJobRequestMessage message, string orc { workerOutput.Add(stdout.Data); } - + if (printToStdout) { term.WriteLine(stdout.Data, skipTracing: true); @@ -512,7 +512,7 @@ await processChannel.SendAsync( var accessToken = systemConnection?.Authorization?.Parameters["AccessToken"]; notification.JobStarted(message.JobId, accessToken, systemConnection.Url); - HostContext.WritePerfCounter($"SentJobToWorker_{requestId.ToString()}"); + HostContext.WritePerfCounter($"SentJobToWorker_{requestId}"); try { @@ -620,7 +620,7 @@ await processChannel.SendAsync( } } - // wait worker to exit + // wait worker to exit // if worker doesn't exit within timeout, then kill worker. completedTask = await Task.WhenAny(workerProcessTask, Task.Delay(-1, workerCancelTimeoutKillToken)); @@ -1014,7 +1014,7 @@ private async Task LogWorkerProcessUnhandledException(IJobServer jobServer, Pipe } var unhandledExceptionIssue = new Issue() { Type = IssueType.Error, Message = errorMessage }; - unhandledExceptionIssue.Data[Constants.Runner.InternalTelemetryIssueDataKey] = Constants.Runner.WorkerCrash; + unhandledExceptionIssue[Constants.Runner.InternalTelemetryIssueDataKey] = Constants.Runner.WorkerCrash; jobRecord.ErrorCount++; jobRecord.Issues.Add(unhandledExceptionIssue); await jobServer.UpdateTimelineRecordsAsync(message.Plan.ScopeIdentifier, message.Plan.PlanType, message.Plan.PlanId, message.Timeline.Id, new TimelineRecord[] { jobRecord }, CancellationToken.None); diff --git a/src/Runner.Worker/ActionCommandManager.cs b/src/Runner.Worker/ActionCommandManager.cs index 33fab1f2141..e77bf81369f 100644 --- a/src/Runner.Worker/ActionCommandManager.cs +++ b/src/Runner.Worker/ActionCommandManager.cs @@ -270,12 +270,9 @@ public void ProcessCommand(IExecutionContext context, string line, ActionCommand 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}"; + 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,12 +306,9 @@ public void ProcessCommand(IExecutionContext context, string line, ActionCommand { 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; + 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); } @@ -344,12 +338,9 @@ public void ProcessCommand(IExecutionContext context, string line, ActionCommand { 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; + 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); } @@ -618,16 +609,11 @@ public void ProcessCommand(IExecutionContext context, string inputLine, ActionCo 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,14 +644,13 @@ public void ProcessCommand(IExecutionContext context, string inputLine, ActionCo } } - 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)) + .ToList(); + var metadata = new IssueMetadata(issueCategory, false, null, filteredDictionaryEntries); + var issue = context.CreateIssue(this.Type, command.Data, metadata, true); context.AddIssue(issue); } diff --git a/src/Runner.Worker/ActionManager.cs b/src/Runner.Worker/ActionManager.cs index 49e14e68ae3..6014dc32026 100644 --- a/src/Runner.Worker/ActionManager.cs +++ b/src/Runner.Worker/ActionManager.cs @@ -1,7 +1,7 @@ using System; using System.Collections.Generic; using System.IO; -using System.IO.Compression; +using System.IO.Compression; // required for OS_WINDOWS using System.Linq; using System.Net; using System.Net.Http; diff --git a/src/Runner.Worker/DiagnosticLogManager.cs b/src/Runner.Worker/DiagnosticLogManager.cs index 698f2112c16..278a1f8eaae 100644 --- a/src/Runner.Worker/DiagnosticLogManager.cs +++ b/src/Runner.Worker/DiagnosticLogManager.cs @@ -5,7 +5,6 @@ using GitHub.DistributedTask.WebApi; using System.Linq; using System.Globalization; -using System.Threading.Tasks; using Pipelines = GitHub.DistributedTask.Pipelines; using GitHub.Runner.Common; using GitHub.Runner.Sdk; @@ -32,7 +31,7 @@ public sealed class DiagnosticLogManager : RunnerService, IDiagnosticLogManager { private static string DateTimeFormat = "yyyyMMdd-HHmmss"; public void UploadDiagnosticLogs(IExecutionContext executionContext, - IExecutionContext parentContext, + IExecutionContext parentContext, Pipelines.AgentJobRequestMessage message, DateTime jobStartTimeUtc) { diff --git a/src/Runner.Worker/ExecutionContext.cs b/src/Runner.Worker/ExecutionContext.cs index 9034a9b872d..4bfcfa5f75b 100644 --- a/src/Runner.Worker/ExecutionContext.cs +++ b/src/Runner.Worker/ExecutionContext.cs @@ -16,6 +16,7 @@ using GitHub.Runner.Sdk; using GitHub.Runner.Worker.Container; using GitHub.Runner.Worker.Handlers; +using GitHub.Services.Common; using Newtonsoft.Json; using ObjectTemplating = GitHub.DistributedTask.ObjectTemplating; using Pipelines = GitHub.DistributedTask.Pipelines; @@ -90,7 +91,8 @@ public interface IExecutionContext : IRunnerService void SetGitHubContext(string name, string value); void SetOutput(string name, string value, out string reference); void SetTimeout(TimeSpan? timeout); - void AddIssue(Issue issue, string message = null); + 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); @@ -124,7 +126,7 @@ public sealed class ExecutionContext : RunnerService, IExecutionContext 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; @@ -577,74 +579,77 @@ public void Progress(int percentage, string currentOperation = null) _jobServerQueue.QueueTimelineRecordUpdate(_mainTimelineId, _record); } - // This is not thread safe, the caller need to take lock before calling issue() - public void AddIssue(Issue issue, string logMessage = null) + // This is not thread safe, the caller needs to take lock before calling issue() + 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); - if (string.IsNullOrEmpty(logMessage)) - { - logMessage = issue.Message; - } + var result = new Issue() { + Type = issueType, + Message = refinedMessage, + }; - issue.Message = HostContext.SecretMasker.MaskSecrets(issue.Message); - if (issue.Message.Length > _maxIssueMessageLength) + if (metadata != null) { - issue.Message = issue.Message[.._maxIssueMessageLength]; + result.Category = metadata.Category; + result.IsInfrastructureIssue = metadata.IsInfrastructureIssue; + foreach (var kvp in metadata.Data) + { + result[kvp.Key] = kvp.Value; + } } - // Tracking the line number (logFileLineNumber) and step number (stepNumber) for each issue that gets created - // Actions UI from the run summary page use both values to easily link to an exact locations in logs where annotations originate from + // 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["stepNumber"] = _record.Order.ToString(); } - if (issue.Type == IssueType.Error) + string wellKnownTag = null; + Int32? previousCountForIssueType = null; + switch (issueType) { - if (!string.IsNullOrEmpty(logMessage)) - { - long logLineNumber = Write(WellKnownTags.Error, logMessage); - issue.Data["logFileLineNumber"] = logLineNumber.ToString(); - } - - if (_record.ErrorCount < _maxIssueCount) - { - _record.Issues.Add(issue); - } - - _record.ErrorCount++; + case IssueType.Error: + wellKnownTag = WellKnownTags.Error; + previousCountForIssueType = _record.ErrorCount++; + break; + case IssueType.Warning: + wellKnownTag = WellKnownTags.Warning; + previousCountForIssueType = _record.WarningCount++; + break; + case IssueType.Notice: + wellKnownTag = WellKnownTags.Notice; + previousCountForIssueType = _record.NoticeCount++; + break; } - else if (issue.Type == IssueType.Warning) + + if (!string.IsNullOrEmpty(wellKnownTag)) { - if (!string.IsNullOrEmpty(logMessage)) + if (writeToLog) { - long logLineNumber = Write(WellKnownTags.Warning, logMessage); - issue.Data["logFileLineNumber"] = logLineNumber.ToString(); + //Note that ::Write() has it's own secret masking logic + string logText = metadata?.LogMessageOverride ?? result.Message; + if (!string.IsNullOrEmpty(logText)) + { + long logLineNumber = Write(wellKnownTag, logText); + result["logFileLineNumber"] = logLineNumber.ToString(); + } } - - if (_record.WarningCount < _maxIssueCount) + if (previousCountForIssueType.GetValueOrDefault(0) < _maxIssueCount) { - _record.Issues.Add(issue); + _record.Issues.Add(result); } - - _record.WarningCount++; } - else if (issue.Type == IssueType.Notice) - { - if (!string.IsNullOrEmpty(logMessage)) - { - long logLineNumber = Write(WellKnownTags.Notice, logMessage); - issue.Data["logFileLineNumber"] = logLineNumber.ToString(); - } - if (_record.NoticeCount < _maxIssueCount) - { - _record.Issues.Add(issue); - } + return result; + } - _record.NoticeCount++; - } + + // 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 @@ -1017,16 +1022,7 @@ public void PublishStepTelemetry() if ((issue.Type == IssueType.Error || issue.Type == IssueType.Warning) && !string.IsNullOrEmpty(issue.Message)) { - string issueTelemetry; - if (issue.Message.Length > _maxIssueMessageLengthInTelemetry) - { - issueTelemetry = $"{issue.Message[.._maxIssueMessageLengthInTelemetry]}"; - } - else - { - issueTelemetry = issue.Message; - } - + string issueTelemetry = PrimitiveExtensions.TrimExcess(issue.Message, _maxIssueMessageLengthInTelemetry); StepTelemetry.ErrorMessages.Add(issueTelemetry); // Only send over the first 3 issues to avoid sending too much data. @@ -1200,19 +1196,23 @@ public static void Error(this IExecutionContext context, Exception ex) // Do not add a format string overload. See comment on ExecutionContext.Write(). public static void Error(this IExecutionContext context, string message) { - context.AddIssue(new Issue() { Type = IssueType.Error, Message = message }); + 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) { - context.AddIssue(new Issue() { Type = IssueType.Error, Message = message, IsInfrastructureIssue = true }); + var metadata = new IssueMetadata(null, true, null, 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) { - context.AddIssue(new Issue() { Type = IssueType.Warning, Message = message }); + 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 8b97c867d95..6ab611b5d87 100644 --- a/src/Runner.Worker/Handlers/OutputManager.cs +++ b/src/Runner.Worker/Handlers/OutputManager.cs @@ -4,6 +4,7 @@ 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; @@ -97,7 +98,7 @@ public void OnDataReceived(object sender, ProcessDataReceivedEventArgs e) var matchers = _matchers; // Strip color codes - var stripped = line.Contains(_colorCodePrefix) ? _colorCodeRegex.Replace(line, string.Empty) : line; + var refinedLine = line.Contains(_colorCodePrefix) ? _colorCodeRegex.Replace(line, string.Empty) : line; foreach (var matcher in matchers) { @@ -107,8 +108,7 @@ public void OnDataReceived(object sender, ProcessDataReceivedEventArgs e) // Match try { - match = matcher.Match(stripped); - + match = matcher.Match(refinedLine); break; } catch (RegexMatchTimeoutException ex) @@ -116,7 +116,7 @@ public void OnDataReceived(object sender, ProcessDataReceivedEventArgs e) if (attempt < _maxAttempts) { // Debug - _executionContext.Debug($"Timeout processing issue matcher '{matcher.Owner}' against line '{stripped}'. Exception: {ex.ToString()}"); + _executionContext.Debug($"Timeout processing issue matcher '{matcher.Owner}' against line '{refinedLine}'. Exception: {ex}"); } else { @@ -139,12 +139,10 @@ public void OnDataReceived(object sender, ProcessDataReceivedEventArgs e) // Convert to issue var issue = ConvertToIssue(match); - if (issue != null) { // Log issue - _executionContext.AddIssue(issue, stripped); - + _executionContext.AddIssue(issue); return; } } @@ -196,7 +194,7 @@ private void Remove(IssueMatcher matcher) } } - private DTWebApi.Issue ConvertToIssue(IssueMatch match) + private DTWebApi.IReadOnlyIssue ConvertToIssue(IssueMatch match) { // Validate the message if (string.IsNullOrWhiteSpace(match.Message)) @@ -225,18 +223,14 @@ private DTWebApi.Issue ConvertToIssue(IssueMatch match) 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 { @@ -249,7 +243,7 @@ private DTWebApi.Issue ConvertToIssue(IssueMatch match) { 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 { @@ -260,7 +254,7 @@ private DTWebApi.Issue ConvertToIssue(IssueMatch match) // Code if (!string.IsNullOrWhiteSpace(match.Code)) { - issue.Data["code"] = match.Code.Trim(); + issueData["code"] = match.Code.Trim(); } // File @@ -312,7 +306,7 @@ private DTWebApi.Issue ConvertToIssue(IssueMatch match) 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 { @@ -327,9 +321,11 @@ private DTWebApi.Issue ConvertToIssue(IssueMatch match) } 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, match.SourceText, issueData); + var issue = _executionContext.CreateIssue(issueType, match.Message, metadata, true); return issue; } diff --git a/src/Runner.Worker/IssueMatcher.cs b/src/Runner.Worker/IssueMatcher.cs index 0a6eb0fc4a4..752a5d86a04 100644 --- a/src/Runner.Worker/IssueMatcher.cs +++ b/src/Runner.Worker/IssueMatcher.cs @@ -8,6 +8,31 @@ namespace GitHub.Runner.Worker { public delegate void OnMatcherChanged(object sender, MatcherChangedEventArgs e); + + public sealed class IssueMetadata + { + public IssueMetadata(string key, string value) + : this(null, false, null, new []{ KeyValuePair.Create(key, value) }) + { + } + + public IssueMetadata(string category, bool infrastructureIssue, string logMessageOverride, IEnumerable> data) + { + this.Category = category; + this.IsInfrastructureIssue = infrastructureIssue; + this.LogMessageOverride = logMessageOverride; + + // Close-over the incoming IEnumerable to force immediate evaluation. + var empty = Enumerable.Empty>(); + this.Data = new Dictionary(data ?? empty, StringComparer.OrdinalIgnoreCase); + } + + public readonly string Category; + public readonly bool IsInfrastructureIssue; + public readonly string LogMessageOverride; + public readonly IEnumerable> Data; + } + public sealed class MatcherChangedEventArgs : EventArgs { public MatcherChangedEventArgs(IssueMatcherConfig config) @@ -69,7 +94,7 @@ public IssueMatch Match(string line) if (regexMatch.Success) { - return new IssueMatch(null, pattern, regexMatch.Groups, DefaultSeverity); + return new IssueMatch(line, null, pattern, regexMatch.Groups, DefaultSeverity); } return null; @@ -110,13 +135,13 @@ public IssueMatch Match(string line) } // Return - return new IssueMatch(runningMatch, pattern, regexMatch.Groups, DefaultSeverity); + return new IssueMatch(line, runningMatch, pattern, regexMatch.Groups, DefaultSeverity); } // Not the last pattern else { // Store the match - _state[i] = new IssueMatch(runningMatch, pattern, regexMatch.Groups); + _state[i] = new IssueMatch(line, runningMatch, pattern, regexMatch.Groups); } } // Not matched @@ -184,8 +209,9 @@ public IssuePattern(IssuePatternConfig config, TimeSpan timeout) public sealed class IssueMatch { - public IssueMatch(IssueMatch runningMatch, IssuePattern pattern, GroupCollection groups, string defaultSeverity = null) + public IssueMatch(string sourceText, IssueMatch runningMatch, IssuePattern pattern, GroupCollection groups, string defaultSeverity = null) { + SourceText = sourceText; File = runningMatch?.File ?? GetValue(groups, pattern.File); Line = runningMatch?.Line ?? GetValue(groups, pattern.Line); Column = runningMatch?.Column ?? GetValue(groups, pattern.Column); @@ -200,6 +226,8 @@ public IssueMatch(IssueMatch runningMatch, IssuePattern pattern, GroupCollection } } + public string SourceText { get; } + public string File { get; } public string Line { get; } @@ -455,7 +483,7 @@ public void Validate( if (Loop && Message == null) { throw new ArgumentException($"The {_loopPropertyName} pattern must set '{_messagePropertyName}'"); - } + } var regex = new Regex(Pattern ?? string.Empty, RegexOptions); var groupCount = regex.GetGroupNumbers().Length; diff --git a/src/Runner.Worker/JobExtension.cs b/src/Runner.Worker/JobExtension.cs index 246274734d9..52f039d3185 100644 --- a/src/Runner.Worker/JobExtension.cs +++ b/src/Runner.Worker/JobExtension.cs @@ -325,10 +325,10 @@ public async Task> InitializeJob(IExecutionContext jobContext, Pipel if (message.ContextData.TryGetValue("inputs", out var pipelineContextData)) { var inputs = pipelineContextData.AssertDictionary("inputs"); - if (inputs.Any()) + if (inputs.Any()) { context.Output($"##[group] Inputs"); - foreach (var input in inputs) + foreach (var input in inputs) { context.Output($" {input.Key}: {input.Value}"); } @@ -336,7 +336,7 @@ public async Task> InitializeJob(IExecutionContext jobContext, Pipel } } - if (!string.IsNullOrWhiteSpace(message.JobDisplayName)) + if (!string.IsNullOrWhiteSpace(message.JobDisplayName)) { context.Output($"Complete job name: {message.JobDisplayName}"); } @@ -660,8 +660,9 @@ private async Task CheckDiskSpaceAsync(IExecutionContext context, CancellationTo 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; + 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 562ef9068fd..b5a1bcc256e 100644 --- a/src/Runner.Worker/JobRunner.cs +++ b/src/Runner.Worker/JobRunner.cs @@ -84,7 +84,8 @@ public async Task RunAsync(Pipelines.AgentJobRequestMessage message, default: throw new ArgumentException(HostContext.RunnerShutdownReason.ToString(), nameof(HostContext.RunnerShutdownReason)); } - jobContext.AddIssue(new Issue() { Type = IssueType.Error, Message = errorMessage }); + 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 f15bf502b67..4b00293c702 100644 --- a/src/Sdk/Common/Common/Utility/PrimitiveExtensions.cs +++ b/src/Sdk/Common/Common/Utility/PrimitiveExtensions.cs @@ -27,6 +27,18 @@ public static DateTime FromUnixEpochTime(this Int64 unixTime) } } + public static string TrimExcess(string text, int maxLength) + { + string result = text; + + if (!string.IsNullOrEmpty(result) && result.Length > maxLength) + { + result = result.Substring(0, maxLength); + } + + return result; + } + public static string ToBase64StringNoPaddingFromString(string utf8String) { return ToBase64StringNoPadding(Encoding.UTF8.GetBytes(utf8String)); @@ -46,7 +58,7 @@ public static string FromBase64StringNoPaddingToString(string base64String) //These methods convert To and From base64 strings without padding //for JWT scenarios - //code taken from the JWS spec here: + //code taken from the JWS spec here: //http://tools.ietf.org/html/draft-ietf-jose-json-web-signature-08#appendix-C public static String ToBase64StringNoPadding(this byte[] bytes) { diff --git a/src/Sdk/DTWebApi/WebApi/Issue.cs b/src/Sdk/DTWebApi/WebApi/Issue.cs index af4d2f85034..2235dd86135 100644 --- a/src/Sdk/DTWebApi/WebApi/Issue.cs +++ b/src/Sdk/DTWebApi/WebApi/Issue.cs @@ -1,30 +1,38 @@ using System; using System.Collections.Generic; using System.Runtime.Serialization; +using GitHub.Services.Common; namespace GitHub.DistributedTask.WebApi { + public interface IReadOnlyIssue + { + IssueType Type { get; } + string Category { get; } + string Message { get; } + bool? IsInfrastructureIssue { get; } + string this[string key] { get; } + } + [DataContract] - public class Issue + public class Issue : IReadOnlyIssue { public Issue() + : this(null) { } - private Issue(Issue issueToBeCloned) + private Issue(Issue original) { - this.Type = issueToBeCloned.Type; - this.Category = issueToBeCloned.Category; - this.Message = issueToBeCloned.Message; - this.IsInfrastructureIssue = issueToBeCloned.IsInfrastructureIssue; - - if (issueToBeCloned.m_data != null) + this.EnsureInitialized(); + if (original != null) { - foreach (var item in issueToBeCloned.m_data) - { - this.Data.Add(item); - } + this.Type = original.Type; + this.Category = original.Category; + this.Message = original.Message; + this.IsInfrastructureIssue = original.IsInfrastructureIssue; + m_data.AddRange(original.m_data); } } @@ -36,14 +44,14 @@ public IssueType Type } [DataMember(Order = 2)] - public String Category + public string Category { get; set; } [DataMember(Order = 3)] - public String Message + public string Message { get; set; @@ -56,15 +64,16 @@ public bool? IsInfrastructureIssue set; } - public IDictionary Data + public string this[string key] { get { - if (m_data == null) - { - m_data = new Dictionary(StringComparer.OrdinalIgnoreCase); - } - return m_data; + m_data.TryGetValue(key, out string result); + return result; + } + set + { + m_data[key] = value; } } @@ -77,6 +86,7 @@ public Issue Clone() private void OnDeserialized(StreamingContext context) { SerializationHelper.Copy(ref m_serializedData, ref m_data, StringComparer.OrdinalIgnoreCase, true); + this.EnsureInitialized(); } [OnSerializing] @@ -91,9 +101,21 @@ private void OnSerialized(StreamingContext context) m_serializedData = null; } + /// + /// DataContractSerializer bypasses all constructor logic and inline initialization! + /// This method takes the place of a workhorse constructor for baseline initialization. + /// The expectation is for this logic to be accessible to constructors and also to the OnDeserialized helper. + /// + private void EnsureInitialized() + { + //Note that ?? is a short-circuiting operator. + m_data = m_data ?? new Dictionary(StringComparer.OrdinalIgnoreCase); + } + [DataMember(Name = "Data", EmitDefaultValue = false, Order = 4)] - private IDictionary m_serializedData; + private IDictionary m_serializedData; + + private IDictionary m_data; - private IDictionary m_data; } } diff --git a/src/Sdk/DTWebApi/WebApi/TimelineRecord.cs b/src/Sdk/DTWebApi/WebApi/TimelineRecord.cs index 51317d1713b..e42607e1019 100644 --- a/src/Sdk/DTWebApi/WebApi/TimelineRecord.cs +++ b/src/Sdk/DTWebApi/WebApi/TimelineRecord.cs @@ -10,69 +10,76 @@ namespace GitHub.DistributedTask.WebApi public sealed class TimelineRecord { public TimelineRecord() + : this(null) { - this.Attempt = 1; } private TimelineRecord(TimelineRecord recordToBeCloned) { - this.Attempt = recordToBeCloned.Attempt; - this.ChangeId = recordToBeCloned.ChangeId; - this.CurrentOperation = recordToBeCloned.CurrentOperation; - this.FinishTime = recordToBeCloned.FinishTime; - this.Id = recordToBeCloned.Id; - this.Identifier = recordToBeCloned.Identifier; - this.LastModified = recordToBeCloned.LastModified; - this.Location = recordToBeCloned.Location; - this.Name = recordToBeCloned.Name; - this.Order = recordToBeCloned.Order; - this.ParentId = recordToBeCloned.ParentId; - this.PercentComplete = recordToBeCloned.PercentComplete; - this.RecordType = recordToBeCloned.RecordType; - this.Result = recordToBeCloned.Result; - this.ResultCode = recordToBeCloned.ResultCode; - this.StartTime = recordToBeCloned.StartTime; - this.State = recordToBeCloned.State; - this.TimelineId = recordToBeCloned.TimelineId; - this.WorkerName = recordToBeCloned.WorkerName; - this.RefName = recordToBeCloned.RefName; - this.ErrorCount = recordToBeCloned.ErrorCount; - this.WarningCount = recordToBeCloned.WarningCount; - this.NoticeCount = recordToBeCloned.NoticeCount; - this.AgentPlatform = recordToBeCloned.AgentPlatform; - - if (recordToBeCloned.Log != null) + this.EnsureInitialized(); + if (recordToBeCloned != null) { - this.Log = new TaskLogReference + this.Attempt = recordToBeCloned.Attempt; + this.ChangeId = recordToBeCloned.ChangeId; + this.CurrentOperation = recordToBeCloned.CurrentOperation; + this.FinishTime = recordToBeCloned.FinishTime; + this.Id = recordToBeCloned.Id; + this.Identifier = recordToBeCloned.Identifier; + this.LastModified = recordToBeCloned.LastModified; + this.Location = recordToBeCloned.Location; + this.Name = recordToBeCloned.Name; + this.Order = recordToBeCloned.Order; + this.ParentId = recordToBeCloned.ParentId; + this.PercentComplete = recordToBeCloned.PercentComplete; + this.RecordType = recordToBeCloned.RecordType; + this.Result = recordToBeCloned.Result; + this.ResultCode = recordToBeCloned.ResultCode; + this.StartTime = recordToBeCloned.StartTime; + this.State = recordToBeCloned.State; + this.TimelineId = recordToBeCloned.TimelineId; + this.WorkerName = recordToBeCloned.WorkerName; + this.RefName = recordToBeCloned.RefName; + this.ErrorCount = recordToBeCloned.ErrorCount; + this.WarningCount = recordToBeCloned.WarningCount; + this.NoticeCount = recordToBeCloned.NoticeCount; + this.AgentPlatform = recordToBeCloned.AgentPlatform; + + if (recordToBeCloned.Log != null) { - Id = recordToBeCloned.Log.Id, - Location = recordToBeCloned.Log.Location, - }; - } + this.Log = new TaskLogReference + { + Id = recordToBeCloned.Log.Id, + Location = recordToBeCloned.Log.Location, + }; + } - if (recordToBeCloned.Details != null) - { - this.Details = new TimelineReference + if (recordToBeCloned.Details != null) { - ChangeId = recordToBeCloned.Details.ChangeId, - Id = recordToBeCloned.Details.Id, - Location = recordToBeCloned.Details.Location, - }; - } + this.Details = new TimelineReference + { + ChangeId = recordToBeCloned.Details.ChangeId, + Id = recordToBeCloned.Details.Id, + Location = recordToBeCloned.Details.Location, + }; + } - if (recordToBeCloned.m_issues?.Count> 0) - { - this.Issues.AddRange(recordToBeCloned.Issues.Select(i => i.Clone())); - } + if (recordToBeCloned.m_issues?.Count > 0) + { + m_issues.AddRange(recordToBeCloned.m_issues.Select(i => i.Clone())); + } - if (recordToBeCloned.m_previousAttempts?.Count > 0) - { - this.PreviousAttempts.AddRange(recordToBeCloned.PreviousAttempts); - } + if (recordToBeCloned.m_previousAttempts?.Count > 0) + { + m_previousAttempts.AddRange(recordToBeCloned.m_previousAttempts); + } - if (recordToBeCloned.m_variables?.Count > 0) - { - this.m_variables = recordToBeCloned.Variables.ToDictionary(k => k.Key, v => v.Value.Clone()); + if (recordToBeCloned.m_variables?.Count > 0) + { + // Don't pave over the case-insensitive Dictionary we initialized above. + foreach (var kvp in recordToBeCloned.m_variables) { + m_variables[kvp.Key] = kvp.Value.Clone(); + } + } } } @@ -234,10 +241,6 @@ public List Issues { get { - if (m_issues == null) - { - m_issues = new List(); - } return m_issues; } } @@ -274,22 +277,14 @@ public IList PreviousAttempts { get { - if (m_previousAttempts == null) - { - m_previousAttempts = new List(); - } return m_previousAttempts; } } - public IDictionary Variables + public IDictionary Variables { get { - if (m_variables == null) - { - m_variables = new Dictionary(StringComparer.OrdinalIgnoreCase); - } return m_variables; } } @@ -299,11 +294,32 @@ public TimelineRecord Clone() return new TimelineRecord(this); } + [OnDeserialized] + private void OnDeserialized(StreamingContext context) + { + this.EnsureInitialized(); + } + + /// + /// DataContractSerializer bypasses all constructor logic and inline initialization! + /// This method takes the place of a workhorse constructor for baseline initialization. + /// The expectation is for this logic to be accessible to constructors and also to the OnDeserialized helper. + /// + private void EnsureInitialized() + { + //Note that ?? is a short-circuiting operator. + m_issues = m_issues ?? new List(); + m_variables = m_variables ?? new Dictionary(StringComparer.OrdinalIgnoreCase); + m_previousAttempts = m_previousAttempts ?? new List(); + this.Attempt = Math.Max(this.Attempt, 1); + } + + [DataMember(Name = "Issues", EmitDefaultValue = false, Order = 60)] private List m_issues; [DataMember(Name = "Variables", EmitDefaultValue = false, Order = 80)] - private Dictionary m_variables; + private Dictionary m_variables; [DataMember(Name = "PreviousAttempts", EmitDefaultValue = false, Order = 120)] private List m_previousAttempts; diff --git a/src/Test/L0/TestUtil.cs b/src/Test/L0/TestUtil.cs index c6c60be56a7..1f245865551 100644 --- a/src/Test/L0/TestUtil.cs +++ b/src/Test/L0/TestUtil.cs @@ -2,6 +2,8 @@ using Xunit; using GitHub.Runner.Sdk; using System.Runtime.CompilerServices; +using GitHub.DistributedTask.WebApi; +using GitHub.Runner.Worker; namespace GitHub.Runner.Common.Tests { @@ -41,5 +43,26 @@ public static string GetTestDataPath() Assert.True(Directory.Exists(testDataDir)); return testDataDir; } + + public static IReadOnlyIssue CreateTestIssue(IssueType type, string message, IssueMetadata metadata, bool writeToLog) + { + var result = new Issue() + { + Type = type, + Message = message, + }; + + if (metadata != null) + { + result.Category = metadata.Category; + result.IsInfrastructureIssue = metadata.IsInfrastructureIssue; + foreach (var kvp in metadata.Data) + { + result[kvp.Key] = kvp.Value; + } + } + + return result; + } } } diff --git a/src/Test/L0/Worker/ActionCommandManagerL0.cs b/src/Test/L0/Worker/ActionCommandManagerL0.cs index cef826fd94e..63cfea9e3a2 100644 --- a/src/Test/L0/Worker/ActionCommandManagerL0.cs +++ b/src/Test/L0/Worker/ActionCommandManagerL0.cs @@ -32,10 +32,10 @@ public void EnablePluginInternalCommand() 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 +59,10 @@ public void DisablePluginInternalCommand() 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 +92,12 @@ public void StopProcessCommand() 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(TestUtil.CreateTestIssue); + _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 07cf9b48700..093305f33bf 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,11 @@ using GitHub.Runner.Sdk; using GitHub.Runner.Worker; using GitHub.Runner.Worker.Container; + +#if OS_WINDOWS +using System.IO.Compression; +#endif + using Moq; using Moq.Protected; using Xunit; @@ -2147,7 +2151,9 @@ private void Setup([CallerMemberName] string name = "", bool enableComposite = t _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(TestUtil.CreateTestIssue); + _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 b6e3f855b34..cd357d16961 100644 --- a/src/Test/L0/Worker/ActionManifestManagerL0.cs +++ b/src/Test/L0/Worker/ActionManifestManagerL0.cs @@ -4,10 +4,10 @@ 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 @@ public void Load_PluginAction() { Teardown(); } - } + } [Fact] [Trait("Level", "L0")] @@ -715,7 +715,7 @@ public void Load_CompositeActionNoUsing() //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,10 @@ private void Setup([CallerMemberName] string name = "") _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(TestUtil.CreateTestIssue); + _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 31d0743659b..236cba3d04d 100644 --- a/src/Test/L0/Worker/ActionRunnerL0.cs +++ b/src/Test/L0/Worker/ActionRunnerL0.cs @@ -3,20 +3,15 @@ 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 @@ public async void WarnInvalidInputs() 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,9 @@ private void Setup([CallerMemberName] string name = "") _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(TestUtil.CreateTestIssue); + _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 67631ed4bd7..38acb1e2ab2 100644 --- a/src/Test/L0/Worker/CreateStepSummaryCommandL0.cs +++ b/src/Test/L0/Worker/CreateStepSummaryCommandL0.cs @@ -11,6 +11,7 @@ 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 @@ public sealed class CreateStepSummaryCommandL0 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 @@ private TestHostContext Setup([CallerMemberName] string name = "") { var hostContext = new TestHostContext(this, name); - _issues = new List>(); + _issues = new List(); // Setup a job request TaskOrchestrationPlanReference plan = new(); @@ -247,13 +248,14 @@ private TestHostContext Setup([CallerMemberName] string name = "") 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(TestUtil.CreateTestIssue); _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 3aef7fb58ff..216737ab595 100644 --- a/src/Test/L0/Worker/ExecutionContextL0.cs +++ b/src/Test/L0/Worker/ExecutionContextL0.cs @@ -52,36 +52,36 @@ public void AddIssue_CountWarningsErrors() // 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(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.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(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 @@ public void AddIssue_TrimMessageSize() 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,13 +242,13 @@ public void AddIssue_AddStepAndLineNumberInformation() 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); - 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.Notice).Count() == 1)), Times.AtLeastOnce); + jobServerQueue.Verify(x => x.QueueTimelineRecordUpdate(It.IsAny(), It.Is(t => t.Issues.Where(i => i["stepNumber"] != null && i["logFileLineNumber"] != null && i.Type == IssueType.Error).Count() == 1)), Times.AtLeastOnce); + jobServerQueue.Verify(x => x.QueueTimelineRecordUpdate(It.IsAny(), It.Is(t => t.Issues.Where(i => i["stepNumber"] != null && i["logFileLineNumber"] != null && i.Type == IssueType.Warning).Count() == 1)), Times.AtLeastOnce); + jobServerQueue.Verify(x => x.QueueTimelineRecordUpdate(It.IsAny(), It.Is(t => t.Issues.Where(i => i["stepNumber"] != null && i["logFileLineNumber"] != null && i.Type == IssueType.Notice).Count() == 1)), Times.AtLeastOnce); } } @@ -626,12 +626,12 @@ public void PublishStepTelemetry_RegularStep() 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 @@ public void PublishStepTelemetry_EmbeddedStep() 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 @@ public void ActionVariables_AddedToVarsContext() 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 @@ public void ActionVariables_AddedToVarsContext() 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 @@ public void ActionVariables_DebugUsingVars() 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 @@ public void ActionVariables_DebugUsingVars() 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 @@ public void ActionVariables_SecretsPrecedenceForDebugUsingVars() 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 @@ public void ActionVariables_SecretsPrecedenceForDebugUsingVars() 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 e0364e9f0fc..9b74295e247 100644 --- a/src/Test/L0/Worker/OutputManagerL0.cs +++ b/src/Test/L0/Worker/OutputManagerL0.cs @@ -9,6 +9,7 @@ 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 @@ public sealed class OutputManagerL0 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 @@ public void AddMatcher_Clobber() 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 @@ public void AddMatcher_Prepend() 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 @@ public void MatcherCode() 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 @@ public void DoesNotResetMatchingMatcher() 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 @@ public void InitialMatchers() 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 @@ public void MatcherLineColumn() 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 @@ public void MatcherDoesNotReceiveCommand() 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 @@ public void MatcherRemoveColorCodes() }); 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 @@ public void RemoveMatcher() 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 @@ public void ResetsOtherMatchers() 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 @@ public void MatcherSeverity() 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 @@ public void MatcherTimeout() 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 @@ public async void MatcherFile() 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 @@ public async void MatcherFile_JobContainer() 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 @@ public async void MatcherFile_StepContainer() 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 @@ public async void MatcherFromPath() // 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 @@ private TestHostContext Setup( matchers?.Validate(); _onMatcherChanged = null; - _issues = new List>(); + _issues = new List(); _messages = new List(); _commands = new List(); @@ -983,10 +983,12 @@ private TestHostContext Setup( { _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(TestUtil.CreateTestIssue); + _executionContext.Setup(x => x.AddIssue(It.IsAny())) + .Callback((DTWebApi.IReadOnlyIssue issue) => { - _issues.Add(new Tuple(issue, logMessage)); + _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 45296c70978..01503f2b2ec 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 @@ private void WriteContent( 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 @@ private TestHostContext Setup([CallerMemberName] string name = "") 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 41f8499d582..a49b896556e 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 @@ private void WriteContent( private TestHostContext Setup([CallerMemberName] string name = "") { - _issues = new List>(); + _issues = new List(); var hostContext = new TestHostContext(this, name); @@ -411,12 +405,11 @@ private TestHostContext Setup([CallerMemberName] string name = "") 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 8af9695db46..0a9fea55fed 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 @@ private void WriteContent( 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 @@ private TestHostContext Setup([CallerMemberName] string name = "") 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) =>