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

Make improvements to retrieve-codeowners tool: change accepted params (targetPath etc.) + refactor #5104

Merged
merged 1 commit into from
Jan 12, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
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 @@ -143,33 +143,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)
public void TestGetMatchingCodeownersEntry(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);
VerifyGetMatchingCodeownersEntry(testCase, codeownersEntries, useNewImpl: false, testCase.ExpectedLegacyMatch);
VerifyGetMatchingCodeownersEntry(testCase, codeownersEntries, useNewImpl: true, testCase.ExpectedNewMatch);
}

private static void VerifyFindOwnersForClosestMatch(
private static void VerifyGetMatchingCodeownersEntry(
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

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
using System.Collections.Generic;
Copy link
Contributor Author

@konrad-jamrozik konrad-jamrozik Jan 11, 2023

Choose a reason for hiding this comment

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

FYI this file is renamed from MainTests.

using System.Text.Json;
using Azure.Sdk.Tools.CodeOwnersParser;
using NUnit.Framework;

namespace Azure.Sdk.Tools.RetrieveCodeOwners.Tests
{
/// <summary>
/// Test class for Azure.Sdk.Tools.RetrieveCodeOwners.Program.
/// </summary>
[TestFixture]
public class ProgramTests
{
private const string CodeownersFilePath = "CODEOWNERS";

private static readonly object[] sourceLists =
{
new object[] {"sdk", false, new List<string> { "person1", "person2" } },
new object[] { "/sdk", false, new List<string> { "person1", "person2" } },
new object[] { "sdk/noPath", false, new List<string> { "person1", "person2" } },
new object[] { "/sdk/azconfig", false, new List<string> { "person3", "person4" } },
new object[] { "/sdk/azconfig/package", false, new List<string> { "person3", "person4" } },
new object[] { "/sdk/testUser/", true, new List<string> { "azure-sdk" } },
new object[] { "/sd", true, new List<string>() }
};

[TestCaseSource(nameof(sourceLists))]
public void TestOnNormalOutput(string targetPath, bool excludeNonUserAliases, List<string> expectedOwners)
{
using var consoleOutput = new ConsoleOutput();

// Act
Program.Main(targetPath, CodeownersFilePath, excludeNonUserAliases);

string actualOutput = consoleOutput.GetOutput();
AssertOwners(actualOutput, expectedOwners);
}

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

private static void AssertOwners(string actualOutput, List<string> expectedOwners)
{
CodeownersEntry? actualEntry = JsonSerializer.Deserialize<CodeownersEntry>(actualOutput);
List<string> actualOwners = actualEntry!.Owners;
Assert.That(actualOwners, Has.Count.EqualTo(expectedOwners.Count));
for (int i = 0; i < actualOwners.Count; i++)
{
Assert.That(actualOwners[i], Is.EqualTo(expectedOwners[i]));
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,36 +5,47 @@
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 path or https url codeownersFilePathOrUrl,
/// prints out to stdout owners of the targetPath as determined by the CODEOWNERS data.
/// </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>
/// <returns>Exit code</returns>

/// <param name="targetPath">The path whose owners are to be determined.</param>
/// <param name="codeownersFilePathOrUrl">The https url or path to the CODEOWNERS file.</param>
/// <param name="excludeNonUserAliases">Whether owners that aren't users should be excluded from the
/// returned owners.</param>
/// <returns>
/// On STDOUT: The JSON representation of the matched CodeownersEntry.
/// "new CodeownersEntry()" if no path in the CODEOWNERS data matches.
/// <br/><br/>
/// From the Main method: exit code. 0 if successful, 1 if error.
/// </returns>
public static int Main(
string codeOwnerFilePath,
string targetDirectory,
bool filterOutNonUserAliases = false
)
string targetPath,
Copy link
Member

Choose a reason for hiding this comment

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

Just keep in mind that you will need to update places like https://github.com/Azure/azure-sdk-tools/blob/main/eng/common/scripts/get-codeowners.ps1 when you update to the latest package version.

Copy link
Contributor Author

@konrad-jamrozik konrad-jamrozik Jan 12, 2023

Choose a reason for hiding this comment

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

string codeownersFilePathOrUrl,
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, codeownersFilePathOrUrl);
if (excludeNonUserAliases)
{
codeOwnerEntry.FilterOutNonUserAliases();
codeownersEntry.ExcludeNonUserAliases();
}
var codeOwnerJson = JsonSerializer.Serialize<CodeOwnerEntry>(codeOwnerEntry, new JsonSerializerOptions { WriteIndented = true });
Console.WriteLine(codeOwnerJson);

var 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
109 changes: 0 additions & 109 deletions tools/code-owners-parser/CodeOwnersParser/CodeOwnersFile.cs

This file was deleted.

Loading