Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add notice annotation level and support more annotation fields #1175

Merged
merged 15 commits into from
Jul 13, 2021
Merged
1 change: 1 addition & 0 deletions src/Runner.Common/ExtensionManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ private List<IExtension> LoadExtensions<T>() where T : class, IExtension
Add<T>(extensions, "GitHub.Runner.Worker.RemoveMatcherCommandExtension, Runner.Worker");
Add<T>(extensions, "GitHub.Runner.Worker.WarningCommandExtension, Runner.Worker");
Add<T>(extensions, "GitHub.Runner.Worker.ErrorCommandExtension, Runner.Worker");
Add<T>(extensions, "GitHub.Runner.Worker.NoticeCommandExtension, Runner.Worker");
Add<T>(extensions, "GitHub.Runner.Worker.DebugCommandExtension, Runner.Worker");
Add<T>(extensions, "GitHub.Runner.Worker.GroupCommandExtension, Runner.Worker");
Add<T>(extensions, "GitHub.Runner.Worker.EndGroupCommandExtension, Runner.Worker");
Expand Down
5 changes: 5 additions & 0 deletions src/Runner.Common/JobServerQueue.cs
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,11 @@ private List<TimelineRecord> MergeTimelineRecords(List<TimelineRecord> timelineR
timelineRecord.WarningCount = rec.WarningCount;
}

if (rec.NoticeCount != null && rec.NoticeCount > 0)
{
timelineRecord.NoticeCount = rec.NoticeCount;
}

if (rec.Issues.Count > 0)
{
timelineRecord.Issues.Clear();
Expand Down
84 changes: 83 additions & 1 deletion src/Runner.Worker/ActionCommandManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@ public bool TryProcessCommand(IExecutionContext context, string input, Container
return false;
}

if (!ActionCommandManager.EnhancedAnnotationsEnabled(context) && actionCommand.Command == "notice")
{
context.Debug($"Enhanced Annotations not enabled on the server: 'notice' command will not be processed.");
return false;
luketomlinson marked this conversation as resolved.
Show resolved Hide resolved
}

// Serialize order
lock (_commandSerializeLock)
{
Expand Down Expand Up @@ -141,6 +147,10 @@ public bool TryProcessCommand(IExecutionContext context, string input, Container

return true;
}

internal static bool EnhancedAnnotationsEnabled(IExecutionContext context) {
luketomlinson marked this conversation as resolved.
Show resolved Hide resolved
return context.Global.Variables.GetBoolean("DistributedTask.EnhancedAnnotations") ?? false;
}
}

public interface IActionCommandExtension : IExtension
Expand Down Expand Up @@ -498,6 +508,13 @@ public sealed class ErrorCommandExtension : IssueCommandExtension
public override string Command => "error";
}

public sealed class NoticeCommandExtension : IssueCommandExtension
{
public override IssueType Type => IssueType.Notice;

public override string Command => "notice";
}

public abstract class IssueCommandExtension : RunnerService, IActionCommandExtension
{
public abstract IssueType Type { get; }
Expand All @@ -512,6 +529,11 @@ public void ProcessCommand(IExecutionContext context, string inputLine, ActionCo
command.Properties.TryGetValue(IssueCommandProperties.Line, out string line);
command.Properties.TryGetValue(IssueCommandProperties.Column, out string column);

if (!ActionCommandManager.EnhancedAnnotationsEnabled(context))
{
context.Debug("Enhanced Annotations not enabled on the server. The 'title', 'end_line', and 'end_column' fields are unsupported.");
}

Issue issue = new Issue()
{
Category = "General",
Expand Down Expand Up @@ -563,13 +585,73 @@ public void ProcessCommand(IExecutionContext context, string inputLine, ActionCo
context.AddIssue(issue);
}

public static void ValidateLinesAndColumns(ActionCommand command, IExecutionContext context)
{
command.Properties.TryGetValue(IssueCommandProperties.Line, out string line);
command.Properties.TryGetValue(IssueCommandProperties.EndLine, out string endLine);
command.Properties.TryGetValue(IssueCommandProperties.Column, out string column);
command.Properties.TryGetValue(IssueCommandProperties.EndColumn, out string endColumn);
luketomlinson marked this conversation as resolved.
Show resolved Hide resolved

var hasStartLine = int.TryParse(line, out int lineNumber);
var hasEndLine = int.TryParse(endLine, out int endLineNumber);
var hasStartColumn = int.TryParse(column, out int columnNumber);
var hasEndColumn = int.TryParse(endColumn, out int endColumnNumber);
var hasColumn = hasStartColumn || hasEndColumn;

if (hasEndLine && !hasStartLine)
{
context.Debug($"Invalid {command.Command} command value. '{IssueCommandProperties.EndLine}' can only be set if '{IssueCommandProperties.Line}' is provided");
command.Properties[IssueCommandProperties.Line] = endLine;
hasStartLine = true;
line = endLine;
}

luketomlinson marked this conversation as resolved.
Show resolved Hide resolved
if (hasEndColumn && !hasStartColumn)
{
context.Debug($"Invalid {command.Command} command value. '{IssueCommandProperties.EndColumn}' can only be set if '{IssueCommandProperties.Column}' is provided");
command.Properties[IssueCommandProperties.Column] = endColumn;
hasStartColumn = true;
column = endColumn;
}

if (!hasStartLine && hasColumn)
{
context.Debug($"Invalid {command.Command} command value. '{IssueCommandProperties.Column}' and '{IssueCommandProperties.EndColumn}' can only be set if '{IssueCommandProperties.Line}' value is provided.");
command.Properties.Remove(IssueCommandProperties.Column);
command.Properties.Remove(IssueCommandProperties.EndColumn);
}

if (hasEndLine && line != endLine && hasColumn)
{
context.Debug($"Invalid {command.Command} command value. '{IssueCommandProperties.Column}' and '{IssueCommandProperties.EndColumn}' cannot be set if '{IssueCommandProperties.Line}' and '{IssueCommandProperties.EndLine}' are different values.");
command.Properties.Remove(IssueCommandProperties.Column);
command.Properties.Remove(IssueCommandProperties.EndColumn);
}

if (hasStartLine && hasEndLine && endLineNumber < lineNumber)
{
context.Debug($"Invalid {command.Command} command value. '{IssueCommandProperties.EndLine}' cannot be less than '{IssueCommandProperties.Line}'.");
command.Properties.Remove(IssueCommandProperties.Line);
command.Properties.Remove(IssueCommandProperties.EndLine);
}

if (hasStartColumn && hasEndColumn && endColumnNumber < columnNumber)
{
context.Debug($"Invalid {command.Command} command value. '{IssueCommandProperties.EndColumn}' cannot be less than '{IssueCommandProperties.Column}'.");
command.Properties.Remove(IssueCommandProperties.Column);
command.Properties.Remove(IssueCommandProperties.EndColumn);
}
}

private static class IssueCommandProperties
{
public const String File = "file";
public const String Line = "line";
public const String EndLine = "endLine";
public const String Column = "col";
public const String EndColumn = "endColumn";
public const String Title = "title";
}

}

public sealed class GroupCommandExtension : GroupingCommandExtension
Expand Down
20 changes: 20 additions & 0 deletions src/Runner.Worker/ExecutionContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,24 @@ public void AddIssue(Issue issue, string logMessage = null)
}

_record.WarningCount++;
}
else if (issue.Type == IssueType.Notice)
{

// tracking line number for each issue in log file
luketomlinson marked this conversation as resolved.
Show resolved Hide resolved
// log UI use this to navigate from issue to log
if (!string.IsNullOrEmpty(logMessage))
{
long logLineNumber = Write(WellKnownTags.Notice, logMessage);
issue.Data["logFileLineNumber"] = logLineNumber.ToString();
}

if (_record.NoticeCount < _maxIssueCount)
{
_record.Issues.Add(issue);
}

_record.NoticeCount++;
}

_jobServerQueue.QueueTimelineRecordUpdate(_mainTimelineId, _record);
Expand Down Expand Up @@ -841,6 +859,7 @@ private void InitializeTimelineRecord(Guid timelineId, Guid timelineRecordId, Gu
_record.State = TimelineRecordState.Pending;
_record.ErrorCount = 0;
_record.WarningCount = 0;
_record.NoticeCount = 0;

if (parentTimelineRecordId != null && parentTimelineRecordId.Value != Guid.Empty)
{
Expand Down Expand Up @@ -1012,6 +1031,7 @@ public static class WellKnownTags
public static readonly string Command = "##[command]";
public static readonly string Error = "##[error]";
public static readonly string Warning = "##[warning]";
public static readonly string Notice = "##[notice]";
Copy link
Collaborator

Choose a reason for hiding this comment

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

we might want to make the UI changes first before we start writing this.... actually i guess it doesnt matter because no one will be using this initially. So we have some time to make that change in parallel

public static readonly string Debug = "##[debug]";
}
}
4 changes: 4 additions & 0 deletions src/Runner.Worker/Handlers/OutputManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,10 @@ private DTWebApi.Issue ConvertToIssue(IssueMatch match)
{
issueType = DTWebApi.IssueType.Warning;
}
else if (string.Equals(match.Severity, "notice", StringComparison.OrdinalIgnoreCase))
{
issueType = DTWebApi.IssueType.Notice;
}
else
{
_executionContext.Debug($"Skipped logging an issue for the matched line because the severity '{match.Severity}' is not supported.");
Expand Down
5 changes: 4 additions & 1 deletion src/Sdk/DTWebApi/WebApi/IssueType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ public enum IssueType
Error = 1,

[EnumMember]
Warning = 2
Warning = 2,

[EnumMember]
Notice = 3
}
}
8 changes: 8 additions & 0 deletions src/Sdk/DTWebApi/WebApi/TimelineRecord.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ private TimelineRecord(TimelineRecord recordToBeCloned)
this.RefName = recordToBeCloned.RefName;
this.ErrorCount = recordToBeCloned.ErrorCount;
this.WarningCount = recordToBeCloned.WarningCount;
this.NoticeCount = recordToBeCloned.NoticeCount;
this.AgentPlatform = recordToBeCloned.AgentPlatform;

if (recordToBeCloned.Log != null)
Expand Down Expand Up @@ -222,6 +223,13 @@ public Int32? WarningCount
set;
}

[DataMember(Order = 55)]
public Int32? NoticeCount
{
get;
set;
}

public List<Issue> Issues
{
get
Expand Down
83 changes: 83 additions & 0 deletions src/Test/L0/Worker/ActionCommandManagerL0.cs
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,84 @@ public void EchoProcessCommandDebugOn()
}
}

[Fact]
[Trait("Level", "L0")]
[Trait("Category", "Worker")]
public void IssueCommandInvalidColumns()
{
using (TestHostContext hc = CreateTestContext())
{
_ec.Setup(x => x.Write(It.IsAny<string>(), It.IsAny<string>()))
.Returns((string tag, string line) =>
{
hc.GetTrace().Info($"{tag} {line}");
return 1;
});

var registeredCommands = new HashSet<string>(new string[1]{ "warning" });
ActionCommand command;

// Columns when lines are different
ActionCommand.TryParseV2("::warning line=1,endLine=2,col=1,endColumn=2::this is a warning", registeredCommands, out command);
Assert.Equal("1", command.Properties["col"]);
IssueCommandExtension.ValidateLinesAndColumns(command, _ec.Object);
Assert.False(command.Properties.ContainsKey("col"));

// No lines with columns
ActionCommand.TryParseV2("::warning col=1,endColumn=2::this is a warning", registeredCommands, out command);
Assert.Equal("1", command.Properties["col"]);
Assert.Equal("2", command.Properties["endColumn"]);
IssueCommandExtension.ValidateLinesAndColumns(command, _ec.Object);
Assert.False(command.Properties.ContainsKey("col"));
Assert.False(command.Properties.ContainsKey("endColumn"));

// No line with endLine
ActionCommand.TryParseV2("::warning endLine=1::this is a warning", registeredCommands, out command);
Assert.Equal("1", command.Properties["endLine"]);
IssueCommandExtension.ValidateLinesAndColumns(command, _ec.Object);
Assert.Equal(command.Properties["endLine"], command.Properties["line"]);

// No column with endColumn
ActionCommand.TryParseV2("::warning line=1,endColumn=2::this is a warning", registeredCommands, out command);
Assert.Equal("2", command.Properties["endColumn"]);
IssueCommandExtension.ValidateLinesAndColumns(command, _ec.Object);
Assert.Equal(command.Properties["endColumn"], command.Properties["col"]);

// Empty Strings
ActionCommand.TryParseV2("::warning line=,endLine=3::this is a warning", registeredCommands, out command);
IssueCommandExtension.ValidateLinesAndColumns(command, _ec.Object);
Assert.Equal(command.Properties["line"], command.Properties["endLine"]);

// Nonsensical line values
ActionCommand.TryParseV2("::warning line=4,endLine=3::this is a warning", registeredCommands, out command);
IssueCommandExtension.ValidateLinesAndColumns(command, _ec.Object);
Assert.False(command.Properties.ContainsKey("line"));
Assert.False(command.Properties.ContainsKey("endLine"));

/// Nonsensical column values
ActionCommand.TryParseV2("::warning line=1,endLine=1,col=3,endColumn=2::this is a warning", registeredCommands, out command);
IssueCommandExtension.ValidateLinesAndColumns(command, _ec.Object);
Assert.False(command.Properties.ContainsKey("col"));
Assert.False(command.Properties.ContainsKey("endColumn"));

// Valid
ActionCommand.TryParseV2("::warning line=1,endLine=1,col=1,endColumn=2::this is a warning", registeredCommands, out command);
IssueCommandExtension.ValidateLinesAndColumns(command, _ec.Object);
Assert.Equal("1", command.Properties["line"]);
Assert.Equal("1", command.Properties["endLine"]);
Assert.Equal("1", command.Properties["col"]);
Assert.Equal("2", command.Properties["endColumn"]);

// Backwards compatibility
ActionCommand.TryParseV2("::warning line=1,col=1,file=test.txt::this is a warning", registeredCommands, out command);
IssueCommandExtension.ValidateLinesAndColumns(command, _ec.Object);
Assert.Equal("1", command.Properties["line"]);
Assert.False(command.Properties.ContainsKey("endLine"));
Assert.Equal("1", command.Properties["col"]);
Assert.False(command.Properties.ContainsKey("endColumn"));
}
}

[Fact]
[Trait("Level", "L0")]
[Trait("Category", "Worker")]
Expand Down Expand Up @@ -268,6 +346,7 @@ private TestHostContext CreateTestContext([CallerMemberName] string testName = "
new EchoCommandExtension(),
new InternalPluginSetRepoPathCommandExtension(),
new SetEnvCommandExtension(),
new WarningCommandExtension(),
};
foreach (var command in commands)
{
Expand All @@ -285,6 +364,10 @@ private TestHostContext CreateTestContext([CallerMemberName] string testName = "
_ec = new Mock<IExecutionContext>();
_ec.SetupAllProperties();
_ec.Setup(x => x.Global).Returns(new GlobalContext());
_ec.Object.Global.Variables = new Variables(
hostContext,
new Dictionary<string, VariableValue>()
);

// Command manager
_commandManager = new ActionCommandManager();
Expand Down