Skip to content

Commit 648ad6c

Browse files
benbpbenbp
committed
Use logger factory instead of console for codeowners library logging
1 parent ec499b9 commit 648ad6c

10 files changed

Lines changed: 91 additions & 44 deletions

File tree

‎tools/azsdk-cli/Azure.Sdk.Tools.Cli.Tests/Tools/CodeownersToolsTests.cs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,11 @@ public void Setup()
3131
_mockLogger = new Mock<ILogger<CodeownersTools>>();
3232
_mockCodeownersValidator = new Mock<ICodeownersValidatorHelper>();
3333

34-
3534
_tool = new CodeownersTools(
3635
_mockGithub,
3736
_mockOutput.Object,
3837
_mockLogger.Object,
38+
null,
3939
_mockCodeownersValidator.Object);
4040
}
4141

@@ -63,7 +63,7 @@ public async Task Update_LabelMissing_NotInReview_NoPath_Throws()
6363
gh.Setup(s => s.GetContentsSingleAsync(Constants.AZURE_OWNER_PATH, "repo", Constants.AZURE_CODEOWNERS_PATH, It.IsAny<string>()))
6464
.ReturnsAsync(new RepositoryContent("CODEOWNERS", ".github/CODEOWNERS", "shaCode", 0, ContentType.File, null, null, null, null, "utf-8", Convert.ToBase64String(System.Text.Encoding.UTF8.GetBytes("")), null, null));
6565

66-
var tool = new CodeownersTools(gh.Object, _mockOutput.Object, _mockLogger.Object, _mockCodeownersValidator.Object);
66+
var tool = new CodeownersTools(gh.Object, _mockOutput.Object, _mockLogger.Object, null, _mockCodeownersValidator.Object);
6767

6868
var result = await tool.UpdateCodeowners("repo", false, "", "NonExistService", new List<string>(), new List<string>(), true);
6969

@@ -92,7 +92,7 @@ public async Task Update_AddOwnersToExistingEntry_Success()
9292
var validator = new Mock<ICodeownersValidatorHelper>();
9393
validator.Setup(v => v.ValidateCodeOwnerAsync(It.IsAny<string>(), It.IsAny<bool>())).ReturnsAsync(new CodeownersValidationResult { Username = "user", IsValidCodeOwner = true });
9494

95-
var tool = new CodeownersTools(gh.Object, _mockOutput.Object, _mockLogger.Object, validator.Object);
95+
var tool = new CodeownersTools(gh.Object, _mockOutput.Object, _mockLogger.Object, null, validator.Object);
9696
var result = await tool.UpdateCodeowners("repoName", false, "/sdk/myservice/", "MyService", new List<string> { "@oldowner", "@newowner" }, new List<string>() { "@newowner", "@newowner2" }, true);
9797
Assert.IsNotNull(result);
9898
Assert.IsTrue(result.Contains("URL:") || result.Contains("Created"));
@@ -119,7 +119,7 @@ public async Task Update_RemoveOwnersFromExistingEntry_Success()
119119
var validator = new Mock<ICodeownersValidatorHelper>();
120120
validator.Setup(v => v.ValidateCodeOwnerAsync(It.IsAny<string>(), It.IsAny<bool>())).ReturnsAsync(new CodeownersValidationResult { Username = "user", IsValidCodeOwner = true });
121121

122-
var tool = new CodeownersTools(gh.Object, _mockOutput.Object, _mockLogger.Object, validator.Object);
122+
var tool = new CodeownersTools(gh.Object, _mockOutput.Object, _mockLogger.Object, null, validator.Object);
123123
// Remove @removeowner, keep @oldowner
124124
var result = await tool.UpdateCodeowners("repoName", false, "/sdk/myservice/", "MyService", new List<string> { "@oldowner" }, new List<string>(), false);
125125
Assert.IsNotNull(result);
@@ -160,7 +160,7 @@ public async Task Update_CreateNewEntry_PRSuccess()
160160
validator.Setup(v => v.ValidateCodeOwnerAsync(It.IsAny<string>(), It.IsAny<bool>()))
161161
.ReturnsAsync(new CodeownersValidationResult { Username = "user", IsValidCodeOwner = true });
162162

163-
var tool = new CodeownersTools(gh.Object, _mockOutput.Object, _mockLogger.Object, validator.Object);
163+
var tool = new CodeownersTools(gh.Object, _mockOutput.Object, _mockLogger.Object, null, validator.Object);
164164

165165
var result = await tool.UpdateCodeowners("repoName", false, "/sdk/newsvc/", "NewSvc", new List<string> { "@a", "@b" }, new List<string> { "@s1", "@s2" }, true);
166166
Assert.IsNotNull(result);
@@ -196,7 +196,7 @@ public async Task Update_CreateNewEntry_PRFailure_ReturnsPRFailureMessage()
196196
validator.Setup(v => v.ValidateCodeOwnerAsync(It.IsAny<string>(), It.IsAny<bool>()))
197197
.ReturnsAsync(new CodeownersValidationResult { Username = "user", IsValidCodeOwner = true });
198198

199-
var tool = new CodeownersTools(gh.Object, _mockOutput.Object, _mockLogger.Object, validator.Object);
199+
var tool = new CodeownersTools(gh.Object, _mockOutput.Object, _mockLogger.Object, null, validator.Object);
200200

201201
var result = await tool.UpdateCodeowners("repoName", false, "/sdk/newsvc/", "NewSvc", new List<string> { "@a", "@b" }, new List<string> { "@s1", "@s2" }, true);
202202
Assert.IsNotNull(result);
@@ -239,7 +239,7 @@ public async Task Validate_CODEOWNERS_Missing_ReturnsError()
239239
var output = new Mock<IOutputHelper>();
240240
var validator = new Mock<ICodeownersValidatorHelper>();
241241

242-
var tool = new CodeownersTools(gh.Object, output.Object, _mockLogger.Object, validator.Object);
242+
var tool = new CodeownersTools(gh.Object, output.Object, _mockLogger.Object, null, validator.Object);
243243

244244
var result = await tool.ValidateCodeownersEntryForService("test-repo", "Any", null);
245245
Assert.IsNotNull(result);

‎tools/azsdk-cli/Azure.Sdk.Tools.Cli/Tools/EngSys/CodeownersTools/CodeownersTools.cs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@
99
using Azure.Sdk.Tools.Cli.Helpers;
1010
using Azure.Sdk.Tools.Cli.Services;
1111
using Azure.Sdk.Tools.Cli.Contract;
12-
using Azure.Sdk.Tools.CodeownersUtils.Parsing;
1312
using Azure.Sdk.Tools.CodeownersUtils.Editing;
13+
using Azure.Sdk.Tools.CodeownersUtils.Parsing;
1414
using Azure.Sdk.Tools.Cli.Configuration;
1515
using Azure.Sdk.Tools.Cli.Models.Responses;
1616
using Azure.Sdk.Tools.Cli.Commands;
@@ -47,13 +47,16 @@ public CodeownersTools(
4747
IGitHubService githubService,
4848
IOutputHelper output,
4949
ILogger<CodeownersTools> logger,
50+
ILoggerFactory? loggerFactory,
5051
ICodeownersValidatorHelper codeownersValidator) : base()
5152
{
5253
this.githubService = githubService;
5354
this.output = output;
5455
this.logger = logger;
5556
this.codeownersValidator = codeownersValidator;
5657

58+
CodeownersUtils.Utils.Log.Configure(loggerFactory);
59+
5760
CommandHierarchy =
5861
[
5962
SharedCommandGroups.EngSys

‎tools/codeowners-utils/Azure.Sdk.Tools.CodeownersLinter.Tests/TestHelpers.cs‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using Azure.Sdk.Tools.CodeownersUtils.Caches;
99
using Azure.Sdk.Tools.CodeownersUtils.Parsing;
1010
using Azure.Sdk.Tools.CodeownersUtils.Utils;
11+
using Microsoft.Extensions.Logging;
1112

1213
namespace Azure.Sdk.Tools.CodeownersUtils.Tests
1314
{
@@ -26,7 +27,7 @@ public static class TestHelpers
2627
/// The user/org visibility data will consist of 5 users, TestOwner0..TestOwner4, with only even users
2728
/// being visible.
2829
/// The team/user data will consist of 5 teams with the number of users in each team equal to the
29-
/// team number, TestTeam0...TestTeam4.The users in each team will consist of the same users in
30+
/// team number, TestTeam0...TestTeam4.The users in each team will consist of the same users in
3031
/// the user/org data.
3132
/// </summary>
3233
/// <returns>Populated OwnerDataUtils</returns>
@@ -38,7 +39,7 @@ public static OwnerDataUtils SetupOwnerData()
3839
Dictionary<string, List<string>> teamUserDict = new Dictionary<string, List<string>>(StringComparer.InvariantCultureIgnoreCase);
3940

4041
// Create 5 teams
41-
for (int i = 0;i < _maxItems; i++)
42+
for (int i = 0; i < _maxItems; i++)
4243
{
4344
string team = $"{TestTeamNamePartial}{i}";
4445
List<string> users = new List<string>();
@@ -58,7 +59,7 @@ public static OwnerDataUtils SetupOwnerData()
5859
UserOrgVisibilityCache userOrgVisibilityCache = new UserOrgVisibilityCache(null);
5960
Dictionary<string, bool> userOrgVisDict = new Dictionary<string, bool>(StringComparer.InvariantCultureIgnoreCase);
6061
// Make every even user visible
61-
for (int i = 0;i < 5;i++)
62+
for (int i = 0; i < 5; i++)
6263
{
6364
string user = $"{TestOwnerNamePartial}{i}";
6465
if (i % 2 == 0)
@@ -82,7 +83,7 @@ public static RepoLabelDataUtils SetupRepoLabelData()
8283
{
8384
Dictionary<string, HashSet<string>> repoLabelDict = new Dictionary<string, HashSet<string>>(StringComparer.InvariantCultureIgnoreCase);
8485
HashSet<string> repoLabels = new HashSet<string>(StringComparer.InvariantCultureIgnoreCase);
85-
for (int i = 0;i < 5;i++)
86+
for (int i = 0; i < 5; i++)
8687
{
8788
string label = $"{TestLabelNamePartial}{i}";
8889
repoLabels.Add(label);
@@ -112,7 +113,7 @@ public static bool StringListsAreEqual(List<string> actuaList, List<string> expe
112113
/// <param name="actualCodeownerEntries">The actual, or parsed, list of Codeowners entries.</param>
113114
/// <param name="expectedCodeownerEntries">The expected list of Codeowners entries</param>
114115
/// <returns>True if they're equal, false otherwise</returns>
115-
public static bool CodeownersEntryListsAreEqual(List<CodeownersEntry> actualCodeownerEntries,
116+
public static bool CodeownersEntryListsAreEqual(List<CodeownersEntry> actualCodeownerEntries,
116117
List<CodeownersEntry> expectedCodeownerEntries)
117118
{
118119
// SequenceEqual determines whether two sequences are equal by comparing the length and
@@ -190,8 +191,8 @@ public static void TempGetSerializedString(List<CodeownersEntry> codeownersEntri
190191
WriteIndented = true
191192
};
192193
string jsonString = JsonSerializer.Serialize(codeownersEntries, jsonSerializerOptions);
193-
Console.WriteLine(jsonString);
194-
Console.WriteLine("something to break on");
194+
Log.Logger.LogInformation("{Json}", jsonString);
195+
Log.Logger.LogInformation("something to break on");
195196
}
196197
}
197198
}

‎tools/codeowners-utils/Azure.Sdk.Tools.CodeownersLinter.Tests/Utils/BaselineUtilsTests.cs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
using Azure.Sdk.Tools.CodeownersUtils.Utils;
1010
using Azure.Sdk.Tools.CodeownersUtils.Verification;
1111
using NUnit.Framework;
12+
using Microsoft.Extensions.Logging;
1213

1314
namespace Azure.Sdk.Tools.CodeownersUtils.Tests.Utils
1415
{
@@ -43,7 +44,7 @@ public void CleanupTestData()
4344
// Cleanup any temporary files created by the tests
4445
if (_tempFiles.Count > 0)
4546
{
46-
foreach(string tempFile in _tempFiles)
47+
foreach (string tempFile in _tempFiles)
4748
{
4849
int tryNumber = 0;
4950
while (tryNumber < maxTries)
@@ -55,7 +56,7 @@ public void CleanupTestData()
5556
}
5657
catch (Exception ex)
5758
{
58-
Console.WriteLine($"Exception trying to delete {tempFile}. Ex={ex}");
59+
Log.Logger.LogError(ex, "Exception trying to delete {TempFile}.", tempFile);
5960
// sleep 100ms and try again
6061
System.Threading.Thread.Sleep(100);
6162
}
@@ -64,7 +65,7 @@ public void CleanupTestData()
6465
// Report that the test wasn't able to delete the file after successive tries
6566
if (tryNumber == maxTries)
6667
{
67-
Console.WriteLine($"Unable to delete {tempFile} after {maxTries}, see above for exception details.");
68+
Log.Logger.LogWarning("Unable to delete {TempFile} after {MaxTries}, see above for exception details.", tempFile, maxTries);
6869
}
6970
}
7071
}
@@ -89,7 +90,7 @@ public void TestBaselineGenerationAndFiltering(string testCodeownerFile,
8990
// Load the expected baseline file and, for sanity's sake, ensure that all errors are filtered
9091
BaselineUtils expectedBaselineUtils = new BaselineUtils(expectedBaselineFile);
9192
var filteredWithExpected = expectedBaselineUtils.FilterErrorsUsingBaseline(actualErrors);
92-
93+
9394
// This piece is for sanity, to verify that the the filter only filters out SingleLineErrors and leaves
9495
// the BlockFormattingErrors.
9596
if (expectedNumberOfBlockErrors > 0)

‎tools/codeowners-utils/Azure.Sdk.Tools.CodeownersLinter/Azure.Sdk.Tools.CodeownersLinter.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
<ItemGroup>
1212
<PackageReference Include="System.CommandLine" Version="2.0.0-beta4.22272.1" />
13+
<PackageReference Include="Microsoft.Extensions.Logging.Console" Version="8.0.0" />
1314
</ItemGroup>
1415

1516
<ItemGroup>

‎tools/codeowners-utils/Azure.Sdk.Tools.CodeownersLinter/Program.cs‎

Lines changed: 28 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
using System.IO;
1010
using System.Collections.Generic;
1111
using System.Linq;
12+
using Microsoft.Extensions.Logging;
1213

1314
namespace Azure.Sdk.Tools.CodeownersLinter
1415
{
@@ -20,6 +21,14 @@ static void Main(string[] args)
2021
Stopwatch stopWatch = new Stopwatch();
2122
stopWatch.Start();
2223

24+
// Configure logging for the application and pass the logger to the library helper.
25+
var loggerFactory = LoggerFactory.Create(builder =>
26+
{
27+
builder.AddSimpleConsole();
28+
builder.SetMinimumLevel(LogLevel.Information);
29+
});
30+
Log.Configure(loggerFactory.CreateLogger(typeof(Program).Namespace));
31+
2332
// The storage URIs are in the azure-sdk-write-teams-container-blobs pipeline variable group.
2433
var teamUserBlobStorageUriOption = new Option<string>
2534
(name: "--teamUserBlobStorageURI",
@@ -123,17 +132,17 @@ static void Main(string[] args)
123132
string elapsedTime = String.Format("{0:00}:{1:00}:{2:00}.{3:00}",
124133
ts.Hours, ts.Minutes, ts.Seconds,
125134
ts.Milliseconds / 10);
126-
Console.WriteLine($"Total run time: {elapsedTime}");
135+
Log.Logger.LogInformation("Total run time: {ElapsedTime}", elapsedTime);
127136

128-
Console.WriteLine($"Exiting with return code {returnCode}");
137+
Log.Logger.LogInformation("Exiting with return code {ReturnCode}", returnCode);
129138
Environment.Exit(returnCode);
130139
}
131140

132141
/// <summary>
133-
/// Verify the arguments and call to process the CODEOWNERS file. If errors are being filtered with a
142+
/// Verify the arguments and call to process the CODEOWNERS file. If errors are being filtered with a
134143
/// baseline, or used to regenerate the baseline, that's done in here. Note that filtering errors and
135144
/// regenerating the baseline cannot both be done in the same run.
136-
///
145+
///
137146
/// The baseBranchBaselineFile
138147
/// This file will be primarily used in PR validation where two calls will be made. be made. The first
139148
/// call will use the -gbl option and generate the secondary file to a different location. It can't use
@@ -156,13 +165,13 @@ static void Main(string[] args)
156165
/// <param name="baseBranchBaselineFile">The name of the base branch baseline file to generate or use.</param>
157166
/// <returns>integer, used to set the return code</returns>
158167
/// <exception cref="ArgumentException">Thrown if any arguments, or argument combinations, are invalid.</exception>
159-
static int LintCodeownersFile(string teamUserBlobStorageUri,
160-
string userOrgVisibilityBlobStorageUri,
161-
string repoLabelBlobStorageUri,
162-
string repoRoot,
168+
static int LintCodeownersFile(string teamUserBlobStorageUri,
169+
string userOrgVisibilityBlobStorageUri,
170+
string repoLabelBlobStorageUri,
171+
string repoRoot,
163172
string repoName,
164-
bool filterBaselineErrors,
165-
bool generateBaseline,
173+
bool filterBaselineErrors,
174+
bool generateBaseline,
166175
string baseBranchBaselineFile)
167176
{
168177
// Don't allow someone to create and use a baseline in the same run
@@ -213,15 +222,15 @@ static int LintCodeownersFile(string teamUserBlobStorageUri,
213222
}
214223
else
215224
{
216-
Console.WriteLine($"The CODEOWNERS baseline error file, {codeownersBaselineFile}, file for {repoName} does not exist. No filtering will be done for errors.");
225+
Log.Logger.LogWarning("The CODEOWNERS baseline error file, {BaselineFile}, file for {RepoName} does not exist. No filtering will be done for errors.", codeownersBaselineFile, repoName);
217226
}
218227
}
219228

220229
DirectoryUtils directoryUtils = new DirectoryUtils(repoRoot);
221230
OwnerDataUtils ownerData = new OwnerDataUtils(teamUserBlobStorageUri, userOrgVisibilityBlobStorageUri);
222231

223-
var errors = CodeownersUtils.Verification.CodeownersLinter.LintCodeownersFile(directoryUtils,
224-
ownerData,
232+
var errors = CodeownersUtils.Verification.CodeownersLinter.LintCodeownersFile(directoryUtils,
233+
ownerData,
225234
repoLabelData,
226235
codeownersFileFullPath);
227236

@@ -251,7 +260,7 @@ static int LintCodeownersFile(string teamUserBlobStorageUri,
251260
{
252261
if (errors.Count == 0)
253262
{
254-
Console.WriteLine($"##vso[task.LogIssue type=warning;]There were no CODEOWNERS parsing errors but there is a baseline file {codeownersBaselineFile} for filtering. If the file is empty, or all errors have been fixed, then it should be deleted.");
263+
Log.Logger.LogWarning("##vso[task.LogIssue type=warning;]There were no CODEOWNERS parsing errors but there is a baseline file {BaselineFile} for filtering. If the file is empty, or all errors have been fixed, then it should be deleted.", codeownersBaselineFile);
255264
}
256265
else
257266
{
@@ -279,11 +288,11 @@ static int LintCodeownersFile(string teamUserBlobStorageUri,
279288
// DevOps only adds the first 4 errors to the github checks list so lets always add the generic one first and then as many of the individual ones as can be found afterwards
280289
if (loggingInDevOps)
281290
{
282-
Console.WriteLine($"##vso[task.logissue type=error;]There are linter errors. Please visit {linterErrorsHelpLink} for guidance on how to handle them.");
291+
Log.Logger.LogError("##vso[task.logissue type=error;]There are linter errors. Please visit {HelpLink} for guidance on how to handle them.", linterErrorsHelpLink);
283292
}
284293
else
285294
{
286-
Console.WriteLine($"There are linter errors. Please visit {linterErrorsHelpLink} for guidance on how to handle them.");
295+
Log.Logger.LogError("There are linter errors. Please visit {HelpLink} for guidance on how to handle them.", linterErrorsHelpLink);
287296
}
288297

289298
// Output the errors sorted ascending by line number and by type. If there's a block
@@ -296,11 +305,11 @@ static int LintCodeownersFile(string teamUserBlobStorageUri,
296305
if (loggingInDevOps)
297306
{
298307
// Environment.NewLine needs to be replaced by an encoded NewLine "%0D%0A" in order to display on GitHub and DevOps checks
299-
Console.WriteLine($"##vso[task.logissue type=error;sourcepath={codeownersFileFullPath};linenumber={error.LineNumber};columnnumber=1;]{error.ToString().Replace(Environment.NewLine,"%0D%0A")}");
308+
Log.Logger.LogError("##vso[task.logissue type=error;sourcepath={SourcePath};linenumber={LineNumber};columnnumber=1;]{Error}", codeownersFileFullPath, error.LineNumber, error.ToString().Replace(Environment.NewLine, "%0D%0A"));
300309
}
301-
else
310+
else
302311
{
303-
Console.WriteLine(error + Environment.NewLine);
312+
Log.Logger.LogError("{Error}{NewLine}", error, Environment.NewLine);
304313
}
305314
}
306315
}

0 commit comments

Comments
 (0)