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

Use Check Runs API to report on status checks (success and failures) #1129

Merged
merged 7 commits into from
Jun 24, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .github/workflows/renumber-sections.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,11 @@ on:
jobs:
renumber-sections:
runs-on: ubuntu-latest
permissions:
checks: write
env:
DOTNET_NOLOGO: true
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}

steps:
- name: Check out our repo
Expand All @@ -29,4 +32,4 @@ jobs:
- name: Run section renumbering dry run
run: |
cd tools
./run-section-renumber.sh --dryrun
./run-section-renumber.sh ${{ github.event.pull_request.head.sha }}
6 changes: 6 additions & 0 deletions tools/StandardAnchorTags/DiagnosticIDs.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
namespace StandardAnchorTags;
internal class DiagnosticIDs
{
public const string TOC001 = nameof(TOC001);
public const string TOC002 = nameof(TOC002);
}
BillWagner marked this conversation as resolved.
Show resolved Hide resolved
292 changes: 163 additions & 129 deletions tools/StandardAnchorTags/Program.cs

Large diffs are not rendered by default.

22 changes: 9 additions & 13 deletions tools/StandardAnchorTags/ReferenceUpdateProcessor.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Text;
using Utilities;

namespace StandardAnchorTags;

Expand All @@ -9,11 +10,12 @@ internal class ReferenceUpdateProcessor
private readonly IReadOnlyDictionary<string, SectionLink> linkMap;
private readonly bool dryRun;
private readonly string PathToFiles;
public int ErrorCount { get; private set; }
private readonly StatusCheckLogger logger;

public ReferenceUpdateProcessor(string pathToFiles, IReadOnlyDictionary<string, SectionLink> linkMap, bool dryRun)
public ReferenceUpdateProcessor(string pathToFiles, StatusCheckLogger logger, IReadOnlyDictionary<string, SectionLink> linkMap, bool dryRun)
{
PathToFiles = pathToFiles;
this.logger = logger;
this.linkMap = linkMap;
this.dryRun = dryRun;
}
Expand All @@ -30,7 +32,7 @@ public async Task ReplaceReferences(string file)
{
lineNumber++;
var updatedLine = line.Contains(sectionReference)
? ProcessSectionLinks(line, lineNumber, file)
? ProcessSectionLinks(line, lineNumber, inputPath)
: line;
await writeStream.WriteLineAsync(updatedLine);
}
Expand All @@ -47,7 +49,7 @@ public async Task ReplaceReferences(string file)
}
}

private string ProcessSectionLinks(string line, int lineNumber, string file)
private string ProcessSectionLinks(string line, int lineNumber, string path)
{
var returnedLine = new StringBuilder();
int index = 0;
Expand All @@ -60,14 +62,8 @@ private string ProcessSectionLinks(string line, int lineNumber, string file)
if ((referenceText.Length > 1) &&
(!linkMap.ContainsKey(referenceText)))
{
var msg = $"Section reference [{referenceText}] not found at line {lineNumber} in {file}";
if (dryRun)
{
ErrorCount++;
Console.WriteLine(msg);
}
else
throw new InvalidOperationException(msg);
var diagnostic = new Diagnostic(path, lineNumber, lineNumber, $"`{referenceText}` not found", DiagnosticIDs.TOC002);
logger.LogFailure(diagnostic);
} else
{
linkText = linkMap[referenceText].FormattedMarkdownLink;
Expand Down Expand Up @@ -135,4 +131,4 @@ private static Range ExpandToIncludeExistingLink(string line, Range range)

return new Range(previous, endIndex + 1);
}
}
}
4 changes: 4 additions & 0 deletions tools/StandardAnchorTags/StandardAnchorTags.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@
<Folder Include="Properties\" />
</ItemGroup>

<ItemGroup>
<PackageReference Include="System.CommandLine.DragonFruit" Version="0.4.0-alpha.22272.1" />
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\Utilities\Utilities.csproj" />
</ItemGroup>
Expand Down
23 changes: 13 additions & 10 deletions tools/StandardAnchorTags/TocSectionNumberBuilder.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System.Text;
using System.Text.RegularExpressions;
using Utilities;

namespace StandardAnchorTags;

Expand All @@ -25,6 +26,7 @@ private struct SectionHeader
private const string AnnexPattern = @"^[A-Z](\.\d+)*$";

private readonly string PathToStandardFiles;
private readonly StatusCheckLogger logger;
private readonly bool dryRun;

// String builder to store the full TOC for the standard.
Expand All @@ -39,9 +41,10 @@ private struct SectionHeader
/// <summary>
/// Construct the map Builder.
/// </summary>
public TocSectionNumberBuilder(string pathFromToolToStandard, bool dryRun)
public TocSectionNumberBuilder(string pathFromToolToStandard, StatusCheckLogger logger, bool dryRun)
{
PathToStandardFiles = pathFromToolToStandard;
this.logger = logger;
this.dryRun = dryRun;
}

Expand All @@ -55,19 +58,19 @@ public TocSectionNumberBuilder(string pathFromToolToStandard, bool dryRun)
/// </remarks>
public async Task AddFrontMatterTocEntries(string fileName)
{
using var stream = File.OpenText(Path.Combine(PathToStandardFiles,fileName));
string path = Path.Combine(PathToStandardFiles, fileName);
using var stream = File.OpenText(path);
string? line = await stream.ReadLineAsync();
if (line?.StartsWith("# ") == true)
{
if (line?.StartsWith("# ") == true)
{
var linkText = line[2..];
tocContent.AppendLine($"- [{linkText}]({fileName})");
// Done: return.
return;
}
var linkText = line[2..];
tocContent.AppendLine($"- [{linkText}]({fileName})");
// Done: return.
return;
}
// Getting here means this file doesn't have an H1. That's an error:
throw new InvalidOperationException($"File {fileName} doesn't have an H1 tag as its first line.");
var diagnostic = new Diagnostic(path, 1, 1, "File doesn't have an H1 tag as its first line.", DiagnosticIDs.TOC001);
logger.LogFailure(diagnostic);
}

public async Task AddContentsToTOC(string filename)
Expand Down
146 changes: 146 additions & 0 deletions tools/Utilities/StatusCheckLogger.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
using Octokit;
BillWagner marked this conversation as resolved.
Show resolved Hide resolved

namespace Utilities;

/// <summary>
/// Record for a single diagnostic
/// </summary>
/// <param name="file">The source file in the PR</param>
/// <param name="Message">The message for the output daignostic</param>
/// <param name="Id">The error message ID</param>
/// <param name="StartLine">The start line (index from 1)</param>
/// <param name="EndLine">The end line (index from 1)</param>
public record Diagnostic(string file, int StartLine, int EndLine, string Message, string Id);

/// <summary>
/// This class writes the status of the check to the console in the format GitHub supports
/// </summary>
/// <remarks>
/// For all of our tools, if all error and warning messages are formatted correctly, GitHub
/// will show those errors and warnings inline in the files tab for the PR. Let's format
/// them correctly.
/// </remarks>
/// <param name="pathToRoot">The path to the root of the repository</param>
/// <param name="toolName">The name of the tool that is running the check</param>
public class StatusCheckLogger(string pathToRoot, string toolName)
{
private List<NewCheckRunAnnotation> annotations = [];
public bool Success { get; private set; } = true;

// Utility method to format the path to unix style, from the root of the repository.
private string FormatPath(string path) => Path.GetRelativePath(pathToRoot, path).Replace(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar);

private void WriteMessageToConsole(string prefix, Diagnostic d) => Console.WriteLine($"{prefix}{toolName}-{d.Id}::file={FormatPath(d.file)},line={d.StartLine}::{d.Message}");

/// <summary>
/// Log a notice from the status check
/// </summary>
/// <param name="d">The diagnostic</param>
/// <remarks>
/// Add the diagnostic to the annotation list and
/// log the diagnostic information to console.
/// </remarks>
public void LogNotice(Diagnostic d)
{
WriteMessageToConsole("", d);
annotations.Add(
new(FormatPath(d.file),
d.StartLine, d.EndLine,
CheckAnnotationLevel.Notice, $"{d.Id}::{d.Message}")
);
}

/// <summary>
/// Log a warning from the status check
/// </summary>
/// <param name="d">The diagnostic</param>
/// <remarks>
/// Add the diagnostic to the annotation list and
/// log the warning notice to the console.
/// </remarks>
public void LogWarning(Diagnostic d)
{
WriteMessageToConsole("⚠️", d);
annotations.Add(
new(FormatPath(d.file),
d.StartLine, d.EndLine,
CheckAnnotationLevel.Warning, $"{d.Id}::{d.Message}")
);
Success = false;
BillWagner marked this conversation as resolved.
Show resolved Hide resolved
}

/// <summary>
/// Log a failure from the status check
/// </summary>
/// <param name="d">The diagnostic</param>
/// <remarks>
/// Add the diagnostic to the annotation list and
/// log the failure notice to the console.
/// This method is distinct from <see cref="ExitOnFailure(Diagnostic)"/> in
/// that this method does not throw an exception. Its purpose is to log
/// the failure but allow the tool to continue running further checks.
/// </remarks>
public void LogFailure(Diagnostic d)
{
WriteMessageToConsole("❌", d);
annotations.Add(
new(FormatPath(d.file),
d.StartLine, d.EndLine,
CheckAnnotationLevel.Failure, $"{d.Id}::{d.Message}")
);
Success = false;
}

/// <summary>
/// Log a failure from the status check and throw for exit
/// </summary>
/// <param name="d">The diagnostic</param>
/// <remarks>
/// Add the diagnostic to the annotation list and
/// log the failure notice to the console.
/// This method is distinct from <see cref="LogFailure(Diagnostic)"/> in
/// that this method throws an exception. Its purpose is to log
/// the failure and immediately exit, foregoing any further checks.
/// </remarks>
public void ExitOnFailure(Diagnostic d)
{
LogFailure(d);
throw new InvalidOperationException(d.Message);
}

/// <summary>
/// Return the object required for a full status check
/// </summary>
/// <param name="owner">The GitHub owner (or organization)</param>
/// <param name="repo">The GitHub repo name</param>
/// <param name="sha">The head sha when running as a GitHub action</param>
/// <returns>The full check run result object</returns>
public async Task BuildCheckRunResult(string token, string owner, string repo, string sha)
{
NewCheckRun result = new(toolName, sha)
{
Status = CheckStatus.Completed,
Conclusion = Success ? CheckConclusion.Success : CheckConclusion.Failure,
Output = new($"{toolName} Check Run results", $"{toolName} result is {(Success ? "success" : "failure")} with {annotations.Count} diagnostics.")
{
Annotations = annotations
}
};

var prodInformation = new ProductHeaderValue("TC49-TG2", "1.0.0");
var tokenAuth = new Credentials(token);
var client = new GitHubClient(prodInformation);
client.Credentials = tokenAuth;

try
{
await client.Check.Run.Create(owner, repo, result);
}
// If the token does not have the correct permissions, we will get a 403
// Once running on a branch on the dotnet org, this should work correctly.
catch (Octokit.ForbiddenException)
{
Console.WriteLine("===== WARNING: Could not create a check run.=====");
}
}
}
1 change: 1 addition & 0 deletions tools/Utilities/Utilities.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Octokit" Version="11.0.1" />
<PackageReference Include="System.Text.Json" Version="8.0.3" />
</ItemGroup>

Expand Down
7 changes: 1 addition & 6 deletions tools/run-section-renumber.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,7 @@ set -e

declare -r PROJECT=StandardAnchorTags

if [ "$1" == "--dryrun" ]
then
echo "Performing a dry run"
fi

dotnet run --project $PROJECT -- $1
dotnet run --project $PROJECT -- --owner dotnet --repo csharpstandard --dryrun true --head-sha $1
BillWagner marked this conversation as resolved.
Show resolved Hide resolved

if [ -n "$GITHUB_OUTPUT" ]
then
Expand Down
Loading