Adding Multiple Skill Directories Simplification + Integration Tests …

…readme changes (#599) ### Motivation and Context - Improvement It is a common scenario having to import multiple skills. Adding this as a `param` array to the extension will reduce number of calls and improve cleanliness. ### Description As part of this PR I also spotted missing in the Readme the recommendation on using UserSecrets for Integration Tests. Changed the readme and example config files to use davinci-003 models as our current standard for integration tests.
microsoft · Apr 26, 2023 · 5b8a269 · 5b8a269
1 parent 15dc234
commit 5b8a269
Show file tree

Hide file tree

Showing 10 changed files with 145 additions and 102 deletions.
diff --git a/dotnet/src/IntegrationTests/Connectors/OpenAI/OpenAICompletionTests.cs b/dotnet/src/IntegrationTests/Connectors/OpenAI/OpenAICompletionTests.cs
@@ -47,7 +47,7 @@ public async Task OpenAITestAsync(string prompt, string expectedAnswerContains)
 
         this.ConfigureOpenAI(target);
 
-        IDictionary<string, ISKFunction> skill = TestHelpers.GetSkill("ChatSkill", target);
+        IDictionary<string, ISKFunction> skill = TestHelpers.GetSkills(target, "ChatSkill");
 
         // Act
         SKContext actual = await target.RunAsync(prompt, skill["Chat"]);
@@ -65,7 +65,7 @@ public async Task AzureOpenAITestAsync(string prompt, string expectedAnswerConta
 
         this.ConfigureAzureOpenAI(target);
 
-        IDictionary<string, ISKFunction> skill = TestHelpers.GetSkill("ChatSkill", target);
+        IDictionary<string, ISKFunction> skill = TestHelpers.GetSkills(target, "ChatSkill");
 
         // Act
         SKContext actual = await target.RunAsync(prompt, skill["Chat"]);
@@ -95,7 +95,7 @@ public async Task OpenAIHttpRetryPolicyTestAsync(string prompt, string expectedO
             modelId: openAIConfiguration.ModelId,
             apiKey: "INVALID_KEY");
 
-        IDictionary<string, ISKFunction> skill = TestHelpers.GetSkill("SummarizeSkill", target);
+        IDictionary<string, ISKFunction> skill = TestHelpers.GetSkills(target, "SummarizeSkill");
 
         // Act
         var context = await target.RunAsync(prompt, skill["Summarize"]);
@@ -119,7 +119,7 @@ public async Task OpenAIHttpInvalidKeyShouldReturnErrorDetailAsync()
             modelId: openAIConfiguration.ModelId,
             apiKey: "INVALID_KEY");
 
-        IDictionary<string, ISKFunction> skill = TestHelpers.GetSkill("SummarizeSkill", target);
+        IDictionary<string, ISKFunction> skill = TestHelpers.GetSkills(target, "SummarizeSkill");
 
         // Act
         var context = await target.RunAsync("Any", skill["Summarize"]);
@@ -146,7 +146,7 @@ public async Task AzureOpenAIHttpInvalidKeyShouldReturnErrorDetailAsync()
             endpoint: azureOpenAIConfiguration.Endpoint,
             apiKey: "INVALID_KEY");
 
-        IDictionary<string, ISKFunction> skill = TestHelpers.GetSkill("SummarizeSkill", target);
+        IDictionary<string, ISKFunction> skill = TestHelpers.GetSkills(target, "SummarizeSkill");
 
         // Act
         var context = await target.RunAsync("Any", skill["Summarize"]);
@@ -173,7 +173,7 @@ public async Task AzureOpenAIHttpExceededMaxTokensShouldReturnErrorDetailAsync()
             endpoint: azureOpenAIConfiguration.Endpoint,
             apiKey: azureOpenAIConfiguration.ApiKey);
 
-        IDictionary<string, ISKFunction> skill = TestHelpers.GetSkill("SummarizeSkill", target);
+        IDictionary<string, ISKFunction> skill = TestHelpers.GetSkills(target, "SummarizeSkill");
 
         // Act
         var context = await skill["Summarize"].InvokeAsync(string.Join('.', Enumerable.Range(1, 40000)));
@@ -203,7 +203,7 @@ public async Task CompletionWithDifferentLineEndingsAsync(string lineEnding, AIS
 
         this._serviceConfiguration[service](target);
 
-        IDictionary<string, ISKFunction> skill = TestHelpers.GetSkill("ChatSkill", target);
+        IDictionary<string, ISKFunction> skill = TestHelpers.GetSkills(target, "ChatSkill");
 
         // Act
         SKContext actual = await target.RunAsync(prompt, skill["Chat"]);

diff --git a/dotnet/src/IntegrationTests/IntegrationTests.csproj b/dotnet/src/IntegrationTests/IntegrationTests.csproj
@@ -6,6 +6,7 @@
     <TargetFramework>net6.0</TargetFramework>
     <IsPackable>false</IsPackable>
     <NoWarn>CA2007,VSTHRD111</NoWarn>
+    <UserSecretsId>b7762d10-e29b-4bb1-8b74-b6d69a667dd4</UserSecretsId>
   </PropertyGroup>
 
   <ItemGroup>

diff --git a/dotnet/src/IntegrationTests/Orchestration/PlanTests.cs b/dotnet/src/IntegrationTests/Orchestration/PlanTests.cs
@@ -75,7 +75,7 @@ public async Task CanExecuteRunSimpleStepsAsync(string goal, string inputToTrans
         // Arrange
         IKernel target = this.InitializeKernel();
         var emailSkill = target.ImportSkill(new EmailSkillFake());
-        var writerSkill = TestHelpers.GetSkill("WriterSkill", target);
+        var writerSkill = TestHelpers.GetSkills(target, "WriterSkill");
         var expectedBody = $"Sent email to: {expectedEmail}. Body:".Trim();
 
         var plan = new Plan(goal);
@@ -195,8 +195,8 @@ public async Task CanExecuteRunPlanAsync(string goal, string inputToSummarize, s
         // Arrange
         IKernel target = this.InitializeKernel();
 
-        var summarizeSkill = TestHelpers.GetSkill("SummarizeSkill", target);
-        var writerSkill = TestHelpers.GetSkill("WriterSkill", target);
+        var summarizeSkill = TestHelpers.GetSkills(target, "SummarizeSkill");
+        var writerSkill = TestHelpers.GetSkills(target, "WriterSkill");
         var emailSkill = target.ImportSkill(new EmailSkillFake());
 
         var expectedBody = $"Sent email to: {expectedEmail}. Body:".Trim();
@@ -264,8 +264,8 @@ public async Task CanExecuteRunSequentialAsync(string goal, string inputToSummar
     {
         // Arrange
         IKernel target = this.InitializeKernel();
-        var summarizeSkill = TestHelpers.GetSkill("SummarizeSkill", target);
-        var writerSkill = TestHelpers.GetSkill("WriterSkill", target);
+        var summarizeSkill = TestHelpers.GetSkills(target, "SummarizeSkill");
+        var writerSkill = TestHelpers.GetSkills(target, "WriterSkill");
         var emailSkill = target.ImportSkill(new EmailSkillFake());
 
         var expectedBody = $"Sent email to: {expectedEmail}. Body:".Trim();
@@ -319,8 +319,8 @@ public async Task CanExecuteRunSequentialFunctionsAsync(string goal, string inpu
         // Arrange
         IKernel target = this.InitializeKernel();
 
-        var summarizeSkill = TestHelpers.GetSkill("SummarizeSkill", target);
-        var writerSkill = TestHelpers.GetSkill("WriterSkill", target);
+        var summarizeSkill = TestHelpers.GetSkills(target, "SummarizeSkill");
+        var writerSkill = TestHelpers.GetSkills(target, "WriterSkill");
         var emailSkill = target.ImportSkill(new EmailSkillFake());
 
         var expectedBody = $"Sent email to: {expectedEmail}. Body:".Trim();

diff --git a/dotnet/src/IntegrationTests/Planning/SequentialPlanParserTests.cs b/dotnet/src/IntegrationTests/Planning/SequentialPlanParserTests.cs
@@ -42,8 +42,7 @@ public void CanCallToPlanFromXml()
             })
             .Build();
         kernel.ImportSkill(new EmailSkillFake(), "email");
-        var summarizeSkill = TestHelpers.GetSkill("SummarizeSkill", kernel);
-        var writerSkill = TestHelpers.GetSkill("WriterSkill", kernel);
+        TestHelpers.GetSkills(kernel, "SummarizeSkill", "WriterSkill");
 
         var planString =
             @"<plan>

diff --git a/dotnet/src/IntegrationTests/Planning/SequentialPlannerTests.cs b/dotnet/src/IntegrationTests/Planning/SequentialPlannerTests.cs
@@ -37,7 +37,7 @@ public async Task CreatePlanFunctionFlowAsync(string prompt, string expectedFunc
     {
         // Arrange
         IKernel kernel = this.InitializeKernel();
-        TestHelpers.GetSkill("FunSkill", kernel);
+        TestHelpers.GetSkills(kernel, "FunSkill");
 
         var planner = new SequentialPlanner(kernel);
 
@@ -58,7 +58,7 @@ public async Task CreatePlanWithDefaultsAsync(string prompt, string expectedFunc
     {
         // Arrange
         IKernel kernel = this.InitializeKernel();
-        TestHelpers.GetSkill("WriterSkill", kernel);
+        TestHelpers.GetSkills(kernel, "WriterSkill");
 
         var planner = new SequentialPlanner(kernel);
 

diff --git a/dotnet/src/IntegrationTests/README.md b/dotnet/src/IntegrationTests/README.md
@@ -11,6 +11,40 @@
 
 ## Setup
 
+### Option 1: Use Secret Manager
+
+Integration tests will require secrets and credentials, to access OpenAI, Azure OpenAI,
+Bing and other resources. 
+
+We suggest using .NET [Secret Manager](https://learn.microsoft.com/en-us/aspnet/core/security/app-secrets)
+to avoid the risk of leaking secrets into the repository, branches and pull requests.
+You can also use environment variables if you prefer.
+
+To set your secrets with Secret Manager:
+
+```
+cd dotnet/src/IntegrationTests
+
+dotnet user-secrets init
+dotnet user-secrets set "OpenAI:ServiceId" "text-davinci-003"
+dotnet user-secrets set "OpenAI:ModelId" "text-davinci-003"
+dotnet user-secrets set "OpenAI:ApiKey" "..."
+
+dotnet user-secrets set "AzureOpenAI:ServiceId" "azure-text-davinci-003"
+dotnet user-secrets set "AzureOpenAI:DeploymentName" "text-davinci-003"
+dotnet user-secrets set "AzureOpenAI:Endpoint" "https://contoso.openai.azure.com/"
+dotnet user-secrets set "AzureOpenAI:ApiKey" "..."
+
+dotnet user-secrets set "AzureOpenAIEmbeddings:ServiceId" "azure-text-embedding-ada-002"
+dotnet user-secrets set "AzureOpenAIEmbeddings:DeploymentName" "text-embedding-ada-002"
+dotnet user-secrets set "AzureOpenAIEmbeddings:Endpoint" "https://contoso.openai.azure.com/"
+dotnet user-secrets set "AzureOpenAIEmbeddings:ApiKey" "..."
+
+dotnet user-secrets set "HuggingFace:ApiKey" "..."
+dotnet user-secrets set "Bing:ApiKey" "..."
+```
+
+### Option 2: Use Configuration File
 1. Create a `testsettings.development.json` file next to `testsettings.json`. This file will be ignored by git,
    the content will not end up in pull requests, so it's safe for personal settings. Keep the file safe.
 2. Edit `testsettings.development.json` and
@@ -52,28 +86,31 @@ For example:
 }
 ```
 
-3. (Optional) You may also set the test settings in your environment variables. The environment variables will override the settings in the `testsettings.development.json` file. When setting environment variables, use a double underscore (i.e. "\_\_") to delineate between parent and child properties. For example:
-
-    - bash:
-
-      ```bash
-      export OpenAI__ApiKey="sk-...."
-      export AzureOpenAI__ApiKey="...."
-      export AzureOpenAI__DeploymentName="azure-text-davinci-003"
-      export AzureOpenAIEmbeddings__DeploymentName="azure-text-embedding-ada-002"
-      export AzureOpenAI__Endpoint="https://contoso.openai.azure.com/"
-      export HuggingFace__ApiKey="...."
-      export Bing__ApiKey="...."
-      ```
-
-    - PowerShell:
-
-      ```ps
-      $env:OpenAI__ApiKey = "sk-...."
-      $env:AzureOpenAI__ApiKey = "...."
-      $env:AzureOpenAI__DeploymentName = "azure-text-davinci-003"
-      $env:AzureOpenAIEmbeddings__DeploymentName = "azure-text-embedding-ada-002"
-      $env:AzureOpenAI__Endpoint = "https://contoso.openai.azure.com/"
-      $env:HuggingFace__ApiKey = "...."
-      $env:Bing__ApiKey = "...."
-      ```
+### Option 3: Use Environment Variables
+You may also set the test settings in your environment variables. The environment variables will override the settings in the `testsettings.development.json` file.
+
+When setting environment variables, use a double underscore (i.e. "\_\_") to delineate between parent and child properties. For example:
+
+- bash:
+
+  ```bash
+  export OpenAI__ApiKey="sk-...."
+  export AzureOpenAI__ApiKey="...."
+  export AzureOpenAI__DeploymentName="azure-text-davinci-003"
+  export AzureOpenAIEmbeddings__DeploymentName="azure-text-embedding-ada-002"
+  export AzureOpenAI__Endpoint="https://contoso.openai.azure.com/"
+  export HuggingFace__ApiKey="...."
+  export Bing__ApiKey="...."
+  ```
+
+- PowerShell:
+
+  ```ps
+  $env:OpenAI__ApiKey = "sk-...."
+  $env:AzureOpenAI__ApiKey = "...."
+  $env:AzureOpenAI__DeploymentName = "azure-text-davinci-003"
+  $env:AzureOpenAIEmbeddings__DeploymentName = "azure-text-embedding-ada-002"
+  $env:AzureOpenAI__Endpoint = "https://contoso.openai.azure.com/"
+  $env:HuggingFace__ApiKey = "...."
+  $env:Bing__ApiKey = "...."
+  ```
diff --git a/dotnet/src/IntegrationTests/TestHelpers.cs b/dotnet/src/IntegrationTests/TestHelpers.cs
@@ -14,20 +14,21 @@ internal static class TestHelpers
 {
     internal static void ImportSampleSkills(IKernel target)
     {
-        var chatSkill = GetSkill("ChatSkill", target);
-        var summarizeSkill = GetSkill("SummarizeSkill", target);
-        var writerSkill = GetSkill("WriterSkill", target);
-        var calendarSkill = GetSkill("CalendarSkill", target);
-        var childrensBookSkill = GetSkill("ChildrensBookSkill", target);
-        var classificationSkill = GetSkill("ClassificationSkill", target);
-        var codingSkill = GetSkill("CodingSkill", target);
-        var funSkill = GetSkill("FunSkill", target);
-        var intentDetectionSkill = GetSkill("IntentDetectionSkill", target);
-        var miscSkill = GetSkill("MiscSkill", target);
-        var qaSkill = GetSkill("QASkill", target);
+        var chatSkill = GetSkills(target,
+            "ChatSkill",
+            "SummarizeSkill",
+            "WriterSkill",
+            "CalendarSkill",
+            "ChildrensBookSkill",
+            "ClassificationSkill",
+            "CodingSkill",
+            "FunSkill",
+            "IntentDetectionSkill",
+            "MiscSkill",
+            "QASkill");
     }
 
-    internal static IDictionary<string, ISKFunction> GetSkill(string skillName, IKernel target)
+    internal static IDictionary<string, ISKFunction> GetSkills(IKernel target, params string[] skillNames)
     {
         string? currentAssemblyDirectory = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location);
         if (string.IsNullOrWhiteSpace(currentAssemblyDirectory))
@@ -37,7 +38,7 @@ internal static void ImportSampleSkills(IKernel target)
 
         string skillParentDirectory = Path.GetFullPath(Path.Combine(currentAssemblyDirectory, "../../../../../../samples/skills"));
 
-        IDictionary<string, ISKFunction> skill = target.ImportSemanticSkillFromDirectory(skillParentDirectory, skillName);
+        IDictionary<string, ISKFunction> skill = target.ImportSemanticSkillFromDirectory(skillParentDirectory, skillNames);
         return skill;
     }
 }
diff --git a/dotnet/src/IntegrationTests/testsettings.json b/dotnet/src/IntegrationTests/testsettings.json
@@ -1,12 +1,12 @@
 {
   "OpenAI": {
-    "ServiceId": "text-davinci-002",
-    "ModelId": "text-davinci-002",
+    "ServiceId": "text-davinci-003",
+    "ModelId": "text-davinci-003",
     "ApiKey": ""
   },
   "AzureOpenAI": {
-    "ServiceId": "azure-text-davinci-002",
-    "DeploymentName": "text-davinci-002",
+    "ServiceId": "azure-text-davinci-003",
+    "DeploymentName": "text-davinci-003",
     "Endpoint": "",
     "ApiKey": ""
   },

diff --git a/dotnet/src/SemanticKernel/KernelExtensions/ImportSemanticSkillFromDirectory.cs b/dotnet/src/SemanticKernel/KernelExtensions/ImportSemanticSkillFromDirectory.cs
@@ -53,48 +53,50 @@ public static class ImportSemanticSkillFromDirectoryExtension
     /// </summary>
     /// <param name="kernel">Semantic Kernel instance</param>
     /// <param name="parentDirectory">Directory containing the skill directory, e.g. "d:\myAppSkills"</param>
-    /// <param name="skillDirectoryName">Name of the directory containing the selected skill, e.g. "StrategySkill"</param>
+    /// <param name="skillDirectoryNames">Name of the directories containing the selected skills, e.g. "StrategySkill"</param>
     /// <returns>A list of all the semantic functions found in the directory, indexed by function name.</returns>
     public static IDictionary<string, ISKFunction> ImportSemanticSkillFromDirectory(
-        this IKernel kernel, string parentDirectory, string skillDirectoryName)
+        this IKernel kernel, string parentDirectory, params string[] skillDirectoryNames)
     {
         const string CONFIG_FILE = "config.json";
         const string PROMPT_FILE = "skprompt.txt";
 
-        Verify.ValidSkillName(skillDirectoryName);
-        var skillDir = Path.Combine(parentDirectory, skillDirectoryName);
-        Verify.DirectoryExists(skillDir);
-
         var skill = new Dictionary<string, ISKFunction>();
 
-        string[] directories = Directory.GetDirectories(skillDir);
-        foreach (string dir in directories)
+        foreach (string skillDirectoryName in skillDirectoryNames)
         {
-            var functionName = Path.GetFileName(dir);
-
-            // Continue only if prompt template exists
-            var promptPath = Path.Combine(dir, PROMPT_FILE);
-            if (!File.Exists(promptPath)) { continue; }
+            Verify.ValidSkillName(skillDirectoryName);
+            var skillDir = Path.Combine(parentDirectory, skillDirectoryName);
+            Verify.DirectoryExists(skillDir);
 
-            // Load prompt configuration. Note: the configuration is optional.
-            var config = new PromptTemplateConfig();
-            var configPath = Path.Combine(dir, CONFIG_FILE);
-            if (File.Exists(configPath))
+            string[] directories = Directory.GetDirectories(skillDir);
+            foreach (string dir in directories)
             {
-                config = PromptTemplateConfig.FromJson(File.ReadAllText(configPath));
-                Verify.NotNull(config, $"Invalid prompt template configuration, unable to parse {configPath}");
-            }
+                var functionName = Path.GetFileName(dir);
 
-            kernel.Log.LogTrace("Config {0}: {1}", functionName, config.ToJson());
+                // Continue only if prompt template exists
+                var promptPath = Path.Combine(dir, PROMPT_FILE);
+                if (!File.Exists(promptPath)) { continue; }
 
-            // Load prompt template
-            var template = new PromptTemplate(File.ReadAllText(promptPath), config, kernel.PromptTemplateEngine);
+                // Load prompt configuration. Note: the configuration is optional.
+                var config = new PromptTemplateConfig();
+                var configPath = Path.Combine(dir, CONFIG_FILE);
+                if (File.Exists(configPath))
+                {
+                    config = PromptTemplateConfig.FromJson(File.ReadAllText(configPath));
+                    Verify.NotNull(config, $"Invalid prompt template configuration, unable to parse {configPath}");
+                }
 
-            // Prepare lambda wrapping AI logic
-            var functionConfig = new SemanticFunctionConfig(config, template);
+                kernel.Log.LogTrace("Config {0}: {1}", functionName, config.ToJson());
 
-            kernel.Log.LogTrace("Registering function {0}.{1} loaded from {2}", skillDirectoryName, functionName, dir);
-            skill[functionName] = kernel.RegisterSemanticFunction(skillDirectoryName, functionName, functionConfig);
+                // Load prompt template
+                var template = new PromptTemplate(File.ReadAllText(promptPath), config, kernel.PromptTemplateEngine);
+
+                var functionConfig = new SemanticFunctionConfig(config, template);
+
+                kernel.Log.LogTrace("Registering function {0}.{1} loaded from {2}", skillDirectoryName, functionName, dir);
+                skill[functionName] = kernel.RegisterSemanticFunction(skillDirectoryName, functionName, functionConfig);
+            }
         }
 
         return skill;