From 2aa5315fa482632adeb4eee31e7b22026f128310 Mon Sep 17 00:00:00 2001 From: Ben Broderick Phillips Date: Tue, 9 Sep 2025 16:17:03 -0400 Subject: [PATCH] Simplify readme generator test services and remove noisy test logging --- .../TestOutputHelper.cs} | 7 +- .../Generators/ReadMeGeneratorToolTests.cs | 101 ++++++------------ 2 files changed, 38 insertions(+), 70 deletions(-) rename tools/azsdk-cli/Azure.Sdk.Tools.Cli.Tests/{Mocks/Helpers/MockOutputHelper.cs => TestHelpers/TestOutputHelper.cs} (88%) diff --git a/tools/azsdk-cli/Azure.Sdk.Tools.Cli.Tests/Mocks/Helpers/MockOutputHelper.cs b/tools/azsdk-cli/Azure.Sdk.Tools.Cli.Tests/TestHelpers/TestOutputHelper.cs similarity index 88% rename from tools/azsdk-cli/Azure.Sdk.Tools.Cli.Tests/Mocks/Helpers/MockOutputHelper.cs rename to tools/azsdk-cli/Azure.Sdk.Tools.Cli.Tests/TestHelpers/TestOutputHelper.cs index e067be52abc..22d57bd66b9 100644 --- a/tools/azsdk-cli/Azure.Sdk.Tools.Cli.Tests/Mocks/Helpers/MockOutputHelper.cs +++ b/tools/azsdk-cli/Azure.Sdk.Tools.Cli.Tests/TestHelpers/TestOutputHelper.cs @@ -3,10 +3,11 @@ namespace Azure.Sdk.Tools.Cli.Tests.Mocks.Helpers; /// -/// A really simple mock that just accumulates all the outputs into a List, which you can assert on +/// An IOutputHelper implementation that can be used in test. +/// It accumulates all the outputs into a List, which you can assert on /// after the test is complete. /// -internal class MockOutputHelper : IOutputHelper +internal class TestOutputHelper : IOutputHelper { /// /// All the collected outputs, in the order they were added. @@ -15,7 +16,7 @@ internal class MockOutputHelper : IOutputHelper public IEnumerable<(string Method, object OutputValue)> Outputs => outputs; private readonly List<(string Method, object OutputValue)> outputs; - public MockOutputHelper() + public TestOutputHelper() { outputs = []; } diff --git a/tools/azsdk-cli/Azure.Sdk.Tools.Cli.Tests/Tools/Generators/ReadMeGeneratorToolTests.cs b/tools/azsdk-cli/Azure.Sdk.Tools.Cli.Tests/Tools/Generators/ReadMeGeneratorToolTests.cs index 1fe6eb719da..8e6da318d57 100644 --- a/tools/azsdk-cli/Azure.Sdk.Tools.Cli.Tests/Tools/Generators/ReadMeGeneratorToolTests.cs +++ b/tools/azsdk-cli/Azure.Sdk.Tools.Cli.Tests/Tools/Generators/ReadMeGeneratorToolTests.cs @@ -1,25 +1,40 @@ using System.CommandLine; using System.CommandLine.Parsing; -using Azure.AI.OpenAI; -using Azure.Sdk.Tools.Cli.Helpers; +using Moq; using Azure.Sdk.Tools.Cli.Microagents; -using Azure.Sdk.Tools.Cli.Services; using Azure.Sdk.Tools.Cli.Tests.Mocks.Helpers; using Azure.Sdk.Tools.Cli.Tests.TestHelpers; using Azure.Sdk.Tools.Cli.Tools.Package; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; -using Moq; -using OpenAI.Chat; namespace Azure.Sdk.Tools.Cli.Tests.Tools.Generators { internal class ReadMeGeneratorToolTests { + private ReadMeGeneratorTool tool; + private TestOutputHelper outputHelper; + private Mock? mockMicroAgentService; + + [SetUp] + public void Setup() + { + outputHelper = new TestOutputHelper(); + mockMicroAgentService = new Mock(); + + tool = new ReadMeGeneratorTool( + new TestLogger(), + outputHelper, + mockMicroAgentService.Object + ); + } + [Test] public async Task TestReadmeGeneratorTool() { - var testClients = SetupMocks(); + var readmeContents = "This is a test response for the readme generation."; + mockMicroAgentService?.Setup(svc => svc.RunAgentToCompletion( + It.IsAny>(), It.IsAny()) + ).Returns(() => Task.FromResult(new ReadmeGenerator.ReadmeContents(readmeContents))); + (DirectoryInfo root, string packagePath) = await CreateFakeLanguageRepo(); var readmeOutputPath = Path.GetTempFileName(); @@ -27,7 +42,6 @@ public async Task TestReadmeGeneratorTool() try { - var tool = ActivatorUtilities.CreateInstance(testClients.ServiceProvider); var command = tool.GetCommand(); int exitCode = command.Invoke($"--output-path \"{readmeOutputPath}\" --service-url \"https://learn.microsoft.com/azure/service-bus-messaging\" --template-path {readmeTemplatePath} --package-path {packagePath}"); @@ -35,8 +49,8 @@ public async Task TestReadmeGeneratorTool() Assert.Multiple(() => { Assert.That(exitCode, Is.EqualTo(0), "Command should execute successfully"); - Assert.That(testClients.OutputHelper.Outputs.First().Method, Is.EqualTo("Output")); - Assert.That(testClients.OutputHelper.Outputs.First().OutputValue, Is.EqualTo($"Readme written to {readmeOutputPath}")); + Assert.That(outputHelper.Outputs.First().Method, Is.EqualTo("Output")); + Assert.That(outputHelper.Outputs.First().OutputValue, Is.EqualTo($"Readme written to {readmeOutputPath}")); }); Assert.That(File.Exists(readmeOutputPath), Is.True, "Readme output file should be created"); @@ -64,9 +78,6 @@ public void TestReadmeGeneratorToolLive() Assert.Ignore("Skipping test as AZURE_SDK_FOR_GO_PATH is not set"); } - var (sp, OutputHelperMock) = CreateServiceProvider(); - - var tool = ActivatorUtilities.CreateInstance(sp); var command = tool.GetCommand(); var readmeOutputPath = Path.GetTempFileName(); var readmeTemplatePath = Path.Combine(AppContext.BaseDirectory, "TestAssets", "README-template.go.md"); @@ -78,13 +89,13 @@ public void TestReadmeGeneratorToolLive() Assert.Multiple(() => { - Assert.That(OutputHelperMock.Outputs.First().Method, Is.EqualTo("Output")); - Assert.That(OutputHelperMock.Outputs.First().OutputValue, Is.EqualTo($"Readme written to {readmeOutputPath}")); + Assert.That(outputHelper.Outputs.First().Method, Is.EqualTo("Output")); + Assert.That(outputHelper.Outputs.First().OutputValue, Is.EqualTo($"Readme written to {readmeOutputPath}")); }); } /// - /// These tests all correspond to "LLM needs to do more work" type of returns from the tool, like removing + /// These tests all correspond to "LLM needs to do more work" type of returns from the tool, like removing /// hardcoded locales, like 'en-us', or having it regenerate if some of the templates tokens make it through /// and weren't replaced. /// @@ -100,7 +111,10 @@ public void TestReadmeGeneratorToolLive() "The readme contains placeholders ((package path)) that should be removed and replaced with a proper package name")] public async Task TestBadReadmeContent(string readmeContent, string expectedFeedback) { - var testClients = SetupMocks(readmeContent); + mockMicroAgentService?.Setup(svc => svc.RunAgentToCompletion( + It.IsAny>(), It.IsAny()) + ).Returns(() => Task.FromResult(new ReadmeGenerator.ReadmeContents(readmeContent))); + (DirectoryInfo root, string packagePath) = await CreateFakeLanguageRepo(); var readmeOutputPath = Path.GetTempFileName(); @@ -108,7 +122,6 @@ public async Task TestBadReadmeContent(string readmeContent, string expectedFeed try { - var tool = ActivatorUtilities.CreateInstance(testClients.ServiceProvider); var command = tool.GetCommand(); int exitCode = command.Invoke($"--output-path \"{readmeOutputPath}\" --service-url \"https://learn.microsoft.com/azure/service-bus-messaging\" --template-path {readmeTemplatePath} --package-path {packagePath}"); @@ -116,8 +129,8 @@ public async Task TestBadReadmeContent(string readmeContent, string expectedFeed Assert.Multiple(() => { Assert.That(exitCode, Is.EqualTo(1), "Command should fail, as the final readme doesn't pass validation"); - Assert.That(testClients.OutputHelper.Outputs.First().Method, Is.EqualTo("OutputError")); - Assert.That(testClients.OutputHelper.Outputs.First().OutputValue, Is.EqualTo($"ReadmeGenerator failed with validation errors: {expectedFeedback}")); + Assert.That(outputHelper.Outputs.First().Method, Is.EqualTo("OutputError")); + Assert.That(outputHelper.Outputs.First().OutputValue, Is.EqualTo($"ReadmeGenerator failed with validation errors: {expectedFeedback}")); }); Assert.That(File.Exists(readmeOutputPath), Is.True, "Readme output file should be created"); @@ -128,45 +141,6 @@ public async Task TestBadReadmeContent(string readmeContent, string expectedFeed } } - /// - /// Creates a service provider in the same way as the normal program. Can be used to instantiate real clients. - /// - /// Optional Action you can use to register your own client instances - /// - private static (ServiceProvider, MockOutputHelper) CreateServiceProvider(Action? customizeServices = default) - { - var serviceCollection = new ServiceCollection(); - var OutputHelper = new MockOutputHelper(); - - serviceCollection.AddLogging(); // so our ILogger's will be available. - - ServiceRegistrations.RegisterCommonServices(serviceCollection); - serviceCollection.AddSingleton(OutputHelper); - - customizeServices?.Invoke(serviceCollection); - - var sp = serviceCollection.BuildServiceProvider(); - return (sp, OutputHelper); - } - - private static TestClients SetupMocks(string readmeContents = "This is a test response for the readme generation.") - { - var (openAIClientMock, chatClientMock) = OpenAIMockHelper.Create("gpt-4.1"); - - var serviceMock = new Mock(); - serviceMock.Setup(svc => svc.RunAgentToCompletion( - It.IsAny>(), It.IsAny()) - ).Returns(() => Task.FromResult(new ReadmeGenerator.ReadmeContents(readmeContents))); - - var (serviceProvider, OutputHelper) = CreateServiceProvider((sc) => - { - sc.AddLogging((lb) => lb.AddConsole()); - sc.AddSingleton(serviceMock.Object); - }); - - return new TestClients(openAIClientMock, chatClientMock, serviceProvider, OutputHelper); - } - private static async Task<(DirectoryInfo root, string packagePath)> CreateFakeLanguageRepo() { var root = Directory.CreateTempSubdirectory("readme-generator-tool-test"); @@ -179,12 +153,5 @@ private static (ServiceProvider, MockOutputHelper) CreateServiceProvider(Action< await File.WriteAllTextAsync(Path.Join(scriptsDir, "Verify-Links.ps1"), "Write-Host 'passed'"); return (root, packagePath); } - - private record TestClients( - Mock OpenAIClient, - Mock ChatClient, - ServiceProvider ServiceProvider, - MockOutputHelper OutputHelper - ); } }