Skip to content

Commit

Permalink
ongoing
Browse files Browse the repository at this point in the history
  • Loading branch information
Konrad Jamrozik committed Jan 11, 2023
1 parent 3418b9a commit f784ccd
Show file tree
Hide file tree
Showing 13 changed files with 183 additions and 176 deletions.
2 changes: 1 addition & 1 deletion .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ insert_final_newline = true
# Organize usings
dotnet_sort_system_directives_first = true
# this. preferences
dotnet_style_qualification_for_field = true:suggestion
dotnet_style_qualification_for_field = false:suggestion
dotnet_style_qualification_for_property = false:silent
dotnet_style_qualification_for_method = false:silent
dotnet_style_qualification_for_event = false:silent
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@
namespace Azure.Sdk.Tools.CodeOwnersParser.Tests;

[TestFixture]
public class CodeOwnersFileTests
public class CodeownersFileTests
{
/// <summary>
/// A battery of test cases specifying behavior of new logic matching target
/// path to CODEOWNERS entries, and comparing it to existing, legacy logic.
///
/// The logic that has changed is located in CodeOwnersFile.FindOwnersForClosestMatch.
/// The logic that has changed is located in CodeownersFile.GetMatchingCodeownersEntry.
///
/// In the test case table below, any discrepancy between legacy and new
/// matcher expected matches that doesn't pertain to wildcard matching denotes
Expand Down Expand Up @@ -135,33 +135,31 @@ public class CodeOwnersFileTests
};

/// <summary>
/// Exercises Azure.Sdk.Tools.CodeOwnersParser.Tests.CodeOwnersFileTests.testCases.
/// Exercises Azure.Sdk.Tools.CodeOwnersParser.Tests.CodeownersFileTests.testCases.
/// See comment on that member for details.
/// </summary>
[TestCaseSource(nameof(testCases))]
public void TestParseAndFindOwnersForClosestMatch(TestCase testCase)
{
List<CodeOwnerEntry>? codeownersEntries =
CodeOwnersFile.ParseContent(testCase.CodeownersPath + "@owner");
List<CodeownersEntry>? codeownersEntries =
CodeownersFile.GetCodeownersEntries(testCase.CodeownersPath + "@owner");

VerifyFindOwnersForClosestMatch(testCase, codeownersEntries, useNewImpl: false, testCase.ExpectedLegacyMatch);
VerifyFindOwnersForClosestMatch(testCase, codeownersEntries, useNewImpl: true, testCase.ExpectedNewMatch);
}

private static void VerifyFindOwnersForClosestMatch(
TestCase testCase,
List<CodeOwnerEntry> codeownersEntries,
List<CodeownersEntry> codeownersEntries,
bool useNewImpl,
bool expectedMatch)
{
CodeOwnerEntry? entryLegacy =
CodeownersEntry? entryLegacy =
// Act
CodeOwnersFile.FindOwnersForClosestMatch(
codeownersEntries,
testCase.TargetPath,
useNewFindOwnersForClosestMatchImpl: useNewImpl);
CodeownersFile.GetMatchingCodeownersEntry(testCase.TargetPath,
codeownersEntries, useNewImpl: useNewImpl);

Assert.That(entryLegacy.Owners.Count, Is.EqualTo(expectedMatch ? 1 : 0));
Assert.That(entryLegacy.Owners, Has.Count.EqualTo(expectedMatch ? 1 : 0));
}

public record TestCase(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ public void Dispose()
Console.SetOut(this.originalOutput);
this.stringWriter.Dispose();
this.originalOutput.Dispose();
// https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1816
GC.SuppressFinalize(this);
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ namespace Azure.Sdk.Tools.RetrieveCodeOwners.Tests
/// This is test of console app Azure.Sdk.Tools.RetrieveCodeOwners.
/// </summary>
[TestFixture]
public class MainTests
public class ProgramTests
{
private const string CodeOwnersFilePath = "CODEOWNERS";
private const string CodeownersFilePath = "CODEOWNERS";

private static readonly object[] sourceLists =
{
Expand All @@ -27,28 +27,26 @@ public class MainTests
[TestCaseSource(nameof(sourceLists))]
public void TestOnNormalOutput(string targetDirectory, bool includeUserAliasesOnly, List<string> expectedReturn)
{
using (var consoleOutput = new ConsoleOutput())
{
Program.Main(CodeOwnersFilePath, targetDirectory, includeUserAliasesOnly);
var output = consoleOutput.GetOutput();
TestExpectResult(expectedReturn, output);
consoleOutput.Dispose();
}
using var consoleOutput = new ConsoleOutput();
Program.Main(targetDirectory, CodeownersFilePath, includeUserAliasesOnly);
var output = consoleOutput.GetOutput();
TestExpectResult(expectedReturn, output);
consoleOutput.Dispose();
}

[TestCase("PathNotExist")]
[TestCase("http://testLink")]
[TestCase("https://testLink")]
public void TestOnError(string codeOwnerPath)
public void TestOnError(string codeownersPath)
{
Assert.That(Program.Main(codeOwnerPath, "sdk"), Is.EqualTo(1));
Assert.That(Program.Main("sdk", codeownersPath), Is.EqualTo(1));
}

private static void TestExpectResult(List<string> expectReturn, string output)
{
CodeOwnerEntry? codeOwnerEntry = JsonSerializer.Deserialize<CodeOwnerEntry>(output);
List<string> actualReturn = codeOwnerEntry!.Owners;
Assert.That(actualReturn.Count, Is.EqualTo(expectReturn.Count));
CodeownersEntry? codeownersEntry = JsonSerializer.Deserialize<CodeownersEntry>(output);
List<string> actualReturn = codeownersEntry!.Owners;
Assert.That(actualReturn, Has.Count.EqualTo(expectReturn.Count));
for (int i = 0; i < actualReturn.Count; i++)
{
Assert.That(actualReturn[i], Is.EqualTo(expectReturn[i]));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,36 +5,40 @@
namespace Azure.Sdk.Tools.RetrieveCodeOwners
{
/// <summary>
/// The tool command to retrieve code owners.
/// See Program.Main comment.
/// </summary>
public static class Program
{
/// <summary>
/// Retrieves CODEOWNERS information for specific section of the repo
/// Given targetPath and CODEOWNERS file codeownersFilePath,
/// prints out to stdout owners of the targetPath as determined by the CODEOWNERS file.
/// </summary>
/// <param name="codeOwnerFilePath">The path of CODEOWNERS file in repo</param>
/// <param name="targetDirectory">The directory whose information is to be retrieved</param>
/// <param name="filterOutNonUserAliases">The option to filter out code owner team alias.</param>
/// <param name="targetPath">The path whose owners are to be determined.</param>
/// <param name="codeownersFilePath">The path to the CODEOWNERS file with ownership information.</param>
/// <param name="excludeNonUserAliases">Whether owners that aren't users should be excluded from the returned owners.</param>
/// <returns>Exit code</returns>

public static int Main(
string codeOwnerFilePath,
string targetDirectory,
bool filterOutNonUserAliases = false
)
string targetPath,
string codeownersFilePath,
bool excludeNonUserAliases = false)
{
var target = targetDirectory.Trim();
try {
var codeOwnerEntry = CodeOwnersFile.ParseAndFindOwnersForClosestMatch(codeOwnerFilePath, target);
if (filterOutNonUserAliases)
targetPath = targetPath.Trim();
try
{
var codeownersEntry = CodeownersFile.GetMatchingCodeownersEntry(targetPath, codeownersFilePath);
if (excludeNonUserAliases)
{
codeOwnerEntry.FilterOutNonUserAliases();
codeownersEntry.ExcludeNonUserAliases();
}
var codeOwnerJson = JsonSerializer.Serialize<CodeOwnerEntry>(codeOwnerEntry, new JsonSerializerOptions { WriteIndented = true });
Console.WriteLine(codeOwnerJson);

string codeownersJson = JsonSerializer.Serialize<CodeownersEntry>(codeownersEntry,
new JsonSerializerOptions { WriteIndented = true });

Console.WriteLine(codeownersJson);
return 0;
}
catch (Exception e) {
catch (Exception e)
{
Console.Error.WriteLine(e.Message);
return 1;
}
Expand Down
124 changes: 65 additions & 59 deletions tools/code-owners-parser/CodeOwnersParser/CodeOwnersFile.cs
Original file line number Diff line number Diff line change
@@ -1,109 +1,115 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;

namespace Azure.Sdk.Tools.CodeOwnersParser
{
public static class CodeOwnersFile
public static class CodeownersFile
{
public static List<CodeOwnerEntry> ParseFile(string filePathOrUrl)
public static List<CodeownersEntry> GetCodeownersEntriesFromFileOrUrl(string codeownersFilePathOrUrl)
{
string content = FileHelpers.GetFileContents(filePathOrUrl);
return ParseContent(content);
string content = FileHelpers.GetFileOrUrlContents(codeownersFilePathOrUrl);
return GetCodeownersEntries(content);
}

public static List<CodeOwnerEntry> ParseContent(string fileContent)
public static List<CodeownersEntry> GetCodeownersEntries(string codeownersContent)
{
List<CodeOwnerEntry> entries = new List<CodeOwnerEntry>();
List<CodeownersEntry> entries = new List<CodeownersEntry>();

// An entry ends when we get to a path (a real path or a commented dummy path)
using (StringReader sr = new StringReader(fileContent))
using (StringReader sr = new StringReader(codeownersContent))
{
CodeOwnerEntry entry = new CodeOwnerEntry();
CodeownersEntry entry = new CodeownersEntry();

// we are going to read line by line until we find a line that is not a comment OR that is using the placeholder entry inside the comment.
// while we are trying to find the folder entry, we parse all comment lines to extract the labels from it.
// when we find the path or placeholder, we add the completed entry and create a new one.
while (sr.ReadLine() is { } line)
{
line = NormalizeLine(line);

if (string.IsNullOrWhiteSpace(line))
{
continue;
}

if (!line.StartsWith("#") || line.IndexOf(CodeOwnerEntry.MissingFolder, System.StringComparison.OrdinalIgnoreCase) >= 0)
{
// If this is not a comment line OR this is a placeholder entry

entry.ParseOwnersAndPath(line);

// only add it if it is a valid entry
if (entry.IsValid)
{
entries.Add(entry);
}

// create a new entry.
entry = new CodeOwnerEntry();
}
else if (line.StartsWith("#"))
{
// try to process the line in case there are markers that need to be extracted
entry.ProcessLabelsOnLine(line);
}

ProcessCodeownersLine(line, entry, entries);
}
}
return entries;
}

public static CodeOwnerEntry ParseAndFindOwnersForClosestMatch(
string codeOwnersFilePathOrUrl,
public static CodeownersEntry GetMatchingCodeownersEntry(
string targetPath,
bool useNewFindOwnersForClosestMatchImpl = false)
string codeownersFilePathOrUrl,
bool useNewImpl = false)
{
var codeOwnerEntries = ParseFile(codeOwnersFilePathOrUrl);
return FindOwnersForClosestMatch(codeOwnerEntries, targetPath, useNewFindOwnersForClosestMatchImpl);
var codeownersEntries = GetCodeownersEntriesFromFileOrUrl(codeownersFilePathOrUrl);
return GetMatchingCodeownersEntry(targetPath, codeownersEntries, useNewImpl);
}

public static CodeOwnerEntry FindOwnersForClosestMatch(
List<CodeOwnerEntry> codeOwnerEntries,
public static CodeownersEntry GetMatchingCodeownersEntry(
string targetPath,
bool useNewFindOwnersForClosestMatchImpl = false)
List<CodeownersEntry> codeownersEntries,
bool useNewImpl = false)
{
Debug.Assert(targetPath != null);
return useNewImpl
? new MatchedCodeownersEntry(codeownersEntries, targetPath).Value
: GetMatchingCodeownersEntryLegacyImpl(codeownersEntries, targetPath);
}

private static void ProcessCodeownersLine(string line, CodeownersEntry entry, List<CodeownersEntry> entries)
{
return useNewFindOwnersForClosestMatchImpl
? new MatchedCodeOwnerEntry(codeOwnerEntries, targetPath).Value
: FindOwnersForClosestMatchLegacyImpl(codeOwnerEntries, targetPath);
line = NormalizeLine(line);

if (string.IsNullOrWhiteSpace(line))
{
return;
}

if (!IsCommentLine(line) || IsPlaceholderEntry(line))
{
entry.ParseOwnersAndPath(line);

// only add it if it is a valid entry
if (entry.IsValid)
entries.Add(entry);
}
else if (line.StartsWith("#"))
{
// try to process the line in case there are markers that need to be extracted
entry.ProcessLabelsOnLine(line);
}
}

private static CodeOwnerEntry FindOwnersForClosestMatchLegacyImpl(List<CodeOwnerEntry> codeOwnerEntries,
private static bool IsPlaceholderEntry(string line)
=> line.Contains(CodeownersEntry.MissingFolder, StringComparison.OrdinalIgnoreCase);

private static bool IsCommentLine(string line)
=> line.StartsWith("#");

private static string NormalizeLine(string line)
=> !string.IsNullOrEmpty(line)
// Remove tabs and trim extra whitespace
? line.Replace('\t', ' ').Trim()
: line;

private static CodeownersEntry GetMatchingCodeownersEntryLegacyImpl(
List<CodeownersEntry> codeownersEntries,
string targetPath)
{
// Normalize the start and end of the paths by trimming slash
targetPath = targetPath.Trim('/');

// We want to find the match closest to the bottom of the codeowners file.
// CODEOWNERS sorts the paths in order of 'RepoPath', 'ServicePath' and then 'PackagePath'.
for (int i = codeOwnerEntries.Count - 1; i >= 0; i--)
for (int i = codeownersEntries.Count - 1; i >= 0; i--)
{
string codeOwnerPath = codeOwnerEntries[i].PathExpression.Trim('/');
string codeownersPath = codeownersEntries[i].PathExpression.Trim('/');
// Note that this only matches on paths without glob patterns which is good enough
// for our current scenarios but in the future might need to support globs
if (targetPath.StartsWith(codeOwnerPath, StringComparison.OrdinalIgnoreCase))
if (targetPath.StartsWith(codeownersPath, StringComparison.OrdinalIgnoreCase))
{
return codeOwnerEntries[i];
return codeownersEntries[i];
}
}

return new CodeOwnerEntry();
return new CodeownersEntry();
}

private static string NormalizeLine(string line)
=> !string.IsNullOrEmpty(line)
// Remove tabs and trim extra whitespace
? line.Replace('\t', ' ').Trim()
: line;
}
}
Loading

0 comments on commit f784ccd

Please sign in to comment.