Skip to content

Commit

Permalink
Add detection of CODEOWNERS paths that are missing suffix slash; ad…
Browse files Browse the repository at this point in the history 
…d support for diffing to `CODEOWNERS` version with some paths removed; add tools for analyzing azure-sdk-for-python `CODEOWNERS`. (#5245)
  • Loading branch information
Konrad Jamrozik authored Jan 27, 2023
1 parent e33ae8f commit b3668aa
Show file tree
Hide file tree
Showing 2 changed files with 107 additions and 14 deletions.
118 changes: 105 additions & 13 deletions ...veCodeOwners.Tests/CodeownersDiffTests.cs → ...rs.Tests/CodeownersManualAnalysisTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ namespace Azure.Sdk.Tools.RetrieveCodeOwners.Tests;
/// selecting "|" as column separator.
/// </summary>
[TestFixture(Ignore = "Tools to be used manually")]
public class CodeownersDiffTests
public class CodeownersManualAnalysisTests
{
private const string DefaultIgnoredPathPrefixes = Program.DefaultIgnoredPrefixes;
private const string OwnersDiffOutputPathSuffix = "_owners_diff.csv";
Expand All @@ -42,17 +42,22 @@ public class CodeownersDiffTests
// <commonParent>/azure-sdk-for-.../
// ...
private const string AzureSdkForNetTargetDirPathSuffix = "/../azure-sdk-for-net";
private const string AzureSdkForNetCodeownersPathSuffix = AzureSdkForNetTargetDirPathSuffix + "/.github/CODEOWNERS";
private const string AzureSdkForPythonTargetDirPathSuffix = "/../azure-sdk-for-python";
// TODO: add more repos here.

private const string CodeownersFilePathSuffix = "/.github/CODEOWNERS";

// Current dir, ".", is "/artifacts/bin/Azure.Sdk.Tools.CodeOwnersParser.Tests/Debug/net6.0".
private const string CurrentDir = "/artifacts/bin/Azure.Sdk.Tools.CodeOwnersParser.Tests/Debug/net6.0";

#region Owners diff

[Test]
public void WriteToFileCodeownersMatcherDiffForAzureSdkTools()
{
// Empty string here means to just use the root directory of the local "azure-sdk-tools" clone.
var targetDirPathSuffix = "";
var codeownersPathSuffix = "/.github/CODEOWNERS";
var codeownersPathSuffix = CodeownersFilePathSuffix;
var ignoredPrefixes = ".git|artifacts";
WriteToFileOwnersDiff(new[]
{
Expand All @@ -67,14 +72,33 @@ public void WriteToFileCodeownersMatcherDiffForAzureSdkForNet()
WriteToFileOwnersDiff(
new[]
{
(AzureSdkForNetTargetDirPathSuffix, AzureSdkForNetCodeownersPathSuffix,
(AzureSdkForNetTargetDirPathSuffix, CodeownersFilePathSuffix,
DefaultIgnoredPathPrefixes, useRegexMatcher: false),
(AzureSdkForNetTargetDirPathSuffix, AzureSdkForNetCodeownersPathSuffix,
(AzureSdkForNetTargetDirPathSuffix, CodeownersFilePathSuffix,
DefaultIgnoredPathPrefixes, useRegexMatcher: true)
},
outputFilePrefix: "azure-sdk-for-net");
}

[Test]
public void WriteToFileWildcardRemovalDiffForAzureSdkForPython()
{
string codeownersCopyPathSuffix = CreateCodeownersCopyWithPathDeletion(
AzureSdkForPythonTargetDirPathSuffix,
CodeownersFilePathSuffix,
pathsToDelete: new[] {"/**/tests.yml", "/**/ci.yml"});

WriteToFileOwnersDiff(
new[]
{
(AzureSdkForPythonTargetDirPathSuffix, CodeownersFilePathSuffix,
DefaultIgnoredPathPrefixes, useRegexMatcher: true),
(AzureSdkForPythonTargetDirPathSuffix, codeownersCopyPathSuffix,
DefaultIgnoredPathPrefixes, useRegexMatcher: true)
},
outputFilePrefix: "azure-sdk-for-python");
}

#endregion

#region Owners data
Expand All @@ -98,11 +122,20 @@ public void WriteToFileRegexMatcherCodeownersForAzureSdkTools()
public void WriteToFileRegexMatcherCodeownersForAzureSdkForNet()
=> WriteToFileOwnersData(
AzureSdkForNetTargetDirPathSuffix,
AzureSdkForNetCodeownersPathSuffix,
CodeownersFilePathSuffix,
DefaultIgnoredPathPrefixes,
useRegexMatcher: true,
outputFilePrefix: "azure-sdk-for-net");

[Test]
public void WriteToFileRegexMatcherCodeownersForAzureSdkForPython()
=> WriteToFileOwnersData(
AzureSdkForPythonTargetDirPathSuffix,
CodeownersFilePathSuffix,
DefaultIgnoredPathPrefixes,
useRegexMatcher: true,
outputFilePrefix: "azure-sdk-for-python");

#endregion

private static void WriteToFileOwnersData(
Expand All @@ -113,6 +146,8 @@ private static void WriteToFileOwnersData(
string outputFilePrefix)
{
var stopwatch = Stopwatch.StartNew();
string rootDir = PathNavigatingToRootDir(CurrentDir);
string targetDir = rootDir + targetDirPathSuffix;

Dictionary<string, CodeownersEntry> ownersData = RunMain(
targetDirPathSuffix,
Expand All @@ -122,7 +157,7 @@ private static void WriteToFileOwnersData(

List<string> outputLines =
new List<string> { "PATH | PATH EXPRESSION | COMMA-SEPARATED OWNERS" };
foreach (var kvp in ownersData)
foreach (KeyValuePair<string, CodeownersEntry> kvp in ownersData)
{
string path = kvp.Key;
CodeownersEntry entry = kvp.Value;
Expand All @@ -132,13 +167,71 @@ private static void WriteToFileOwnersData(
$"| {string.Join(",", entry.Owners)}");
}

WriteToFileMissingSuffixSlashesForDirPaths(targetDir, codeownersPathSuffix, outputLines);

var outputFilePath = outputFilePrefix + OwnersDataOutputPathSuffix;
File.WriteAllLines(outputFilePath, outputLines);
Console.WriteLine($"DONE writing out owners. " +
$"Output written out to {Path.GetFullPath(outputFilePath)}. " +
$"Time taken: {stopwatch.Elapsed}.");
}

private static void WriteToFileMissingSuffixSlashesForDirPaths(
string targetDir,
string codeownersPathSuffix,
List<string> outputLines)
{
List<CodeownersEntry> entries =
CodeownersFile.GetCodeownersEntriesFromFileOrUrl(targetDir + codeownersPathSuffix);

foreach (CodeownersEntry entry in entries.Where(entry => !entry.PathExpression.EndsWith("/")))
{
if (entry.ContainsWildcard)
{
// We do not support "the path is to file while it should be to directory" validation for paths
// with wildcards yet. To do that, we would first need to resolve the path and see if there exists
// a concrete path that includes the the CODEOWNERS paths supposed-file-name as
// infix dir.
// For example, /a/**/b could match against /a/foo/b/c, meaning
// the path is invalid.
outputLines.Add(
$"{entry.PathExpression} " +
$"| WILDCARD_PATH_NEEDS_MANUAL_EVAL " +
$"| {string.Join(",", entry.Owners)}");
}
else
{
string pathToDir = Path.Combine(
targetDir,
entry.PathExpression.TrimStart('/').Replace('/', Path.DirectorySeparatorChar));

if (Directory.Exists(pathToDir))
outputLines.Add(
$"{entry.PathExpression} " +
$"| INVALID_PATH_SHOULD_HAVE_SUFFIX_SLASH_TO_DENOTE_DIR " +
$"| {string.Join(",", entry.Owners)}");
}
}
}

private string CreateCodeownersCopyWithPathDeletion(
string targetDirPathSuffix,
string codeownersFilePathSuffix,
string[] pathsToDelete)
{
string rootDir = PathNavigatingToRootDir(CurrentDir);
string targetDir = rootDir + targetDirPathSuffix;
string codeownersPath = targetDir + codeownersFilePathSuffix;

var codeownersLines = File.ReadAllLines(codeownersPath);
codeownersLines = codeownersLines
.Where(line => !pathsToDelete.Any(line.Contains)).ToArray();

var codeownersCopyPath = codeownersPath + "-copy";
File.WriteAllLines(codeownersCopyPath, codeownersLines);
return codeownersFilePathSuffix + "-copy";
}

private static void WriteToFileOwnersDiff((
string targetDirPathSuffix,
string codeownersPathSuffix,
Expand Down Expand Up @@ -174,9 +267,8 @@ private static Dictionary<string, CodeownersEntry> RunMain(
string ignoredPathPrefixes,
bool useRegexMatcher)
{
// Current dir, ".", is "/artifacts/bin/Azure.Sdk.Tools.CodeOwnersParser.Tests/Debug/net6.0".
const string currentDir = "/artifacts/bin/Azure.Sdk.Tools.CodeOwnersParser.Tests/Debug/net6.0";
string rootDir = PathNavigatingToRootDir(currentDir);
string rootDir = PathNavigatingToRootDir(CurrentDir);
string targetDir = rootDir + targetDirPathSuffix;

string actualOutput, actualErr;
int returnCode;
Expand All @@ -185,9 +277,9 @@ private static Dictionary<string, CodeownersEntry> RunMain(
// Act
returnCode = Program.Main(
targetPath: "/**",
codeownersFilePathOrUrl: rootDir + codeownersPathSuffixToRootDir,
excludeNonUserAliases: false,
targetDir: rootDir + targetDirPathSuffix,
codeownersFilePathOrUrl: targetDir + codeownersPathSuffixToRootDir,
excludeNonUserAliases: true, // true because of Contacts.GetMatchingCodeownersEntry() calls ExcludeNonUserAliases().
targetDir,
ignoredPathPrefixes,
useRegexMatcher);

Expand Down
3 changes: 2 additions & 1 deletion tools/code-owners-parser/CodeOwnersParser/CodeownersFile.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ public static Dictionary<string, CodeownersEntry> GetMatchingCodeownersEntries(
var codeownersEntries = GetCodeownersEntriesFromFileOrUrl(codeownersFilePathOrUrl);

Dictionary<string, CodeownersEntry> codeownersEntriesByPath = targetPath
.ResolveGlob(targetDir, ignoredPathPrefixes).ToDictionary(
.ResolveGlob(targetDir, ignoredPathPrefixes)
.ToDictionary(
path => path,
path => GetMatchingCodeownersEntry(
path,
Expand Down

0 comments on commit b3668aa

Please sign in to comment.