Skip to content

[DocsVerifier] Always add file name to error log #741

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.

Sign up for GitHub

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

Merged
gewarren merged 2 commits into from
Sep 11, 2026
Merged

[DocsVerifier] Always add file name to error log #741

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ed522c7
always add file name to error
gewarren Sep 11, 2026
28ec26d
respond to feedback
gewarren Sep 11, 2026
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
53 changes: 48 additions & 5 deletions actions/docs-verifier/src/DocfxVerifier/PathVerifier.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,21 @@ public static async Task<bool> WriteResultsAsync(TextWriter writer, string? conf
configurationPath ??= FindDocfxConfigurationPath();
if (configurationPath is null)
{
await writer.WriteLineAsync("::error::Unable to find docfx.json in the repository root or its immediate subdirectories.");
await WriteErrorAsync(
writer,
"docfx.json",
lineNumber: null,
"Unable to find docfx.json in the repository root or its immediate subdirectories.");
return false;
}

if (!File.Exists(configurationPath))
{
await writer.WriteLineAsync($"::error::docfx.json file '{configurationPath}' does not exist.");
await WriteErrorAsync(
writer,
configurationPath,
lineNumber: null,
$"docfx.json file '{configurationPath}' does not exist.");
return false;
}

Expand Down Expand Up @@ -268,9 +276,44 @@ int GetLineNumber(long tokenStartIndex)
}

private static Task WriteErrorAsync(TextWriter writer, string filePath, int? lineNumber, string message)
=> lineNumber.HasValue
? writer.WriteLineAsync($"::error file={filePath},line={lineNumber.Value}::{message}")
: writer.WriteLineAsync($"::error file={filePath}::{message}");
{
string annotationFilePath = GetAnnotationFilePath(filePath);
string escapedMessage = EscapeCommandData(message);
return lineNumber.HasValue
? writer.WriteLineAsync($"::error file={annotationFilePath},line={lineNumber.Value}::{escapedMessage}")
: writer.WriteLineAsync($"::error file={annotationFilePath}::{escapedMessage}");
Comment thread
gewarren marked this conversation as resolved.
}

private static string GetAnnotationFilePath(string filePath)
{
string normalizedPath = NormalizePath(filePath);
if (!Path.IsPathRooted(filePath))
{
return EscapeCommandProperty(normalizedPath);
}

string repositoryRoot = Path.GetFullPath(Directory.GetCurrentDirectory());
string fullPath = Path.GetFullPath(filePath);
string relativePath = NormalizePath(Path.GetRelativePath(repositoryRoot, fullPath));
bool isUnderRepository = !relativePath.Equals("..", StringComparison.Ordinal)
&& !relativePath.StartsWith("../", StringComparison.Ordinal);
string pathForAnnotation = isUnderRepository ? relativePath : normalizedPath;
return EscapeCommandProperty(pathForAnnotation);
}

private static string EscapeCommandProperty(string value)
=> value
.Replace("%", "%25", StringComparison.Ordinal)
.Replace("\r", "%0D", StringComparison.Ordinal)
.Replace("\n", "%0A", StringComparison.Ordinal)
.Replace(":", "%3A", StringComparison.Ordinal)
.Replace(",", "%2C", StringComparison.Ordinal);

private static string EscapeCommandData(string value)
=> value
.Replace("%", "%25", StringComparison.Ordinal)
.Replace("\r", "%0D", StringComparison.Ordinal)
.Replace("\n", "%0A", StringComparison.Ordinal);

private readonly record struct ValidationError(int? LineNumber, string Path);

Expand Down
50 changes: 46 additions & 4 deletions actions/docs-verifier/src/RedirectionVerifier/RedirectTargetVerifier.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@ internal static async Task<bool> WriteResultsAsync(

if (!File.Exists(redirectionFilePath))
{
await writer.WriteLineAsync($"::error::Redirection file '{redirectionFilePath}' does not exist.");
await WriteErrorAsync(
writer,
redirectionFilePath,
lineNumber: null,
$"Redirection file '{redirectionFilePath}' does not exist.");
return false;
}

Expand Down Expand Up @@ -241,7 +245,45 @@ int GetLineNumber(long tokenStartIndex)
}

private static Task WriteErrorAsync(TextWriter writer, string filePath, int? lineNumber, string message)
=> lineNumber.HasValue
? writer.WriteLineAsync($"::error file={filePath},line={lineNumber.Value}::{message}")
: writer.WriteLineAsync($"::error file={filePath}::{message}");
{
string annotationFilePath = GetAnnotationFilePath(filePath);
string escapedMessage = EscapeCommandData(message);
return lineNumber.HasValue
? writer.WriteLineAsync($"::error file={annotationFilePath},line={lineNumber.Value}::{escapedMessage}")
: writer.WriteLineAsync($"::error file={annotationFilePath}::{escapedMessage}");
Comment thread
gewarren marked this conversation as resolved.
}

private static string GetAnnotationFilePath(string filePath)
{
string normalizedPath = NormalizePath(filePath);
if (!Path.IsPathRooted(filePath))
{
return EscapeCommandProperty(normalizedPath);
}

string repositoryRoot = Path.GetFullPath(Directory.GetCurrentDirectory());
string fullPath = Path.GetFullPath(filePath);
string relativePath = NormalizePath(Path.GetRelativePath(repositoryRoot, fullPath));
bool isUnderRepository = !relativePath.Equals("..", StringComparison.Ordinal)
&& !relativePath.StartsWith("../", StringComparison.Ordinal);
string pathForAnnotation = isUnderRepository ? relativePath : normalizedPath;
return EscapeCommandProperty(pathForAnnotation);
}

private static string EscapeCommandProperty(string value)
=> value
.Replace("%", "%25", StringComparison.Ordinal)
.Replace("\r", "%0D", StringComparison.Ordinal)
.Replace("\n", "%0A", StringComparison.Ordinal)
.Replace(":", "%3A", StringComparison.Ordinal)
.Replace(",", "%2C", StringComparison.Ordinal);

private static string EscapeCommandData(string value)
=> value
.Replace("%", "%25", StringComparison.Ordinal)
.Replace("\r", "%0D", StringComparison.Ordinal)
.Replace("\n", "%0A", StringComparison.Ordinal);

private static string NormalizePath(string path)
=> path.Replace('\\', '/');
}
57 changes: 56 additions & 1 deletion actions/docs-verifier/tests/GitHub.UnitTests/PathVerifierTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ await File.WriteAllTextAsync("docfx.json", """

Assert.False(result);
Assert.Contains("Invalid path 'missing/path/**/**.{md,yml}'.", output, StringComparison.Ordinal);
Assert.Contains(",line=5::Invalid path 'missing/path/**/**.{md,yml}'.", output, StringComparison.Ordinal);
Assert.Contains("::error file=docfx.json,line=5::Invalid path 'missing/path/**/**.{md,yml}'.", output, StringComparison.Ordinal);
}
finally
{
Expand Down Expand Up @@ -345,6 +345,61 @@ await File.WriteAllTextAsync(modifiedDocfxPath, """
}
}

[Fact]
public async Task WriteResultsAsyncReturnsFalseForMissingSpecifiedDocfxPathWithFileAnnotation()
{
await s_currentDirectoryLock.WaitAsync();
string testRoot = CreateTempDirectory();
string originalDirectory = Directory.GetCurrentDirectory();

try
{
Directory.SetCurrentDirectory(testRoot);
string missingDocfxPath = Path.Combine("missing-docs", "docfx.json");
using var writer = new StringWriter();

bool result = await PathVerifier.WriteResultsAsync(writer, missingDocfxPath);

string output = writer.ToString();
Assert.False(result);
Assert.Contains("docfx.json file", output, StringComparison.Ordinal);
Assert.Contains("file=missing-docs/docfx.json", output, StringComparison.Ordinal);
}
finally
{
Directory.SetCurrentDirectory(originalDirectory);
Directory.Delete(testRoot, recursive: true);
s_currentDirectoryLock.Release();
}
}

[Fact]
public async Task WriteResultsAsyncReturnsFalseWhenDocfxNotFoundWithFileAnnotation()
{
await s_currentDirectoryLock.WaitAsync();
string testRoot = CreateTempDirectory();
string originalDirectory = Directory.GetCurrentDirectory();

try
{
Directory.SetCurrentDirectory(testRoot);
using var writer = new StringWriter();

bool result = await PathVerifier.WriteResultsAsync(writer);

string output = writer.ToString();
Assert.False(result);
Assert.Contains("Unable to find docfx.json", output, StringComparison.Ordinal);
Assert.Contains("file=docfx.json", output, StringComparison.Ordinal);
}
finally
{
Directory.SetCurrentDirectory(originalDirectory);
Directory.Delete(testRoot, recursive: true);
s_currentDirectoryLock.Release();
}
}

[Fact]
public async Task WriteResultsAsyncAllowsTrailingCommas()
{
Expand Down
23 changes: 21 additions & 2 deletions actions/docs-verifier/tests/GitHub.UnitTests/RedirectTargetVerifierTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,10 @@ public async Task WriteResultsAsyncReturnsFalseFor404Url()
_ => Task.FromResult<HttpStatusCode?>(HttpStatusCode.NotFound));

Assert.False(result);
Assert.Contains("returns 404", writer.ToString(), StringComparison.Ordinal);
Assert.Contains(",line=5::Redirect target returns 404", writer.ToString(), StringComparison.Ordinal);
string output = writer.ToString();
Assert.Contains("returns 404", output, StringComparison.Ordinal);
Assert.Contains(Path.GetFileName(redirectionFilePath), output, StringComparison.Ordinal);
Assert.Contains(",line=5::Redirect target returns 404", output, StringComparison.Ordinal);
}
finally
{
Expand Down Expand Up @@ -100,6 +102,23 @@ public async Task WriteResultsAsyncReturnsFalseWhenLearnUrlCannotBeVerified()
}
}

[Fact]
public async Task WriteResultsAsyncReturnsFalseForMissingRedirectionFileWithFileAnnotation()
{
string redirectionFilePath = Path.Combine(Path.GetTempPath(), $"redirect-missing-{Guid.NewGuid():N}.json");
using var writer = new StringWriter();

bool result = await RedirectTargetVerifier.WriteResultsAsync(
writer,
redirectionFilePath,
_ => Task.FromResult<HttpStatusCode?>(HttpStatusCode.OK));

string output = writer.ToString();
Assert.False(result);
Assert.Contains("Redirection file", output, StringComparison.Ordinal);
Assert.Contains(Path.GetFileName(redirectionFilePath), output, StringComparison.Ordinal);
}

private static async Task<string> CreateRedirectionFileAsync(string redirectUrl)
{
string content = $$"""
Expand Down
Loading